Re: [PATCH v2 1/3] uuid: Add inline helpers to operate on raw buffers

[David Sterba <dsterba@suse.cz>]

On Tue, Jul 16, 2019 at 06:22:22PM +0300, Andy Shevchenko wrote:
> On Tue, Jul 16, 2019 at 05:11:33PM +0200, Christoph Hellwig wrote:
> > On Tue, Jul 16, 2019 at 06:04:16PM +0300, Andy Shevchenko wrote:
> > > +static inline void guid_copy_from_raw(guid_t *dst, const __u8 *src)
> > > +{
> > > +	memcpy(dst, (const guid_t *)src, sizeof(guid_t));
> > > +}
> > > +
> > > +static inline void guid_copy_to_raw(__u8 *dst, const guid_t *src)
> > > +{
> > > +	memcpy((guid_t *)dst, src, sizeof(guid_t));
> > > +}
> > 
> > Maybe import_guid/export_guid is a better name?
> 
> Yes, sounds good to me.
> 
> > Either way, I don't think we need the casts, and they probably want
> > kerneldoc comments describing their use.
> > 
> > Same for the uuid side.
> 
> Got it.
> 
> > > +static inline void guid_gen_raw(__u8 *guid)
> > > +{
> > > +	guid_gen((guid_t *)guid);
> > > +}
> > > +
> > > +static inline void uuid_gen_raw(__u8 *uuid)
> > > +{
> > > +	uuid_gen((uuid_t *)uuid);
> > > +}
> > 
> > I hate this raw naming.  If people really want to use the generators on
> > u8 fields a cast seems more descriptive then hiding it.
> 
> This entire patch because of BTRFS maintainers, they didn't want the explicit
> casts. Maybe something has been changed, I dunno.

No change on our side. The uuids are u8 in the on-disk structures, that
will stay. The uuid functions use a different type so the casts have to
be added, that's clear. The question is if it's up to the API to provide
functions that take u8, or btrfs code to put typecasts everywhere or
carry own wrappers that do that.

I tend to avoid the explicit typecasts for widely used functions because
it's easy to forget them, and it overrides the type checks (that could
be caught by compiler but also not).

Specifically for uuid, the endianness might matter, so that we use the
raw buffers makes things more explicit.