[PATCH v8] packet_id: Avoid conversion warnings

Gert Doering <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <[email protected]>
From: Frank Lichtenheld <[email protected]>

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]>
Acked-by: Razvan Cojocaru <[email protected]>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1407
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1407
This mail reflects revision 8 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <[email protected]>

        
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.