| Message ID | 903ef08411ce5f8456f1c5b7a099c526d19dbf21.1689257327.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Minor macro and type cleanups | expand |
On Thu, Jul 13, 2023 at 04:10:24PM +0200, David Sterba wrote: > There's no reason to use 'unsigned char' when we can simply use u8 > for byte buffers like for checksums or send buffers. The char could be > signed or unsigned depending on the compiler settings, per [1] it's not > simple. > > [1] https://lore.kernel.org/lkml/CAHk-=wgz3Uba8w7kdXhsqR1qvfemYL+OFQdefJnkeqXG8qZ_pA@mail.gmail.com/ I don't understand how this link is related. Plain char is indeed a mess because it could be signed or not. unsigned char vs u8 is just plain preference as the latter is always an alias to the former. u8 tends to look nice to my eyes, but this is a simple cleanup / style change and not relatded to the above. > > Checksum buffer item already uses u8 so this unifies the types and in > btrfs_readdir_delayed_dir_index() the unsigned char has been probably > inherited from fs_ftype_to_dtype() bit it's not strictly necessary That one isn't used by a cast anyway.. > item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_csum_item); > + item = (struct btrfs_csum_item *)((u8 *)item + csum_offset * csum_size); This still looks a bit odd to me, because btrfs_csum_item returns the pointer as the last argument, and the next thing we'd do here is to cast that away. If you don't mind using the gcc extention of void pointer arithmetics that we use all over the kernel this could become: item = btrfs_item_ptr(leaf, path->slots[0], void) + csum_offset * csum_size; which I find quite a bit readable. > leaf = path->nodes[0]; > csum: > item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_csum_item); > - item_end = (struct btrfs_csum_item *)((unsigned char *)item + > + item_end = (struct btrfs_csum_item *)((u8 *)item + > btrfs_item_size(leaf, path->slots[0])); > - item = (struct btrfs_csum_item *)((unsigned char *)item + > - csum_offset * csum_size); > + item = (struct btrfs_csum_item *)((u8 *)item + csum_offset * csum_size); And something similar here. No point in asking btrfs_item_ptr to cast to a a btrfs_csum_item, when you then instantly cast it to u8 (or void per the above) for pointer arithmetic.
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index 6b457b010cbc..fd8a9916bf64 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -1733,7 +1733,7 @@ int btrfs_readdir_delayed_dir_index(struct dir_context *ctx, char *name; int name_len; int over = 0; - unsigned char d_type; + u8 d_type; if (list_empty(ins_list)) return 0; diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index 696bf695d8eb..acd09cfaf62c 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -245,8 +245,7 @@ btrfs_lookup_csum(struct btrfs_trans_handle *trans, } } item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_csum_item); - item = (struct btrfs_csum_item *)((unsigned char *)item + - csum_offset * csum_size); + item = (struct btrfs_csum_item *)((u8 *)item + csum_offset * csum_size); return item; fail: if (ret > 0) @@ -1223,10 +1222,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans, leaf = path->nodes[0]; csum: item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_csum_item); - item_end = (struct btrfs_csum_item *)((unsigned char *)item + + item_end = (struct btrfs_csum_item *)((u8 *)item + btrfs_item_size(leaf, path->slots[0])); - item = (struct btrfs_csum_item *)((unsigned char *)item + - csum_offset * csum_size); + item = (struct btrfs_csum_item *)((u8 *)item + csum_offset * csum_size); found: ins_size = (u32)(sums->len - total_bytes) >> fs_info->sectorsize_bits; ins_size *= csum_size; diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 8bfd44750efe..dffdf6c54726 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -796,7 +796,7 @@ static int send_cmd(struct send_ctx *sctx) put_unaligned_le32(sctx->send_size - sizeof(*hdr), &hdr->len); put_unaligned_le32(0, &hdr->crc); - crc = btrfs_crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size); + crc = btrfs_crc32c(0, (u8 *)sctx->send_buf, sctx->send_size); put_unaligned_le32(crc, &hdr->crc); ret = write_buf(sctx->send_filp, sctx->send_buf, sctx->send_size,
There's no reason to use 'unsigned char' when we can simply use u8 for byte buffers like for checksums or send buffers. The char could be signed or unsigned depending on the compiler settings, per [1] it's not simple. [1] https://lore.kernel.org/lkml/CAHk-=wgz3Uba8w7kdXhsqR1qvfemYL+OFQdefJnkeqXG8qZ_pA@mail.gmail.com/ Checksum buffer item already uses u8 so this unifies the types and in btrfs_readdir_delayed_dir_index() the unsigned char has been probably inherited from fs_ftype_to_dtype() bit it's not strictly necessary Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/delayed-inode.c | 2 +- fs/btrfs/file-item.c | 8 +++----- fs/btrfs/send.c | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-)