Skip to contentSkip to conversation

cleanup: Move init_random_seed() to where it is being used

PATCH In review David Sommerseth 2 participants 3 messages Jul 25, 2017 – Aug 11, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cleanup: Move init_random_seed() to where it is being used

The init_random_seed() function is only used by the init_static() in init.c. As this function was pretty basic and it is only being called once, it was merged into init_static() instead of keeping it as a separate function.

(I agree that calling functions often makes the code more readable, but I would rather see that as a part of cleaning up the whole init_static() function - in fact when moving all "unit tests" in init_static() to cmocka, it will not be too bad in the end.)

---
 src/openvpn/init.c | 17 +++++++++++++++--
 src/openvpn/misc.c | 19 -------------------
 src/openvpn/misc.h |  3 ---
 3 files changed, 15 insertions(+), 24 deletions(-)
src/openvpn/init.c +15 -2
index bc3b81e3..860df774 100644
@@ -610,6 +610,7 @@ init_port_share(struct context *c)
 
 #endif /* if PORT_SHARE */
 
+
 bool
 init_static(void)
 {
@@ -619,8 +620,20 @@ init_static(void)
     crypto_init_dmalloc();
 #endif
 
-    init_random_seed();         /* init random() function, only used as
-                                 * source for weak random numbers */
+
+    /*
+     * Initialize random number seed.  random() is only used
+     * when "weak" random numbers are acceptable.
+     * SSL library routines are always used when cryptographically
+     * strong random numbers are required.
+     */
+    struct timeval tv;
+    if (!gettimeofday(&tv, NULL))
+    {
+        const unsigned int seed = (unsigned int) tv.tv_sec ^ tv.tv_usec;
+        srandom(seed);
+    }
+
     error_reset();              /* initialize error.c */
     reset_check_status();       /* initialize status check code in socket.c */
 
src/openvpn/misc.c +0 -19
index 8a76bba8..aff1bb2e 100644
@@ -405,25 +405,6 @@ openvpn_popen(const struct argv *a,  const struct env_set *es)
 
 
 /*
- * Initialize random number seed.  random() is only used
- * when "weak" random numbers are acceptable.
- * OpenSSL routines are always used when cryptographically
- * strong random numbers are required.
- */
-
-void
-init_random_seed(void)
-{
-    struct timeval tv;
-
-    if (!gettimeofday(&tv, NULL))
-    {
-        const unsigned int seed = (unsigned int) tv.tv_sec ^ tv.tv_usec;
-        srandom(seed);
-    }
-}
-
-/*
  * Set environmental variable (int or string).
  *
  * On Posix, we use putenv for portability,
src/openvpn/misc.h +0 -3
index 734e679c..a7aa7622 100644
@@ -100,9 +100,6 @@ void set_std_files_to_null(bool stdin_only);
 extern int inetd_socket_descriptor;
 void save_inetd_socket_descriptor(void);
 
-/* init random() function, only used as source for weak random numbers, when !ENABLE_CRYPTO */
-void init_random_seed(void);
-
 /* set/delete environmental variable */
 void setenv_str_ex(struct env_set *es,
                    const char *name,
Signature · 1 line
2.11.0
#1 lore raw

Hi,

Quoted 88 lines · David Sommerseth

The init_random_seed() function is only used by the init_static() in init.c. As this function was pretty basic and it is only being called once, it was merged into init_static() instead of keeping it as a separate function.

(I agree that calling functions often makes the code more readable, but I would rather see that as a part of cleaning up the whole init_static() function - in fact when moving all "unit tests" in init_static() to cmocka, it will not be too bad in the end.)

---
 src/openvpn/init.c | 17 +++++++++++++++--
 src/openvpn/misc.c | 19 -------------------
 src/openvpn/misc.h |  3 ---
 3 files changed, 15 insertions(+), 24 deletions(-)
src/openvpn/init.c +15 -2
index bc3b81e3..860df774 100644
@@ -610,6 +610,7 @@ init_port_share(struct context *c)
 
 #endif /* if PORT_SHARE */
 
+
 bool
 init_static(void)
 {
@@ -619,8 +620,20 @@ init_static(void)
     crypto_init_dmalloc();
 #endif
 
-    init_random_seed();         /* init random() function, only used as
-                                 * source for weak random numbers */
+
+    /*
+     * Initialize random number seed.  random() is only used
+     * when "weak" random numbers are acceptable.
+     * SSL library routines are always used when cryptographically
+     * strong random numbers are required.
+     */
+    struct timeval tv;
+    if (!gettimeofday(&tv, NULL))
+    {
+        const unsigned int seed = (unsigned int) tv.tv_sec ^ tv.tv_usec;
+        srandom(seed);
+    }
+
     error_reset();              /* initialize error.c */
     reset_check_status();       /* initialize status check code in socket.c */
 
src/openvpn/misc.c +0 -19
index 8a76bba8..aff1bb2e 100644
@@ -405,25 +405,6 @@ openvpn_popen(const struct argv *a,  const struct env_set *es)
 
 
 /*
- * Initialize random number seed.  random() is only used
- * when "weak" random numbers are acceptable.
- * OpenSSL routines are always used when cryptographically
- * strong random numbers are required.
- */
-
-void
-init_random_seed(void)
-{
-    struct timeval tv;
-
-    if (!gettimeofday(&tv, NULL))
-    {
-        const unsigned int seed = (unsigned int) tv.tv_sec ^ tv.tv_usec;
-        srandom(seed);
-    }
-}
-
-/*
  * Set environmental variable (int or string).
  *
  * On Posix, we use putenv for portability,
src/openvpn/misc.h +0 -0
index 734e679c..a7aa7622 100644

src/openvpn/misc.h void set_std_files_to_null(bool stdin_only); · line 105

Quoted 9 lines
src/openvpn/misc.h +0 -3
@@ -100,9 +100,6 @@ void set_std_files_to_null(bool stdin_only);
 extern int inetd_socket_descriptor;
 void save_inetd_socket_descriptor(void);
 
-/* init random() function, only used as source for weak random numbers, when !ENABLE_CRYPTO */
-void init_random_seed(void);
-
 /* set/delete environmental variable */
 void setenv_str_ex(struct env_set *es,
                    const char *name,

I think I would have preferred to keep the function, but I can live with this approach. So, ACK.

-Steffan

#2 lore raw
In reply to David Sommerseth

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Your patch has been applied to the following branches

commit e74e3a4db891b3ace0a96461c597d86e87be06f0 (master) commit 4bd2f4ef5b765ede3df9ae77b16cbeed5b4f7963 (release/2.4)

Author: David Sommerseth
Date:   Tue Jul 25 17:07:23 2017 +0200
     cleanup: Move init_random_seed() to where it is being used

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15136.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCgAGBQJZjg+kAAoJEIbPlEyWcf3y7coQAL8NLyKrUpaALtliRQoPZTah TcwBSn3jMLGsXirGZhkMTBz3hEHwkLcco9qRMWOpL7sZ7PyzOWQHorxQ2Ht3QSvo YCTxjnULVdEEe14/IsoG4bXgg40kNon78klrN1iAFoeqP98G6u7fqY2PzUUF0dZ9 lP7uKKDOmh6z42Ss9GClouR7gfTC088jPHeR2G4Nz6dczBsOpzUmYs/CA6F2O4VU eiMajDXm7bMXwasf+CVolBFFM4JlDct4D0PyDfzNVhQuIaiFjdU+1BUQ3XA49jta WB8bzRDNUmXrhudxloombZqcN/fSIYzIQlG0mKhXLxl/p+pm/1vyHfHOhfjYLUgv 7MiTYjSLB7c+41T7tStiueuqmTpyjo3Y0v1qCjJcn2c9p4QVwwXHlMi3EmOTPKfj AgbUXZIRLxm58nHXgeCahEkq3jDiGOltAJ1Q9qSj0Kf9QrJ5H+IHsG8iEuv574Mg WA0TRcqBnF8nhpHcfXW/bg4p0OF8SPTrW7hljhTKDJOyvuWXDe1zaQsHwfvnSn77 rjGFG6Ynu0S3ymKtClo1bLDdZH5eufjbvryKa0ShvqOh3CYy31Sc0xMHyyy2U8y0 XN+vQDzNO6FjoC4B2DpD4NoYU+hLJavFpfqpKUmW78um4O7W5X2GKUF0bgIgE8iB qwY/WpGhLM8aa9bzR0GR
=T/NO
-----END PGP SIGNATURE-----