Subject: Re: [Boost-users] [Review] UUID library (mini-)review starts today, November 23rd
From: Andy Tompkins (atompkins_at_[hidden])
Date: 2008-12-09 12:07:11
On Sat, 6 Dec 2008 15:59:41 -0500, "Scott McMurray"
<me22.ca+boost_at_[hidden]> said:
> 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.
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