[Openvpn-devel,v8] packet_id: Avoid conversion warnings

Message ID 20260818142226.27172-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v8] packet_id: Avoid conversion warnings |

Commit Message

Gert Doering Aug. 18, 2026, 2:22 p.m. UTC
  From: Frank Lichtenheld <frank@lichtenheld.com>

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 <frank@lichtenheld.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
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 <razvanc@mailbox.org>
  

Patch

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