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

On Sat, 6 Dec 2008 15:59:41 -0500, "Scott McMurray" said:

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.

Thank you. I understand now. I will change this.

...

...

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.

Yes, I like that.

...

...

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.

I understand your desire. I would like to put in mutating iterators. I wrote in my last post, should the uuid library require CHAR_BITS == 8, or can it relax and only require that CHAR_BITS % 8 == 0. That is the uuid library only needs 8 bit bytes. I guess the difference will be in implementating the iterators.

~ Scott

Regards, Andy Tompkins