* [PATCH 0/2] Minor macro and type cleanups
@ 2023-07-13 14:10 David Sterba
2023-07-13 14:10 ` [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts David Sterba
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Sterba @ 2023-07-13 14:10 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Get rid of unsigned char, and use sizeof_filed().
David Sterba (2):
btrfs: replace unsigned char with u8 for type casts
btrfs: use helper sizeof_field in struct accessors
fs/btrfs/accessors.h | 23 +++++++++++------------
fs/btrfs/delayed-inode.c | 2 +-
fs/btrfs/file-item.c | 8 +++-----
fs/btrfs/send.c | 2 +-
4 files changed, 16 insertions(+), 19 deletions(-)
--
2.40.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts
2023-07-13 14:10 [PATCH 0/2] Minor macro and type cleanups David Sterba
@ 2023-07-13 14:10 ` David Sterba
2023-07-14 14:44 ` Christoph Hellwig
2023-07-13 14:10 ` [PATCH 2/2] btrfs: use helper sizeof_field in struct accessors David Sterba
2023-07-14 10:24 ` [PATCH 0/2] Minor macro and type cleanups Johannes Thumshirn
2 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2023-07-13 14:10 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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(-)
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,
--
2.40.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] btrfs: use helper sizeof_field in struct accessors
2023-07-13 14:10 [PATCH 0/2] Minor macro and type cleanups David Sterba
2023-07-13 14:10 ` [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts David Sterba
@ 2023-07-13 14:10 ` David Sterba
2023-07-14 14:45 ` Christoph Hellwig
2023-07-14 10:24 ` [PATCH 0/2] Minor macro and type cleanups Johannes Thumshirn
2 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2023-07-13 14:10 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
There's a helper for obtaining size of a struct member, we can use it
instead of open coding the pointer magic.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/accessors.h | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/accessors.h b/fs/btrfs/accessors.h
index ceadfc5d6c66..8cfc8214109c 100644
--- a/fs/btrfs/accessors.h
+++ b/fs/btrfs/accessors.h
@@ -3,6 +3,8 @@
#ifndef BTRFS_ACCESSORS_H
#define BTRFS_ACCESSORS_H
+#include <linux/stddef.h>
+
struct btrfs_map_token {
struct extent_buffer *eb;
char *kaddr;
@@ -34,13 +36,13 @@ static inline void put_unaligned_le8(u8 val, void *p)
read_extent_buffer(eb, (char *)(result), \
((unsigned long)(ptr)) + \
offsetof(type, member), \
- sizeof(((type *)0)->member)))
+ sizeof_field(type, member)))
#define write_eb_member(eb, ptr, type, member, result) (\
write_extent_buffer(eb, (char *)(result), \
((unsigned long)(ptr)) + \
offsetof(type, member), \
- sizeof(((type *)0)->member)))
+ sizeof_field(type, member)))
#define DECLARE_BTRFS_SETGET_BITS(bits) \
u##bits btrfs_get_token_##bits(struct btrfs_map_token *token, \
@@ -62,25 +64,25 @@ DECLARE_BTRFS_SETGET_BITS(64)
static inline u##bits btrfs_##name(const struct extent_buffer *eb, \
const type *s) \
{ \
- static_assert(sizeof(u##bits) == sizeof(((type *)0))->member); \
+ static_assert(sizeof(u##bits) == sizeof_field(type, member)); \
return btrfs_get_##bits(eb, s, offsetof(type, member)); \
} \
static inline void btrfs_set_##name(const struct extent_buffer *eb, type *s, \
u##bits val) \
{ \
- static_assert(sizeof(u##bits) == sizeof(((type *)0))->member); \
+ static_assert(sizeof(u##bits) == sizeof_field(type, member)); \
btrfs_set_##bits(eb, s, offsetof(type, member), val); \
} \
static inline u##bits btrfs_token_##name(struct btrfs_map_token *token, \
const type *s) \
{ \
- static_assert(sizeof(u##bits) == sizeof(((type *)0))->member); \
+ static_assert(sizeof(u##bits) == sizeof_field(type, member)); \
return btrfs_get_token_##bits(token, s, offsetof(type, member));\
} \
static inline void btrfs_set_token_##name(struct btrfs_map_token *token,\
type *s, u##bits val) \
{ \
- static_assert(sizeof(u##bits) == sizeof(((type *)0))->member); \
+ static_assert(sizeof(u##bits) == sizeof_field(type, member)); \
btrfs_set_token_##bits(token, s, offsetof(type, member), val); \
}
@@ -111,17 +113,14 @@ static inline void btrfs_set_##name(type *s, u##bits val) \
static inline u64 btrfs_device_total_bytes(const struct extent_buffer *eb,
struct btrfs_dev_item *s)
{
- static_assert(sizeof(u64) ==
- sizeof(((struct btrfs_dev_item *)0))->total_bytes);
- return btrfs_get_64(eb, s, offsetof(struct btrfs_dev_item,
- total_bytes));
+ static_assert(sizeof(u64) == sizeof_field(struct btrfs_dev_item, total_bytes));
+ return btrfs_get_64(eb, s, offsetof(struct btrfs_dev_item, total_bytes));
}
static inline void btrfs_set_device_total_bytes(const struct extent_buffer *eb,
struct btrfs_dev_item *s,
u64 val)
{
- static_assert(sizeof(u64) ==
- sizeof(((struct btrfs_dev_item *)0))->total_bytes);
+ static_assert(sizeof(u64) == sizeof_field(struct btrfs_dev_item, total_bytes));
WARN_ON(!IS_ALIGNED(val, eb->fs_info->sectorsize));
btrfs_set_64(eb, s, offsetof(struct btrfs_dev_item, total_bytes), val);
}
--
2.40.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Minor macro and type cleanups
2023-07-13 14:10 [PATCH 0/2] Minor macro and type cleanups David Sterba
2023-07-13 14:10 ` [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts David Sterba
2023-07-13 14:10 ` [PATCH 2/2] btrfs: use helper sizeof_field in struct accessors David Sterba
@ 2023-07-14 10:24 ` Johannes Thumshirn
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2023-07-14 10:24 UTC (permalink / raw)
To: David Sterba, linux-btrfs@vger.kernel.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts
2023-07-13 14:10 ` [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts David Sterba
@ 2023-07-14 14:44 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2023-07-14 14:44 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: use helper sizeof_field in struct accessors
2023-07-13 14:10 ` [PATCH 2/2] btrfs: use helper sizeof_field in struct accessors David Sterba
@ 2023-07-14 14:45 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2023-07-14 14:45 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-14 14:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-13 14:10 [PATCH 0/2] Minor macro and type cleanups David Sterba
2023-07-13 14:10 ` [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts David Sterba
2023-07-14 14:44 ` Christoph Hellwig
2023-07-13 14:10 ` [PATCH 2/2] btrfs: use helper sizeof_field in struct accessors David Sterba
2023-07-14 14:45 ` Christoph Hellwig
2023-07-14 10:24 ` [PATCH 0/2] Minor macro and type cleanups Johannes Thumshirn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox