4:05 p.m.
On Tue, Jan 20, 2009 at 12:22:46PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)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 :|