[Review] The review of the Boost.Guid library starts today (April 30th)
Hi all, The review of the proposed Boost Guid library written by Andy Tompkins starts today (April 30th, 2007) and ends May 10th, 2007. You can download the library here: http://www.boost-consulting.com/vault/ (file guid_v7.zip). --------------------------------------------------- About the library: Guid is a library providing an implementation of Globally Unique Identifiers. Guid's are used in many facets of programming, including databases, networks, remote procedures, serializations, transactions, identifying documents, and identifying classes. This library is efficient and is able to create name-based and random-based guids. --------------------------------------------------- Please always state in your review, whether you think the library should be accepted as a Boost library! Additionally please consider giving feedback on the following general topics: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? Regards Hartmut Review Manager
This is not a review, just a remark.
From the documentation: The boost::guid::create() function returns a random-number-based guid... All functions are thread-safe except boost::guid::create().
Why isn't this routine made thread-safe? Why are the other routines thread-safe when this isn't? /$
"Henrik Sundberg" <storangen@gmail.com> wrote in news:aef9adb00704300822q2459684ex994be6b70700275a@mail.gmail.com:
This is not a review, just a remark.
From the documentation: The boost::guid::create() function returns a random-number-based guid... All functions are thread-safe except boost::guid::create().
Bad working. All functions are thread-safe without using thread primitives, except for boost::guid::create(). The function, boost::guid::create(), needs a mutex to be thread safe. If BOOST_HAS_THREADS is defined, Boost.Thread is used to make boost::guid::create() thread safe. Andy Tompkins.
Why isn't this routine made thread-safe? Why are the other routines thread-safe when this isn't?
/$ _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
"Andy" <atompkins@fastmail.fm> wrote in message news:Xns99228F292C49atompkinsfastmailfm@80.91.229.5...
"Henrik Sundberg" <storangen@gmail.com> wrote in news:aef9adb00704300822q2459684ex994be6b70700275a@mail.gmail.com:
This is not a review, just a remark.
From the documentation: The boost::guid::create() function returns a random-number-based guid... All functions are thread-safe except boost::guid::create().
Bad working.
wording?
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> wrote in news:f15b6a$dht$1@sea.gmane.org:
"Andy" <atompkins@fastmail.fm> wrote in message news:Xns99228F292C49atompkinsfastmailfm@80.91.229.5...
"Henrik Sundberg" <storangen@gmail.com> wrote in news:aef9adb00704300822q2459684ex994be6b70700275a@mail.gmail.com:
This is not a review, just a remark.
From the documentation: The boost::guid::create() function returns a random-number-based guid... All functions are thread-safe except boost::guid::create().
Bad working.
wording?
Sorry. Bad wording. From the post I understand that the sentence does not convey what I intended. Andy Tompkins.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
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@arid.us> * 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
Hi, I like the idea of this library, it's small and useful. implementation: - Why does create_name_based() not take a std::string parameter? instead it takes a const char* and a length parameter. - As was already mentioned it would be neat if one could pass a seed to the create() function or even a different RNG. documentation: - Variant and version bits are mentioned but it is not explained what they mean. - I don't see an explanation for when I would want to use create_named_based() over create(), probably if I understood the above I would know, maybe it is explained in the linked pdf, but it should be in doc. What is the significance of the guid parameter and the name parameter? - The pdf link may go offline someday so the title and author(s) of the pdf should be mentioned in a footnote. I have not tried to compile/use this library. Based on the above I vote yes to accept the library into boost. Kevin
participants (6)
-
Andy -
Dan Nuffer -
Gennadiy Rozental -
Hartmut Kaiser -
Henrik Sundberg -
Kevin Sopp