From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:35778)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1bWwyu-0002ak-8k
	for qemu-devel@nongnu.org; Mon, 08 Aug 2016 22:41:29 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1bWwys-0002hC-BM
	for qemu-devel@nongnu.org; Mon, 08 Aug 2016 22:41:27 -0400
Date: Tue, 9 Aug 2016 10:34:17 +0800
From: Fam Zheng <famz@redhat.com>
Message-ID: <20160809023417.GC16125@al.usersys.redhat.com>
References: <1470636568-8291-1-git-send-email-famz@redhat.com>
	<1470636568-8291-2-git-send-email-famz@redhat.com>
	<87zionecy2.fsf@dusky.pond.sub.org>
	<20160808195125.GK24882@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160808195125.GK24882@localhost.localdomain>
Subject: Re: [Qemu-devel] [PATCH v2 1/8] util: Add UUID API
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>, Jeff Cody <jcody@redhat.com>
Cc: kwolf@redhat.com, qemu-block@nongnu.org, sw@weilnetz.de, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, pbonzini@redhat.com, mreitz@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