Change type of max_clients to uint32_t

peer_id is mostly this already (except in DCO
context for some reason), and max_peerid was
defined as uint32_t as well. So changing max_clients
to uint32_t avoids many -Wsign-compare warnings.

While here fix limit for max_clients in options
parsing. It is not allowed to be MAX_PEER_ID
exactly.

Change-Id: I8d6b7bc1b7744dc6d57aaed3231b8901275752f2
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1564
Message-Id: <20260407112434.5588-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36535.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index a1e373d90e01e387ac2f627bf7cb6646b00e0088..718cd8b2e8afaa857c742f97926e862ad5b31473 100644 (file)
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -559,11 +559,6 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid, int keepalive_interval, in
     return ret;
 }
 
     return ret;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-

 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *nvl)
 {
 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *nvl)
 {


@@ -582,10 +577,6 @@ dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *n
         __func__, peerid, mi->context.c2.dco_read_bytes, mi->context.c2.dco_write_bytes);
 }
 
         __func__, peerid, mi->context.c2.dco_read_bytes, mi->context.c2.dco_write_bytes);
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-

 int
 dco_read_and_process(dco_context_t *dco)
 {
 int
 dco_read_and_process(dco_context_t *dco)
 {

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 4c896f6abeff25fa32308dfa10c248a110b5e060..40746bdcb4b121fd0b7f4f0e2a24297568ebd7c0 100644 (file)
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -859,11 +859,6 @@ dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id)
     }
 }
 
     }
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-

 static int
 ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
 {
 static int
 ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
 {


@@ -890,7 +885,7 @@ ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
     if (dco->ifmode == OVPN_MODE_P2P)
     {
         c2 = &dco->c->c2;
     if (dco->ifmode == OVPN_MODE_P2P)
     {
         c2 = &dco->c->c2;

-        if (c2->tls_multi->dco_peer_id != peer_id)
+        if (c2->tls_multi->dco_peer_id != (int)peer_id)

         {
             return NL_SKIP;
         }
         {
             return NL_SKIP;
         }


@@ -919,10 +914,6 @@ ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
     return NL_OK;
 }
 
     return NL_OK;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-

 static bool
 ovpn_iface_check(dco_context_t *dco, struct nlattr *attrs[])
 {
 static bool
 ovpn_iface_check(dco_context_t *dco, struct nlattr *attrs[])
 {

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index f46c24d5afcd47ec7664bbc8f60697db96477d81..2e26f2a917719fd4c30d7ee9099359efc5fbdd70 100644 (file)
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -739,11 +739,6 @@ dco_read_and_process(dco_context_t *dco)
     return 0;
 }
 
     return 0;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-

 int
 dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err)
 {
 int
 dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err)
 {


@@ -838,9 +833,9 @@ dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err)
     {
         OVPN_PEER_STATS *stat = &peer_stats[i];
 
     {
         OVPN_PEER_STATS *stat = &peer_stats[i];
 

-        if (stat->PeerId >= dco->c->multi->max_clients)
+        if (stat->PeerId >= (int)dco->c->multi->max_clients)

         {
         {

-            msg(M_WARN, "%s: received out of bound peer_id %u (max=%u)", __func__, stat->PeerId,
+            msg(M_WARN, "%s: received out of bound peer_id %d (max=%u)", __func__, stat->PeerId,

                 dco->c->multi->max_clients);
             continue;
         }
                 dco->c->multi->max_clients);
             continue;
         }


@@ -871,10 +866,6 @@ done:
     return ret;
 }
 
     return ret;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-

 int
 dco_get_peer_stats_fallback(struct context *c, const bool raise_sigusr1_on_err)
 {
 int
 dco_get_peer_stats_fallback(struct context *c, const bool raise_sigusr1_on_err)
 {

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index b35975088010b9e8ee452d459e79158b80b5dbf0..432c79a51592d8b4491b09048efce423fdb44ec2 100644 (file)
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -180,11 +180,6 @@ do_pre_decrypt_check(struct multi_context *m, struct tls_pre_decrypt_state *stat
     return false;
 }
 
     return false;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-

 /*
  * Get a client instance based on real address.  If
  * the instance doesn't exist, create it while
 /*
  * Get a client instance based on real address.  If
  * the instance doesn't exist, create it while


@@ -217,7 +212,7 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated, struct lin
             uint32_t peer_id = ((uint32_t)ptr[1] << 16) | ((uint32_t)ptr[2] << 8) | ((uint32_t)ptr[3]);
             peer_id_disabled = (peer_id == MAX_PEER_ID);
 
             uint32_t peer_id = ((uint32_t)ptr[1] << 16) | ((uint32_t)ptr[2] << 8) | ((uint32_t)ptr[3]);
             peer_id_disabled = (peer_id == MAX_PEER_ID);
 

-            if (!peer_id_disabled && (peer_id < m->max_clients) && (m->instances[peer_id]))
+            if (!peer_id_disabled && (peer_id < m->max_clients) && m->instances[peer_id])

             {
                 /* Floating on TCP will never be possible, so ensure we only process
                  * UDP clients */
             {
                 /* Floating on TCP will never be possible, so ensure we only process
                  * UDP clients */


@@ -315,10 +310,6 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated, struct lin
     return mi;
 }
 
     return mi;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-

 /*
  * Send a packet to UDP socket.
  */
 /*
  * Send a packet to UDP socket.
  */

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index c7a91abe52870eb43ae9d90dac52d19541106ee8..fe210ee9bd8b284747f77c39621e98198e557ae3 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -695,11 +695,6 @@ multi_uninit(struct multi_context *m)
     }
 }
 
     }
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-

 /*
  * Create a client instance object for a newly connected client.
  */
 /*
  * Create a client instance object for a newly connected client.
  */


@@ -782,10 +777,6 @@ err:
     return NULL;
 }
 
     return NULL;
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-

 /*
  * Dump tables -- triggered by SIGUSR2.
  * If status file is defined, write to file.
 /*
  * Dump tables -- triggered by SIGUSR2.
  * If status file is defined, write to file.


@@ -3261,7 +3252,7 @@ multi_process_incoming_dco(dco_context_t *dco)
         return;
     }
 
         return;
     }
 

-    if ((peer_id < m->max_clients) && (m->instances[peer_id]))
+    if (((uint32_t)peer_id < m->max_clients) && m->instances[peer_id])

     {
         struct multi_instance *mi = m->instances[peer_id];
         set_prefix(mi);
     {
         struct multi_instance *mi = m->instances[peer_id];
         set_prefix(mi);


@@ -4085,18 +4076,13 @@ init_management_callback_multi(struct multi_context *m)
 #endif /* ifdef ENABLE_MANAGEMENT */
 }
 
 #endif /* ifdef ENABLE_MANAGEMENT */
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-

 void
 multi_assign_peer_id(struct multi_context *m, struct multi_instance *mi)
 {
     /* max_clients must be less then max peer-id value */
     ASSERT(m->max_clients < MAX_PEER_ID);
 
 void
 multi_assign_peer_id(struct multi_context *m, struct multi_instance *mi)
 {
     /* max_clients must be less then max peer-id value */
     ASSERT(m->max_clients < MAX_PEER_ID);
 

-    for (int i = 0; i < m->max_clients; ++i)
+    for (uint32_t i = 0; i < m->max_clients; ++i)

     {
         if (!m->instances[i])
         {
     {
         if (!m->instances[i])
         {


@@ -4117,10 +4103,6 @@ multi_assign_peer_id(struct multi_context *m, struct multi_instance *mi)
     }
 }
 
     }
 }
 

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-

 /**
  * @brief Determines the earliest wakeup interval based on periodic operations.
  *
 /**
  * @brief Determines the earliest wakeup interval based on periodic operations.
  *

diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 935bda15d8d87ffae4f02d3568d1c91dbd12d6f3..3ed08d4ff123ff8249582f0bfffb153f887290a6 100644 (file)
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -182,7 +182,7 @@ struct multi_context
     struct multi_reap *reaper;
     struct mroute_addr local;
     bool enable_c2c;
     struct multi_reap *reaper;
     struct mroute_addr local;
     bool enable_c2c;

-    int max_clients;
+    uint32_t max_clients;

     int tcp_queue_limit;
     int status_file_version;
     int n_clients; /* current number of authenticated clients */
     int tcp_queue_limit;
     int status_file_version;
     int n_clients; /* current number of authenticated clients */

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b8d0f5aa6917f3bc3ddbb52554bd81a18ef63f4d..70af6d2b45281c224a2aae11a9d495c10806459b 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1428,7 +1428,7 @@ show_p2mp_parms(const struct options *o)
     SHOW_INT(cf_per);
     SHOW_INT(cf_initial_max);
     SHOW_INT(cf_initial_per);
     SHOW_INT(cf_per);
     SHOW_INT(cf_initial_max);
     SHOW_INT(cf_initial_per);

-    SHOW_INT(max_clients);
+    SHOW_UINT(max_clients);

     SHOW_INT(max_routes_per_client);
     SHOW_STR(auth_user_pass_verify_script);
     SHOW_BOOL(auth_user_pass_verify_script_via_file);
     SHOW_INT(max_routes_per_client);
     SHOW_STR(auth_user_pass_verify_script);
     SHOW_BOOL(auth_user_pass_verify_script_via_file);


@@ -7378,7 +7378,7 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
     else if (streq(p[0], "max-clients") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
     else if (streq(p[0], "max-clients") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);

-        if (!atoi_constrained(p[1], &options->max_clients, p[0], 1, MAX_PEER_ID, msglevel))
+        if (!atoi_constrained(p[1], (int *)&options->max_clients, p[0], 1, MAX_PEER_ID - 1, msglevel))

         {
             goto err;
         }
         {
             goto err;
         }

diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 422820cf82a225bb40d9127abbb3fde08d815296..a111cf8bf3b8a5562aa243cfecfc4b109ce0c007 100644 (file)
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -530,7 +530,7 @@ struct options
     int cf_initial_max;
     int cf_initial_per;
 
     int cf_initial_max;
     int cf_initial_per;
 

-    int max_clients;
+    uint32_t max_clients;

     int max_routes_per_client;
     int stale_routes_check_interval;
     int stale_routes_ageing_time;
     int max_routes_per_client;
     int stale_routes_check_interval;
     int stale_routes_ageing_time;