[Openvpn-devel] [M] Change in openvpn[master]: packet_id: Avoid conversion warnings

Attention is currently required from: plaisthos.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#8).

The following approvals got outdated and were removed:
Code-Review-1 by plaisthos


Change subject: packet_id: Avoid conversion warnings
......................................................................

packet_id: Avoid conversion warnings

Most of these already have checks that the
values are as expected, so we can just add
the required casts.

Only the circ list x_sizeof is changed to
size_t since that is its purpose (and it is
not actually used anyway).

Change-Id: Ib0584e8728701cca10ac5675c9cb0e6f5eb901ac
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/circ_list.h
M src/openvpn/packet_id.c
M src/openvpn/packet_id.h
3 files changed, 29 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/07/1407/8

diff --git a/src/openvpn/circ_list.h b/src/openvpn/circ_list.h
index c29b0bb8..76484ca 100644
--- a/src/openvpn/circ_list.h
+++ b/src/openvpn/circ_list.h
@@ -33,7 +33,7 @@
         int x_head;           \
         int x_size;           \
         int x_cap;            \
-        int x_sizeof;         \
+        size_t x_sizeof;      \
         type x_list[];        \
     }
 
@@ -58,14 +58,14 @@
         (obj)->x_size = 0;   \
     }
 
-#define CIRC_LIST_ALLOC(dest, list_type, size)                                 \
-    {                                                                          \
-        const int so = sizeof(list_type) + sizeof((dest)->x_list[0]) * (size); \
-        (dest) = (list_type *)malloc(so);                                      \
-        check_malloc_return(dest);                                             \
-        memset((dest), 0, so);                                                 \
-        (dest)->x_cap = size;                                                  \
-        (dest)->x_sizeof = so;                                                 \
+#define CIRC_LIST_ALLOC(dest, list_type, size)                                    \
+    {                                                                             \
+        const size_t so = sizeof(list_type) + sizeof((dest)->x_list[0]) * (size); \
+        (dest) = (list_type *)malloc(so);                                         \
+        check_malloc_return(dest);                                                \
+        memset((dest), 0, so);                                                    \
+        (dest)->x_cap = size;                                                     \
+        (dest)->x_sizeof = so;                                                    \
     }
 
 #define CIRC_LIST_FREE(dest) free(dest)
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index a4b627c..b0211d7 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -71,11 +71,6 @@
 #endif
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 static void
 packet_id_init_recv(struct packet_id_rec *rec, int seq_backtrack, int time_backtrack,
                     const char *name, int unit)
@@ -119,7 +114,7 @@
 
     /* Reinitalise the source */
     CLEAR(*src);
-    packet_id_init_recv(src, dest->seq_backtrack, dest->time_backtrack, dest->name, dest->unit);
+    packet_id_init_recv(src, (int)dest->seq_backtrack, dest->time_backtrack, dest->name, dest->unit);
 }
 
 void
@@ -170,7 +165,7 @@
         diff = p->id - pin->id;
         if (diff < CIRC_LIST_SIZE(p->seq_list) && local_now > SEQ_EXPIRED)
         {
-            CIRC_LIST_ITEM(p->seq_list, diff) = local_now;
+            CIRC_LIST_ITEM(p->seq_list, (int)diff) = local_now;
         }
     }
     else
@@ -263,18 +258,16 @@
                 return false;
             }
 
+            const time_t v = CIRC_LIST_ITEM(p->seq_list, (int)diff);
+            if (v == 0)
             {
-                const time_t v = CIRC_LIST_ITEM(p->seq_list, diff);
-                if (v == 0)
-                {
-                    return true;
-                }
-                else
-                {
-                    /* raised from D_PID_DEBUG_LOW to reduce verbosity */
-                    packet_id_debug(D_PID_DEBUG_MEDIUM, p, pin, "PID_ERR replay", diff);
-                    return false;
-                }
+                return true;
+            }
+            else
+            {
+                /* raised from D_PID_DEBUG_LOW to reduce verbosity */
+                packet_id_debug(D_PID_DEBUG_MEDIUM, p, pin, "PID_ERR replay", diff);
+                return false;
             }
         }
         else if (pin->time < p->time) /* if time goes back, reject */
@@ -386,7 +379,7 @@
         return false;
     }
 
-    const packet_id_type net_id = htonpid(p->id);
+    const packet_id_type net_id = htonpid((packet_id_type)p->id);
     const net_time_t net_time = htontime(p->time);
     if (prepend)
     {
@@ -640,7 +633,7 @@
                p->seq_backtrack, p->time_backtrack, p->max_backtrack_stat, (int)p->initialized);
     if (sl != NULL)
     {
-        buf_printf(&out, " sl=[%d,%d,%d,%d]", sl->x_head, sl->x_size, sl->x_cap, sl->x_sizeof);
+        buf_printf(&out, " sl=[%d,%d,%d,%zu]", sl->x_head, sl->x_size, sl->x_cap, sl->x_sizeof);
     }
 
 
@@ -655,7 +648,6 @@
 {
     uint64_t packet_id;
 
-
     if (!buf_read(buf, &packet_id, sizeof(packet_id)))
     {
         return 0;
@@ -663,16 +655,12 @@
 
     uint64_t id = ntohll(packet_id);
     /* top most 16 bits */
-    uint16_t epoch = id >> 48;
+    uint16_t epoch = (uint16_t)(id >> 48);
 
     pin->id = id & PACKET_ID_MASK;
     return epoch;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 bool
 packet_id_write_epoch(struct packet_id_send *p, uint16_t epoch, struct buffer *buf)
 {
diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h
index ba59067..48752d3 100644
--- a/src/openvpn/packet_id.h
+++ b/src/openvpn/packet_id.h
@@ -280,26 +280,20 @@
     return p->fd >= 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
-/* transfer packet_id -> packet_id_persist */
+/* transfer packet_id -> packet_id_persist
+   Note: Only supported for non-epoch format (i.e. 32bit ID, not 64bit format),
+   must be enforced at the caller.
+ */
 static inline void
 packet_id_persist_save_obj(struct packet_id_persist *p, const struct packet_id *pid)
 {
-    if (packet_id_persist_enabled(p) && pid->rec.time)
+    if (packet_id_persist_enabled(p) && pid->rec.time && pid->rec.id <= PACKET_ID_MAX)
     {
         p->time = pid->rec.time;
-        p->id = pid->rec.id;
+        p->id = (packet_id_type)pid->rec.id;
     }
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 /**
  * Reset the current send packet id to its initial state.
  * Use very carefully (e.g. in the standalone reset packet context) to

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ib0584e8728701cca10ac5675c9cb0e6f5eb901ac
Gerrit-Change-Number: 1407
Gerrit-PatchSet: 8
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel