Re: [PATCH] Fix build on non GLIBC platforms (FreeBSD at least)

[Robert N. <rn...@2h...>]

On Tue, 2009-09-22 at 13:29 -0700, Ian Romanick wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Brian Paul wrote:
> > Robert Noland wrote:
> >> Build was broken by commit 9666529b5a5be1fcde82caadc2fe2efa5ea81e49
> >>
> >> I'm not certain that this is entirely the correct fix since the demo
> >> from bug #23774 seemed to work before the commit that broke the build.
> >>
> >> Signed-off-by: Robert Noland <rn...@2h...>
> >> ---
> >>  src/glx/x11/glxhash.c |    7 +++++++
> >>  1 files changed, 7 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/src/glx/x11/glxhash.c b/src/glx/x11/glxhash.c
> >> index 7d28ada..dcf8c98 100644
> >> --- a/src/glx/x11/glxhash.c
> >> +++ b/src/glx/x11/glxhash.c
> >> @@ -87,6 +87,12 @@
> >>  
> >>  #define HASH_ALLOC malloc
> >>  #define HASH_FREE  free
> >> +#ifndef __GLIBC__
> 
> According to the manual page, _SVID_SOURCE || _BSD_SOURCE is required
> for initstate_r and friends.  The non-_r versions require _SVID_SOURCE
> || _BSD_SOURCE || _XOPEN_SOURCE >= 500.  Specifically, it says:
> 
>    Feature Test Macro Requirements for glibc (see feature_test_macros(7)):
> 
>        random_r(), srandom_r(), initstate_r(), setstate_r(): _SVID_SOURCE ||
>        _BSD_SOURCE
> 
> Does that mean that you have to use (__GLIBC__ && (_SVID_SOURCE ||
> _BSD_SOURCE)), or does it mean that just using (_SVID_SOURCE ||
> _BSD_SOURCE) is sufficient?  I guess the other alternative is to have
> some autoconf mechanism to test for those functions.  That's probably
> the right answer anyway.
> 
> The non-_r versions don't fix the bug.  The bug was that the hash
> functions modified the global random number state.
> 
> >> +#define HASH_RANDOM_DECL    char *ps, rs[256]
> >> +#define HASH_RANDOM_INIT(seed)    ps = initstate(seed, rs, sizeof(rs))
> >> +#define HASH_RANDOM        random()
> >> +#define HASH_RANDOM_DESTROY    setstate(ps)
> >> +#else
> >>  #define HASH_RANDOM_DECL    struct random_data rd; int32_t rv; char
> >> rs[256]
> >>  #define HASH_RANDOM_INIT(seed)                    \
> >>     do {                                \
> >> @@ -95,6 +101,7 @@
> >>     } while(0)
> >>  #define HASH_RANDOM             ((void) random_r(&rd, &rv), rv)
> >>  #define HASH_RANDOM_DESTROY
> >> +#endif
> >>  
> >>  typedef struct __glxHashBucket
> >>  {
> > 
> > I was just looking at this after my coworker found the build failure on
> > MacOS.
> > 
> > Would nrand48() be a more portable option?  Ian?
> > 
> > See http://evanjones.ca/random-thread-safe.html
> 
> nrand48 may be thread-safe, but it still brings back the bug.

I guess that I am still trying to understand the original bug... The
demo program that was included in the bug report seems to work fine
here, unless I don't know what the output *should* look like.  It seems
to work both before and after this change on FreeBSD.  FWIW, we use mesa
glut, not freeglut.

balrog% ./opengl-demo
These numbers are random
1219944491
1580682328
7689659
391079993
1581482531
571799598
246523261
820492564
1026025761
121279717
IRQ's not enabled, falling back to busy waits: 2 0

These numbers are not
386222616
1547925878
1373331788
433122960
1675509037
327321748
1590998669
1595741086
1844648666
1936201370

robert.

> I guess we can use feature test macros to select the _r functions on
> platforms that support it, and we can let the bug continue to exist on
> platforms that leave no (reasonable) alternative.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAkq5M54ACgkQX1gOwKyEAw8aEQCgkYH9x9mvStfpx9AFJbEN9lDm
> powAoIqN4zG1H1+SfWozGa4X121WjWL7
> =FYL2
> -----END PGP SIGNATURE-----
-- 
Robert Noland <rn...@2h...>
2Hip Networks