Re: [libvirt] PATCH: 21/25: Use random_r for random numbers

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Tue, Jan 20, 2009 at 12:22:46PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange@xxxxxxxxxx> 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 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list