On Tue, Jan 20, 2009 at 12:22:46PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
Now that gnulib's rand module is imported, we have a decent quality random number generator that's portable. We don't want to mess with the apps state, so in virInitialize() we explicitly initialize our own private random nubmer generator state with virRandomInitialize().
The util.h file gains a convenience macro, since random_r() is horrible to call and we need to protect our global state
int virRandom(int max) ... +int virRandomInitialize(void)
This all looks good and correct. Only one suggestion: consider exposing the seed-setting functionality by moving it up a level:
int virRandomInitialize(int seed)
in case we ever want reproducibly random MAC addresses and UUIDs.
How about this then.... BTW, what's good practice for actually initializing a seed ? time() ? time() ^ getpid() ? anything better ? Daniel diff --git a/Makefile.maint b/Makefile.maint --- a/Makefile.maint +++ b/Makefile.maint @@ -117,7 +117,7 @@ sc_prohibit_nonreentrant: @fail=0 ; \ for i in $(NON_REENTRANT) ; \ do \ - grep -nE "\<$$i\>[:space:]*\(" $$($(VC_LIST_EXCEPT)) && \ + grep --before 2 --after 1 -nE "\<$$i\>[:space:]*\(" $$($(VC_LIST_EXCEPT)) && \ fail=1 && echo "$(ME): use $${i}_r, not $${i}" || : ; \ done ; \ exit $$fail diff --git a/src/libvirt.c b/src/libvirt.c --- a/src/libvirt.c +++ b/src/libvirt.c @@ -257,7 +257,8 @@ virInitialize(void) initialized = 1; if (virThreadInitialize() < 0 || - virErrorInitialize() < 0) + virErrorInitialize() < 0 || + virRandomInitialize(time(NULL) ^ getpid())) return -1; #ifdef ENABLE_DEBUG @@ -332,23 +333,19 @@ DllMain (HINSTANCE instance ATTRIBUTE_UN { switch (reason) { case DLL_PROCESS_ATTACH: - fprintf(stderr, "Initializing DLL\n"); virInitialize(); break; case DLL_THREAD_ATTACH: - fprintf(stderr, "Thread start\n"); /* Nothing todo in libvirt yet */ break; case DLL_THREAD_DETACH: - fprintf(stderr, "Thread exit\n"); /* Release per-thread local data */ virThreadOnExit(); break; case DLL_PROCESS_DETACH: - fprintf(stderr, "Process exit\n"); /* Don't bother releasing per-thread data since (hopefully) windows cleans up everything on process exit */ diff --git a/src/util.c b/src/util.c --- a/src/util.c +++ b/src/util.c @@ -32,6 +32,7 @@ #include <fcntl.h> #include <errno.h> #include <poll.h> +#include <time.h> #include <sys/types.h> #include <sys/stat.h> #include <sys/ioctl.h> @@ -59,6 +60,7 @@ #include "buf.h" #include "util.h" #include "memory.h" +#include "threads.h" #ifndef NSIG # define NSIG 32 @@ -1285,9 +1287,9 @@ void virGenerateMacAddr(const unsigned c addr[0] = prefix[0]; addr[1] = prefix[1]; addr[2] = prefix[2]; - addr[3] = (int)(256*(rand()/(RAND_MAX+1.0))); - addr[4] = (int)(256*(rand()/(RAND_MAX+1.0))); - addr[5] = (int)(256*(rand()/(RAND_MAX+1.0))); + addr[3] = virRandom(256); + addr[4] = virRandom(256); + addr[5] = virRandom(256); } @@ -1436,6 +1438,36 @@ int virKillProcess(pid_t pid, int sig) } +static char randomState[128]; +static struct random_data randomData; +static virMutex randomLock; + +int virRandomInitialize(unsigned int seed) +{ + if (virMutexInit(&randomLock) < 0) + return -1; + + if (initstate_r(seed, + randomState, + sizeof(randomState), + &randomData) < 0) + return -1; + + return 0; +} + +int virRandom(int max) +{ + int32_t ret; + + virMutexLock(&randomLock); + random_r(&randomData, &ret); + virMutexUnlock(&randomLock); + + return (int) ((double)max * ((double)ret / (double)RAND_MAX)); +} + + #ifdef HAVE_GETPWUID_R char *virGetUserDirectory(virConnectPtr conn, uid_t uid) diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -177,4 +177,7 @@ char *virGetUserDirectory(virConnectPtr uid_t uid); #endif +int virRandomInitialize(unsigned int seed); +int virRandom(int max); + #endif /* __VIR_UTIL_H__ */ diff --git a/src/uuid.c b/src/uuid.c --- a/src/uuid.c +++ b/src/uuid.c @@ -35,6 +35,7 @@ #include "c-ctype.h" #include "internal.h" +#include "util.h" #define qemudLog(level, msg...) fprintf(stderr, msg) @@ -74,9 +75,8 @@ static int virUUIDGeneratePseudoRandomBytes(unsigned char *buf, int buflen) { - srand(time(NULL)); while (buflen > 0) { - *buf = (int) (255.0 * (rand() / (double) RAND_MAX)); + *buf = virRandom(256); buflen--; } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|