[Openvpn-devel] [S] Change in openvpn[master]: init: Fix conversion warnings

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1259?usp=email

to review the following change.


Change subject: init: Fix conversion warnings
......................................................................

init: Fix conversion warnings

Can be separated in mostly two classes of issues:
 - Return type of get_random is long, but we ensure that value
   is in range so we can cast it to whatever we need.
 - Doing MTU match in size_t even though most of the input
   values are int.

Change-Id: I41b9f7686180f2549bd4984cbbf66059f0ba2b15
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/init.c
1 file changed, 14 insertions(+), 26 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/59/1259/1

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index f8a0fee..81b9ca9 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -455,11 +455,6 @@
 }
 #endif /* ENABLE_MANAGEMENT */
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 /*
  * Initialize and possibly randomize the connection list.
  *
@@ -482,7 +477,7 @@
         int i;
         for (i = l->len - 1; i > 0; --i)
         {
-            const int j = get_random() % (i + 1);
+            const long j = get_random() % (i + 1);
             if (i != j)
             {
                 struct connection_entry *tmp;
@@ -842,7 +837,7 @@
     struct timeval tv;
     if (!gettimeofday(&tv, NULL))
     {
-        const unsigned int seed = (unsigned int)tv.tv_sec ^ tv.tv_usec;
+        const unsigned int seed = (unsigned int)(tv.tv_sec ^ tv.tv_usec);
         srandom(seed);
     }
 
@@ -2873,17 +2868,17 @@
     }
 }
 
-static size_t
+static int
 get_frame_mtu(struct context *c, const struct options *o)
 {
-    size_t mtu;
+    int mtu;
 
     if (o->ce.link_mtu_defined)
     {
         ASSERT(o->ce.link_mtu_defined);
         /* if we have a link mtu defined we calculate what the old code
          * would have come up with as tun-mtu */
-        size_t overhead = frame_calculate_protocol_header_size(&c->c1.ks.key_type, o, true);
+        int overhead = (int)frame_calculate_protocol_header_size(&c->c1.ks.key_type, o, true);
         mtu = o->ce.link_mtu - overhead;
     }
     else
@@ -2894,7 +2889,7 @@
 
     if (mtu < TUN_MTU_MIN)
     {
-        msg(M_WARN, "TUN MTU value (%zu) must be at least %d", mtu, TUN_MTU_MIN);
+        msg(M_WARN, "TUN MTU value (%d) must be at least %d", mtu, TUN_MTU_MIN);
         frame_print(&c->c2.frame, M_FATAL, "MTU is too small");
     }
     return mtu;
@@ -2923,7 +2918,7 @@
      * space to allow server to push "baby giant" MTU sizes */
     frame->tun_max_mtu = max_int(TUN_MTU_MAX_MIN, frame->tun_max_mtu);
 
-    size_t payload_size = frame->tun_max_mtu;
+    int payload_size = frame->tun_max_mtu;
 
     /* we need to be also large enough to hold larger control channel packets
      * if configured */
@@ -2942,7 +2937,7 @@
 
     /* the space that is reserved before the payload to add extra headers to it
      * we always reserve the space for the worst case */
-    size_t headroom = 0;
+    int headroom = 0;
 
     /* includes IV and packet ID */
     headroom += crypto_max_overhead();
@@ -2966,11 +2961,11 @@
     /* the space after the payload, this needs some extra buffer space for
      * encryption so headroom is probably too much but we do not really care
      * the few extra bytes */
-    size_t tailroom = headroom;
+    int tailroom = headroom;
 
 #ifdef USE_COMP
     msg(D_MTU_DEBUG,
-        "MTU: adding %zu buffer tailroom for compression for %zu "
+        "MTU: adding %d buffer tailroom for compression for %d "
         "bytes of payload",
         COMP_EXTRA_BUFFER(payload_size), payload_size);
     tailroom += COMP_EXTRA_BUFFER(payload_size);
@@ -3289,18 +3284,15 @@
     if (options->renegotiate_seconds_min < 0)
     {
         /* Add 10% jitter to reneg-sec by default (server side only) */
-        int auto_jitter = options->mode != MODE_SERVER
-                              ? 0
-                              : get_random() % max_int(options->renegotiate_seconds / 10, 1);
+        int jitter_max = max_int(options->renegotiate_seconds / 10, 1);
+        int auto_jitter = options->mode != MODE_SERVER ? 0 : (int)(get_random() % jitter_max);
         to.renegotiate_seconds = options->renegotiate_seconds - auto_jitter;
     }
     else
     {
         /* Add user-specified jitter to reneg-sec */
-        to.renegotiate_seconds =
-            options->renegotiate_seconds
-            - (get_random()
-               % max_int(options->renegotiate_seconds - options->renegotiate_seconds_min, 1));
+        int jitter_max = max_int(options->renegotiate_seconds - options->renegotiate_seconds_min, 1);
+        to.renegotiate_seconds = options->renegotiate_seconds - (int)(get_random() % jitter_max);
     }
     to.single_session = options->single_session;
     to.mode = options->mode;
@@ -3495,10 +3487,6 @@
     }
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 /*
  * No encryption or authentication.
  */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1259?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I41b9f7686180f2549bd4984cbbf66059f0ba2b15
Gerrit-Change-Number: 1259
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>