[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/8] util: Add UUID API
From: |
Fam Zheng |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/8] util: Add UUID API |
Date: |
Tue, 9 Aug 2016 10:34:17 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- [Qemu-block] [PATCH v2 0/8] UUID clean ups for 2.8, Fam Zheng, 2016/08/08
- [Qemu-block] [PATCH v2 2/8] tests: Add uuid tests, Fam Zheng, 2016/08/08
- [Qemu-block] [PATCH v2 3/8] vhdx: Use QEMU UUID API, Fam Zheng, 2016/08/08
- [Qemu-block] [PATCH v2 4/8] vdi: Use QEMU UUID API, Fam Zheng, 2016/08/08
- [Qemu-block] [PATCH v2 5/8] vpc: Use QEMU UUID API, Fam Zheng, 2016/08/08
- [Qemu-block] [PATCH v2 6/8] crypto: Switch to QEMU UUID API, Fam Zheng, 2016/08/08