Re: [PATCH 1/2] btrfs: replace unsigned char with u8 for type casts

[Christoph Hellwig <hch@infradead.org>]

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.