Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays

[Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>]

On Wed, May 27, 2015 at 11:25:59PM +0000, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> > Sent: Thursday, May 28, 2015 02:23
> > To: Winkler, Tomas
> > Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw
> > arrays
> > 
> > On Wed, May 27, 2015 at 04:14:20PM -0700, Greg KH wrote:
> > > On Wed, May 27, 2015 at 05:42:22PM +0000, Winkler, Tomas wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> > > > > Sent: Wednesday, May 27, 2015 20:29
> > > > > To: Winkler, Tomas
> > > > > Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning
> > raw
> > > > > arrays
> > > > >
> > > > > On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
> > > > > >
> > > > > > >
> > > > > > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > > > > > > In order for mei client devices to use device id based on uuid we
> > > > > > > > have to use common types between user space (file2alias.c).
> > > > > > > > Similarly to vmbus,  mei  uses raw 16 byte array for that.
> > > > > > > > To leverage on existing infrastructure around uuid_le type
> > > > > > > > defined in uuid.h we add helper macros to handle conversions
> > between
> > > > > > > > raw 16 byte array and uuid_{le,be} types.
> > > > > > >
> > > > > > > You aren't adding a helper macro, you are just redefining the existing
> > > > > > > macros using a different one.
> > > > > >
> > > > > > Not exactly I'm using both the one I've added for device ids and the old
> > one for
> > > > > all the other flows.
> > > > > >
> > > > > >  But I can't see why this is needed, what
> > > > > > > does this solve that vmbus and other uses of the existing macros don't
> > > > > > > need?  In other words, what makes mei so special that it needs a
> > "lower"
> > > > > > > level macro than every other subsystem?
> > > > > >
> > > > > > It's not special there is actually a lot of code duplication around uuid
> > handling
> > > > > > every subsystem is using their own macros but it can be consolidated
> > around
> > > > > uuid.h
> > > > > >
> > > > > > So vmbus can use that
> > > > > > Instead of
> > > > > > /*
> > > > > >  * Network GUID
> > > > > >  * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> > > > > >  */
> > > > > > #define HV_NIC_GUID \
> > > > > >         .guid = { \
> > > > > >                         0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> > > > > >                         0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> > > > > >                 }
> > > > > >
> > > > > > The can use the new macro to make it more readable, something in spirit
> > of:
> > > > > >
> > > > > > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-
> > f2d2f965ed0e)
> > > > >
> > > > > Why the "__" usage here?  That signifies a "private" namespace, why add
> > > > > that to the user visible header files?
> > > >
> > > > I take any other suggestion for macro names.
> > > > Not sure how I would reuse the macros if I don't export them both,
> > > > second this can be used also by user space.
> > >
> > > But it's not needed at all.
> > >
> > > Below is a patch on top of my current tree that makes this patch not
> > > needed.  Any objection to me just applying it instead?
> > >
> > > > > And are you going to send patches for vmbus and other drivers to fix
> > > > > everything up to use these new macros?  Someone has to...
> > > >
> > > > Can be done but I cannot test their code and  now I'm busy with splitting the
> > big bus patch :)
> > >
> > > Ok, that means no one is ever going to do that work, so it's not a valid
> > > reason to accept such a change.  I prefer the patch below.
> > 
> > Oops, that didn't build, I forgot one change in the mei core (getting
> > rid of two casts.)  Try this one instead.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> > 
> > diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> > index de8fd089a8a4..dd52f224027e 100644
> > --- a/drivers/misc/mei/bus.c
> > +++ b/drivers/misc/mei/bus.c
> > @@ -54,9 +54,9 @@ static int mei_cl_device_match(struct device *dev, struct
> > device_driver *drv)
> > 
> >  	id = driver->id_table;
> > 
> > -	while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) {
> > +	while (uuid_le_cmp(NULL_UUID_LE, id->uuid)) {
> > 
> > -		if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) {
> > +		if (!uuid_le_cmp(*uuid, id->uuid)) {
> >  			if (id->name[0]) {
> >  				if (!strncmp(name, id->name, sizeof(id->name)))
> >  					return 1;
> 
> You can also drop the uuid_le_cast function 

Odd that gcc didn't complain about that.  Now removed, I'll resend it in
"official" form.

thanks,

greg k-h