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

On Sat, Dec 6, 2008 at 15:11, Andy Tompkins wrote:

On Wed, 3 Dec 2008 15:57:45 -0500, "Scott McMurray" 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(&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