Re: [Qemu-devel] [PATCH v2 1/8] util: Add UUID API

[Fam Zheng <famz@redhat.com>]

On Mon, 08/08 15:51, Jeff Cody wrote:
> > > +typedef unsigned char QemuUUID[16];
> > 
> > I'm afraid this typedef is problematic.  Consider:
> > 
> >     void use_uuid(QemuUUID uuid)
> >     {
> >         printf("sizeof(uuid) %zd\n", sizeof(uuid));
> >         uuid[0]++;
> >     }
> > 
> > QemuUUID is obviously a typedef name, so a reasonable reader may assume
> > (1) sizeof(uuid) is the size of the uuid, and (2) since uuid is passed
> > by value, the increment is not visible outside the function.  Both
> > assumptions are wrong, because array arguments degenerate into pointers.
> > 
> > I recommend to wrap it in a struct.
> >
> 
> If we are going for a semantic drop-in replacement for libuuid's uuid_t,
> Fam's typedef here is consistent with libuuid:
> 
>     typedef unsigned char uuid_t[16];
> 
> (Not to say that prohibits changing it, just pointing out there is value in
> mimicking libuuid's interfaces).

I am not 100% comforatble with libuuid way, so yes, it was for consistency. But
Markus' point is great, let's change it to the better way anyway, since it's
not a hard requirement to mimic libuuid.

Fam