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

Subject: Re: [Boost-users] [Review] UUID library (mini-)review starts today, November 23rd
From: Andy Tompkins (atompkins_at_[hidden])
Date: 2008-11-24 20:34:50

On Mon, 24 Nov 2008 17:56:20 +0000 (UTC), "Tim Blechmann"
<tim_at_[hidden]> said:
> > The mini-review of Andy Tompkins UUID library starts today, November
> > 23rd 2008, and will end on November 30th.
> 
> i just did a quick compile/testsuite test for now,  and i see three 
> trivial issues:
> 
> 1. it doesn't compile with gcc-4.4, because of a missing include.
> patch:
> --- a/boost/uuid/seed_rng.hpp
> +++ b/boost/uuid/seed_rng.hpp
> @@ -26,6 +26,7 @@
>  #include <memory.h>^M
>  #include <ctime>^M
>  #include <cstdlib>^M
> +#include <cstdio>^M
>  #include <boost/uuid/sha1.hpp>^M
>  //#include <boost/nondet_random.hpp> //forward declare
>  boost::random_device^M
>  ^M

I'll fix this.

> 
> 2. the testsuite fails on x86_64 because it is using hardcoded 32-bit 
> hash values:
> Running 1 test case...
> libs/uuid/test/test_uuid.cpp(146): error in "test_main_caller( argc, argv
> )": check uuid_hasher(uuid()) == 3565488559U failed [17562157925649023279
> != 3565488559]
> libs/uuid/test/test_uuid.cpp(147): error in "test_main_caller( argc, argv
> )": check uuid_hasher(u1) == 4159045843U failed [6622376135548557651 !=
> 4159045843]
> libs/uuid/test/test_uuid.cpp(148): error in "test_main_caller( argc, argv
> )": check uuid_hasher(u2) == 2713274306U failed [4700797755868797762 !=
> 2713274306]
> 
> *** 3 failures detected in test suite "Test Program"

Hmm, I assume that the value that uuid_hasher produces is correct since
I
mimicked the way Boost.Hash does it.  So, I believe I just need to
adjust the
code in test_uuid.cpp in a 64 bit environment where sizeof(std::size_t)
!= 32?

> 
> 
> 3. the sources use cr+lf eol style, i guess correctly importing the
> sources into subversion will fix this

I believe so this to be true.

> 
> 
> maybe i find some time to do a full review later this week. anyway, 
> i have been using the library for quite some time and would like to 
> see it being part of boost. thus i vote for acceptance.
> 
> cheers, tim
> 
> -- 
> tim_at_[hidden]
> http://tim.klingt.org
> 
> The composer makes plans, music laughs.
>   Morton Feldman
> 

Thanks,
  Andy Tompkins