Re: [Boost-users] [Review] UUID library (mini-)review starts today, November 23rd

Subject: Re: [Boost-users] [Review] UUID library (mini-)review starts today, November 23rd
From: Scott McMurray (me22.ca+boost_at_[hidden])
Date: 2008-12-06 15:59:41

On Sat, Dec 6, 2008 at 15:11, Andy Tompkins <atompkins_at_[hidden]> wrote:
> On Wed, 3 Dec 2008 15:57:45 -0500, "Scott McMurray"
> <me22.ca+boost_at_[hidden]> said:
>>
>> I was just looking deeper into the implementation, and saw some things
>> that worry me.  For example, this looks like an aliasing violation in
>> uuid.hpp line 279:
>>
>>     *reinterpret_cast<unsigned long*>(&data[i]) = _v_gen();
>
> I do not believe this is an aliasing violation.
>

3.10/15 from n2723 (and essentially the same as C++03):

If a program attempts to access the stored value of an object through
an lvalue of other than one of the following types the behavior is
undefined:
   — the dynamic type of the object,
   — a cv-qualified version of the dynamic type of the object,
   — a type similar (as defined in 4.4) to the dynamic type of the object,
   — a type that is the signed or unsigned type corresponding to the
dynamic type of the object,
   — a type that is the signed or unsigned type corresponding to a
cv-qualified version of the dynamic type of the object,
   — an aggregate or union type that includes one of the
aforementioned types among its members (including, recursively, a
member of a subaggregate or contained union),
   — a type that is a (possibly cv-qualified) base class type of the
dynamic type of the object,
   — a char or unsigned char type.

The dynamic type of the stored objects is unsigned char, and they are
being accessed through an lvalue of type unsigned long.

>> It's at least clearly byte-order-dependant, which isn't good, since
>> I'd expect a generator seeded the same way to always produce the same
>> UUIDs, regardless of architecture.
>
> Hmm, I was only trying to be efficient.  The generator produces values
> of size = sizeof(unsigned long) and I wanted to use all those bytes.  Do
> you have a suggestion that would improve this?
>

Actually, the default generator produces uint32_t, so (in the thread
on boost-dev I started earlier) I just switched to using that, and
used the same code that the sha1 uses to put 4-byte ints into 4 bytes.

>> I'm pleased to see proper shifting and masking in the SHA1 code, which
>> made me think of something: Why not just make the UUID class a PoD? It
>> has an architecture-independent in-memory format (rfc4122, section
>> 4.1.2, which is already followed), so turning it into one would be straight-
>>     forward, and would allow it to be very easily & safely splatted to
>>     and from files.
>
> I do really like having a class encapsulate the data and functions.
>

In general, I agree, but POD doesn't prevent having a class.  Also,
the UUID itself imposes no invariant on the stored data other than
that it's initialized, and allowing arbitrary modification can't break
that.

That said, POD-ness is the least important of the aspects I've raised.
 I would like mutating iterators into the array, though.

~ Scott