Re: [boost] [Review] The review of the Boost.Guid library starts today (April 30th)

From: Dan Nuffer (dan-boost_at_[hidden])
Date: 2007-05-02 00:44:27

Hartmut Kaiser wrote:
> - What is your evaluation of the design?
>   

I think it is good.
It would be nice to also be able to generate a time-based version, but I 
realize that's a lot harder to implement.
The seeding of the PRNG is flawed, and hard-coding the PRNG engine may 
cause problems for some applications. For instance, using GUIDs in a 
security application where guessing the next GUID should be infeasible 
would require the use of a cryptographically secure PRNG. I think the 
library should allow any PRNG to be used, this would also allow the user 
to seed it as they desire.

> - What is your evaluation of the implementation?
>   

- A major problem is the seeding of the PRNG with time(0). That means 
that any GUIDs generated by any processes which start in the same second 
will all be identical, which pretty much defeats the point of using a GUID.

- sha1.h says:
 *  Copyright (C) 1998
 *  Paul E. Jones <paulej_at_[hidden]>
 *  All Rights Reserved.
That doesn't sound good from a licensing point of view.

- I don't like that the library is all in the header when it doesn't 
need to be. Some of the functions aren't trivial and would cause 
unnecessary code bloat. I'd like to see the function bodies moved into a 
.ipp file and a library provided.

- operator>> seems a bit inefficient (no I haven't timed it). Creating a 
temporary stringstream to convert two hex characters seems a bit overkill.

> - What is your evaluation of the documentation?
>   

Overall I think it is good.
This paragraph is contradictory at first read and confused me until I 
looked at the code: "All functions are thread-safe except 
boost::guid::create(). Only one random number generator is created and 
used for all threads to minimize the possibility of generating duplicate 
*guid*s. The Boost Thread <http://www.boost.org/doc/html/threads.html> 
library is used to make this thread safe."

> - What is your evaluation of the potential usefulness of the library?
>   

This is a useful library, I have used a similar GUID generation class 
for years.

> - Did you try to use the library?  With what compiler?  Did you have any
> problems?
>   

No.

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
>   

I spent about an hour reading the documentation, code and tests.

> - Are you knowledgeable about the problem domain?
>
>   

Yes, I have read the GUID standard and studied various implementations 
and written my own C++ GUID class, which I've been using for a few years.

Summary:
No vote, because of the time(0) seeding problem and sha1.h licensing 
problem. If those are fixed, then my vote would be a yes.

-- 
Dan Nuffer