[Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors

All of lore.kernel.org
 help / color / mirror / Atom feed

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
@ 2023-09-07  8:45 ` flichtenheld (Code Review)
  2023-09-07  8:45 ` flichtenheld (Code Review)
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: flichtenheld (Code Review) @ 2023-09-07  8:45 UTC (permalink / raw)
  Cc: plaisthos <arne-openvpn@

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/267?usp=email )

Change subject: Various fixes for -Wconversion errors
......................................................................


Patch Set 3:

(1 comment)

Patchset:

PS3:
Blocks applying changes to openvpn3



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Comment-Date: Thu, 07 Sep 2023 08:45:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2058 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
  2023-09-07  8:45 ` [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors flichtenheld (Code Review)
@ 2023-09-07  8:45 ` flichtenheld (Code Review)
  2023-11-20 10:30 ` plaisthos (Code Review)
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: flichtenheld (Code Review) @ 2023-09-07  8:45 UTC (permalink / raw)
  To: plaisthos <arne-openvpn@; +Cc: openvpn-devel

[-- Attachment #1: Type: text/plain, Size: 13558 bytes --]

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/+/267?usp=email

to review the following change.


Change subject: Various fixes for -Wconversion errors
......................................................................

Various fixes for -Wconversion errors

These are all fixes I considered "safe". They either

- Have sufficient checks/shifts for a cast to be safe
- Fix the type of a variable without requiring code changes
- Are in non-critical unittest code

v2:
 - add min_size instead of abusing min_int

Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Signed-off-by: Frank Lichtenheld <frank@...2641...>
---
M src/openvpn/buffer.c
M src/openvpn/crypto.c
M src/openvpn/integer.h
M src/openvpn/mss.c
M src/openvpn/otime.c
M src/openvpn/otime.h
M src/openvpn/packet_id.c
M src/openvpn/reliable.c
M src/openvpn/socket.h
M src/openvpn/tls_crypt.c
M src/openvpn/xkey_helper.c
M tests/unit_tests/openvpn/mock_get_random.c
M tests/unit_tests/openvpn/test_crypto.c
M tests/unit_tests/openvpn/test_packet_id.c
M tests/unit_tests/openvpn/test_provider.c
M tests/unit_tests/openvpn/test_tls_crypt.c
16 files changed, 50 insertions(+), 36 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/67/267/3

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 24f1ef2..a32e7d2 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -352,7 +352,7 @@
         return false;
     }

-    const int size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -889,7 +889,7 @@
         {
             break;
         }
-        line[n++] = c;
+        line[n++] = (char)c;
     }
     while (c);

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index a77b5a1..3152060 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -26,6 +26,8 @@
 #include "config.h"
 #endif

+#include <inttypes.h>
+
 #include "syshead.h"

 #include "crypto.h"
@@ -1275,8 +1277,8 @@
                     hex_byte[hb_index++] = c;
                     if (hb_index == 2)
                     {
-                        unsigned int u;
-                        ASSERT(sscanf((const char *)hex_byte, "%x", &u) == 1);
+                        uint8_t u;
+                        ASSERT(sscanf((const char *)hex_byte, "%" SCNx8, &u) == 1);
                         *out++ = u;
                         hb_index = 0;
                         if (++count == keylen)
@@ -1538,13 +1540,13 @@
     ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH);

-    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    const uint8_t cipher_length = (uint8_t)cipher_kt_key_size(kt->cipher);
     if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }

-    uint8_t hmac_length = md_kt_size(kt->digest);
+    uint8_t hmac_length = (uint8_t)md_kt_size(kt->digest);

     if (!buf_write(buf, &hmac_length, 1))
     {
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index 215c159..30b9ecf 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -28,12 +28,12 @@

 #ifndef htonll
 #define htonll(x) ((1==htonl(1)) ? (x) : \
-                   ((uint64_t)htonl((x) & 0xFFFFFFFF) << 32) | htonl((x) >> 32))
+                   ((uint64_t)htonl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | htonl((uint32_t)((x) >> 32)))
 #endif

 #ifndef ntohll
 #define ntohll(x) ((1==ntohl(1)) ? (x) : \
-                   ((uint64_t)ntohl((x) & 0xFFFFFFFF) << 32) | ntohl((x) >> 32))
+                   ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif

 /*
@@ -65,6 +65,19 @@
     }
 }

+static inline size_t
+min_size(size_t x, size_t y)
+{
+    if (x < y)
+    {
+        return x;
+    }
+    else
+    {
+        return y;
+    }
+}
+
 static inline int
 max_int(int x, int y)
 {
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 816e65b..dbd3681 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -165,7 +165,7 @@
         return;
     }

-    for (olen = hlen - sizeof(struct openvpn_tcphdr),
+    for (olen = hlen - (int) sizeof(struct openvpn_tcphdr),
          opt = (uint8_t *)(tc + 1);
          olen > 1;
          olen -= optlen, opt += optlen)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index b28a90f..e751246 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -105,7 +105,7 @@
 /* format a time_t as ascii, or use current time if 0 */

 const char *
-time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc)
+time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc)
 {
     struct buffer out = alloc_buf_gc(64, gc);
     struct timeval tv;
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index c27be89..d795c3c 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -43,7 +43,7 @@
 bool frequency_limit_event_allowed(struct frequency_limit *f);

 /* format a time_t as ascii, or use current time if 0 */
-const char *time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc);
+const char *time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc);

 /* struct timeval functions */

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index ef83248..3d6f3ee 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -588,14 +588,14 @@
         }
         else
         {
-            diff = (int) prev_now - v;
+            diff = (int)(prev_now - v);
             if (diff < 0)
             {
                 c = 'N';
             }
             else if (diff < 10)
             {
-                c = '0' + diff;
+                c = (char)('0' + diff);
             }
             else
             {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index e7d4d5f..0e26f98 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -257,8 +257,7 @@
                    struct buffer *buf,
                    const struct session_id *sid, int max, bool prepend)
 {
-    int i, j;
-    uint8_t n;
+    int i, j, n;
     struct buffer sub;

     n = ack->len;
@@ -270,9 +269,9 @@
     copy_acks_to_mru(ack, ack_mru, n);

     /* Number of acks we can resend that still fit into the packet */
-    uint8_t total_acks = min_int(max, ack_mru->len);
+    uint8_t total_acks = (uint8_t)min_int(max, ack_mru->len);

-    sub = buf_sub(buf, ACK_SIZE(total_acks), prepend);
+    sub = buf_sub(buf, (int)ACK_SIZE(total_acks), prepend);
     if (!BDEF(&sub))
     {
         goto error;
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index bfc1253..a4acc5d 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -1181,7 +1181,7 @@
 }

 /* write a TCP or UDP packet to link */
-static inline int
+static inline size_t
 link_socket_write(struct link_socket *sock,
                   struct buffer *buf,
                   struct link_socket_actual *to)
@@ -1198,7 +1198,7 @@
     else
     {
         ASSERT(0);
-        return -1; /* NOTREACHED */
+        return 0; /* NOTREACHED */
     }
 }

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 975d31f..f0946cb 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -634,7 +634,7 @@
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));

-    size_t wkc_len = ntohs(net_len);
+    uint16_t wkc_len = ntohs(net_len);
     if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len))
     {
         msg(D_TLS_ERRORS, "Can not locate tls-crypt-v2 client key");
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index 40839f2..fcf8c2e 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -292,7 +292,7 @@
  * @return              false on error, true  on success
  *
  * On return enc_len is  set to actual size of the result.
- * enc is NULL or enc_len is not enough to store the result, it is set
+ * If enc is NULL or enc_len is not enough to store the result, it is set
  * to the required size and false is returned.
  */
 bool
@@ -337,8 +337,8 @@
                         MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
                         MAKE_DI(sha512_256), {0, NULL, 0}};

-    int out_len = 0;
-    int ret = 0;
+    size_t out_len = 0;
+    bool ret = false;

     int nid = OBJ_sn2nid(mdname);
     if (nid == NID_undef)
@@ -354,7 +354,7 @@

     if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
-        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;
     }

@@ -383,13 +383,13 @@

     out_len = tbslen + di->sz;

-    if (enc && (out_len <= (int) *enc_len))
+    if (enc && (out_len <= *enc_len))
     {
         /* combine header and digest */
         memcpy(enc, di->header, di->sz);
         memcpy(enc + di->sz, tbs, tbslen);
-        dmsg(D_XKEY, "encode_pkcs1: digest length = %d encoded length = %d",
-             (int) tbslen, (int) out_len);
+        dmsg(D_XKEY, "encode_pkcs1: digest length = %zu encoded length = %zu",
+             tbslen, out_len);
         ret = true;
     }

diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index 787b5e3..dfc7287 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -41,6 +41,6 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = rand();
+        output[i] = (uint8_t)rand();
     }
 }
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 58eebc0..d765a4e 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -97,8 +97,8 @@

     for (int i = 0; i < strlen(ciphername); i++)
     {
-        upper[i] = toupper(ciphername[i]);
-        lower[i] = tolower(ciphername[i]);
+        upper[i] = (char)toupper((unsigned char)ciphername[i]);
+        lower[i] = (char)tolower((unsigned char)ciphername[i]);
         if (rand() & 0x1)
         {
             random_case[i] = upper[i];
@@ -160,7 +160,7 @@


     uint8_t out[32];
-    ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
+    ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));

     assert_memory_equal(good_prf, out, sizeof(out));
 }
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index 90c67ac..c351858 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -94,7 +94,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -121,7 +121,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -152,7 +152,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index 335fca2..5e57f33 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -365,7 +365,7 @@
     }

     /* return a predefined string as sig */
-    memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+    memcpy(sig, good_sig, min_size(sizeof(good_sig), *siglen));

     return 1;
 }
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index ed7c794..3b885ab 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -138,7 +138,7 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = i;
+        output[i] = (uint8_t)i;
     }
     return true;
 }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-MessageType: newchange

[-- Attachment #2: Type: text/html, Size: 23408 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
  2023-09-07  8:45 ` [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors flichtenheld (Code Review)
  2023-09-07  8:45 ` flichtenheld (Code Review)
@ 2023-11-20 10:30 ` plaisthos (Code Review)
  2023-11-21 10:10 ` [Openvpn-devel] [PATCH v5] " Frank Lichtenheld
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: plaisthos (Code Review) @ 2023-11-20 10:30 UTC (permalink / raw)
  To: flichtenheld <frank@; +Cc: openvpn-devel

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/267?usp=email )

Change subject: Various fixes for -Wconversion errors
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 4
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-Comment-Date: Mon, 20 Nov 2023 10:30:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 1823 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [PATCH v5] Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (2 preceding siblings ...)
  2023-11-20 10:30 ` plaisthos (Code Review)
@ 2023-11-21 10:10 ` Frank Lichtenheld
  2024-09-10 10:56 ` [Openvpn-devel] [M] Change in openvpn[master]: " cron2 (Code Review)
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Frank Lichtenheld @ 2023-11-21 10:10 UTC (permalink / raw)
  To: openvpn-devel; +Cc: Arne Schwabe <arne-openvpn@

These are all fixes I considered "safe". They either

- Have sufficient checks/shifts for a cast to be safe
- Fix the type of a variable without requiring code changes
- Are in non-critical unittest code

v2:
 - add min_size instead of abusing min_int

Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Signed-off-by: Frank Lichtenheld <frank@...2641...>
Acked-by: Arne Schwabe <arne-openvpn@...1227...>
---

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/+/267
This mail reflects revision 5 of this Change.
Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@...1227...>

        
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 24f1ef2..a32e7d2 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -352,7 +352,7 @@
         return false;
     }
 
-    const int size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -889,7 +889,7 @@
         {
             break;
         }
-        line[n++] = c;
+        line[n++] = (char)c;
     }
     while (c);
 
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index e4452d7..7768b9e 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -26,6 +26,8 @@
 #include "config.h"
 #endif
 
+#include <inttypes.h>
+
 #include "syshead.h"
 
 #include "crypto.h"
@@ -1263,8 +1265,8 @@
                     hex_byte[hb_index++] = c;
                     if (hb_index == 2)
                     {
-                        unsigned int u;
-                        ASSERT(sscanf((const char *)hex_byte, "%x", &u) == 1);
+                        uint8_t u;
+                        ASSERT(sscanf((const char *)hex_byte, "%" SCNx8, &u) == 1);
                         *out++ = u;
                         hb_index = 0;
                         if (++count == keylen)
@@ -1526,13 +1528,13 @@
     ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH);
 
-    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    const uint8_t cipher_length = (uint8_t)cipher_kt_key_size(kt->cipher);
     if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }
 
-    uint8_t hmac_length = md_kt_size(kt->digest);
+    uint8_t hmac_length = (uint8_t)md_kt_size(kt->digest);
 
     if (!buf_write(buf, &hmac_length, 1))
     {
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index a012524..a0e421d 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -28,12 +28,12 @@
 
 #ifndef htonll
 #define htonll(x) ((1==htonl(1)) ? (x) : \
-                   ((uint64_t)htonl((x) & 0xFFFFFFFF) << 32) | htonl((x) >> 32))
+                   ((uint64_t)htonl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | htonl((uint32_t)((x) >> 32)))
 #endif
 
 #ifndef ntohll
 #define ntohll(x) ((1==ntohl(1)) ? (x) : \
-                   ((uint64_t)ntohl((x) & 0xFFFFFFFF) << 32) | ntohl((x) >> 32))
+                   ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif
 
 static inline int
@@ -72,6 +72,19 @@
     }
 }
 
+static inline size_t
+min_size(size_t x, size_t y)
+{
+    if (x < y)
+    {
+        return x;
+    }
+    else
+    {
+        return y;
+    }
+}
+
 static inline int
 max_int(int x, int y)
 {
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 1566c64..108b370 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -165,7 +165,7 @@
         return;
     }
 
-    for (olen = hlen - sizeof(struct openvpn_tcphdr),
+    for (olen = hlen - (int) sizeof(struct openvpn_tcphdr),
          opt = (uint8_t *)(tc + 1);
          olen > 1;
          olen -= optlen, opt += optlen)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index b28a90f..e751246 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -105,7 +105,7 @@
 /* format a time_t as ascii, or use current time if 0 */
 
 const char *
-time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc)
+time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc)
 {
     struct buffer out = alloc_buf_gc(64, gc);
     struct timeval tv;
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index c27be89..d795c3c 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -43,7 +43,7 @@
 bool frequency_limit_event_allowed(struct frequency_limit *f);
 
 /* format a time_t as ascii, or use current time if 0 */
-const char *time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc);
+const char *time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc);
 
 /* struct timeval functions */
 
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index ef83248..3d6f3ee 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -588,14 +588,14 @@
         }
         else
         {
-            diff = (int) prev_now - v;
+            diff = (int)(prev_now - v);
             if (diff < 0)
             {
                 c = 'N';
             }
             else if (diff < 10)
             {
-                c = '0' + diff;
+                c = (char)('0' + diff);
             }
             else
             {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index e7d4d5f..0e26f98 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -257,8 +257,7 @@
                    struct buffer *buf,
                    const struct session_id *sid, int max, bool prepend)
 {
-    int i, j;
-    uint8_t n;
+    int i, j, n;
     struct buffer sub;
 
     n = ack->len;
@@ -270,9 +269,9 @@
     copy_acks_to_mru(ack, ack_mru, n);
 
     /* Number of acks we can resend that still fit into the packet */
-    uint8_t total_acks = min_int(max, ack_mru->len);
+    uint8_t total_acks = (uint8_t)min_int(max, ack_mru->len);
 
-    sub = buf_sub(buf, ACK_SIZE(total_acks), prepend);
+    sub = buf_sub(buf, (int)ACK_SIZE(total_acks), prepend);
     if (!BDEF(&sub))
     {
         goto error;
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index bfc1253..a4acc5d 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -1181,7 +1181,7 @@
 }
 
 /* write a TCP or UDP packet to link */
-static inline int
+static inline size_t
 link_socket_write(struct link_socket *sock,
                   struct buffer *buf,
                   struct link_socket_actual *to)
@@ -1198,7 +1198,7 @@
     else
     {
         ASSERT(0);
-        return -1; /* NOTREACHED */
+        return 0; /* NOTREACHED */
     }
 }
 
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 975d31f..f0946cb 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -634,7 +634,7 @@
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));
 
-    size_t wkc_len = ntohs(net_len);
+    uint16_t wkc_len = ntohs(net_len);
     if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len))
     {
         msg(D_TLS_ERRORS, "Can not locate tls-crypt-v2 client key");
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index 40839f2..fcf8c2e 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -292,7 +292,7 @@
  * @return              false on error, true  on success
  *
  * On return enc_len is  set to actual size of the result.
- * enc is NULL or enc_len is not enough to store the result, it is set
+ * If enc is NULL or enc_len is not enough to store the result, it is set
  * to the required size and false is returned.
  */
 bool
@@ -337,8 +337,8 @@
                         MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
                         MAKE_DI(sha512_256), {0, NULL, 0}};
 
-    int out_len = 0;
-    int ret = 0;
+    size_t out_len = 0;
+    bool ret = false;
 
     int nid = OBJ_sn2nid(mdname);
     if (nid == NID_undef)
@@ -354,7 +354,7 @@
 
     if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
-        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;
     }
 
@@ -383,13 +383,13 @@
 
     out_len = tbslen + di->sz;
 
-    if (enc && (out_len <= (int) *enc_len))
+    if (enc && (out_len <= *enc_len))
     {
         /* combine header and digest */
         memcpy(enc, di->header, di->sz);
         memcpy(enc + di->sz, tbs, tbslen);
-        dmsg(D_XKEY, "encode_pkcs1: digest length = %d encoded length = %d",
-             (int) tbslen, (int) out_len);
+        dmsg(D_XKEY, "encode_pkcs1: digest length = %zu encoded length = %zu",
+             tbslen, out_len);
         ret = true;
     }
 
diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index 787b5e3..dfc7287 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -41,6 +41,6 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = rand();
+        output[i] = (uint8_t)rand();
     }
 }
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 5564524..9cfa9dc 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -97,8 +97,8 @@
 
     for (int i = 0; i < strlen(ciphername); i++)
     {
-        upper[i] = toupper(ciphername[i]);
-        lower[i] = tolower(ciphername[i]);
+        upper[i] = (char)toupper((unsigned char)ciphername[i]);
+        lower[i] = (char)tolower((unsigned char)ciphername[i]);
         if (rand() & 0x1)
         {
             random_case[i] = upper[i];
@@ -160,7 +160,7 @@
 
 
     uint8_t out[32];
-    ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
+    ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));
 
     assert_memory_equal(good_prf, out, sizeof(out));
 }
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index 90c67ac..c351858 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -94,7 +94,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }
 
 static void
@@ -121,7 +121,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }
 
 static void
@@ -152,7 +152,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }
 
 static void
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index 335fca2..5e57f33 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -365,7 +365,7 @@
     }
 
     /* return a predefined string as sig */
-    memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+    memcpy(sig, good_sig, min_size(sizeof(good_sig), *siglen));
 
     return 1;
 }
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index ed7c794..3b885ab 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -138,7 +138,7 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = i;
+        output[i] = (uint8_t)i;
     }
     return true;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (3 preceding siblings ...)
  2023-11-21 10:10 ` [Openvpn-devel] [PATCH v5] " Frank Lichtenheld
@ 2024-09-10 10:56 ` cron2 (Code Review)
  2024-09-10 12:14 ` flichtenheld (Code Review)
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: cron2 (Code Review) @ 2024-09-10 10:56 UTC (permalink / raw)
  To: flichtenheld <frank@; +Cc: plaisthos <arne-openvpn@

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]

Attention is currently required from: flichtenheld.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/267?usp=email )

Change subject: Various fixes for -Wconversion errors
......................................................................


Patch Set 5:

(2 comments)

Patchset:

PS5:
I'll deal with it ASAP, but would ask you to look into link_socket_write*() as discussed on IRC.  So yes it will make the patch larger, but if it makes things better aligned...


File src/openvpn/socket.h:

http://gerrit.openvpn.net/c/openvpn/+/267/comment/2bb2272e_c41da5d0 :
PS5, Line 1184: static inline size_t
this looks funny. `ssize_t` and keep the `-1`?

(it's a whole call chain of `link_socket_write_*()` things, but somewhere underneath is a `write()` or `sendto()`, which both use `ssize_t`)



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 5
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: cron2 <gert@...1296...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-Comment-Date: Tue, 10 Sep 2024 10:56:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2972 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (4 preceding siblings ...)
  2024-09-10 10:56 ` [Openvpn-devel] [M] Change in openvpn[master]: " cron2 (Code Review)
@ 2024-09-10 12:14 ` flichtenheld (Code Review)
  2024-09-10 12:15 ` flichtenheld (Code Review)
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: flichtenheld (Code Review) @ 2024-09-10 12:14 UTC (permalink / raw)
  To: plaisthos <arne-openvpn@; +Cc: cron2 <gert@

[-- Attachment #1: Type: text/plain, Size: 13469 bytes --]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos,

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

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

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

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

The change is no longer submittable: Code-Review and checks~ChecksSubmitRule are unsatisfied now.


Change subject: Various fixes for -Wconversion errors
......................................................................

Various fixes for -Wconversion errors

These are all fixes I considered "safe". They either

- Have sufficient checks/shifts for a cast to be safe
- Fix the type of a variable without requiring code changes
- Are in non-critical unittest code

v2:
 - add min_size instead of abusing min_int
v6:
 - remove change of return value of link_socket_write.
   Move to separate patch.

Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Signed-off-by: Frank Lichtenheld <frank@...2641...>
---
M src/openvpn/buffer.c
M src/openvpn/crypto.c
M src/openvpn/integer.h
M src/openvpn/mss.c
M src/openvpn/otime.c
M src/openvpn/otime.h
M src/openvpn/packet_id.c
M src/openvpn/reliable.c
M src/openvpn/tls_crypt.c
M src/openvpn/xkey_helper.c
M tests/unit_tests/openvpn/mock_get_random.c
M tests/unit_tests/openvpn/test_crypto.c
M tests/unit_tests/openvpn/test_packet_id.c
M tests/unit_tests/openvpn/test_provider.c
M tests/unit_tests/openvpn/test_tls_crypt.c
15 files changed, 48 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/67/267/6

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index abe6a9c..9ee76aa 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -326,7 +326,7 @@
         return false;
     }

-    const int size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -863,7 +863,7 @@
         {
             break;
         }
-        line[n++] = c;
+        line[n++] = (char)c;
     }
     while (c);

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c226727..12ad0b9 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -26,6 +26,8 @@
 #include "config.h"
 #endif

+#include <inttypes.h>
+
 #include "syshead.h"
 #include <string.h>

@@ -1283,8 +1285,8 @@
                     hex_byte[hb_index++] = c;
                     if (hb_index == 2)
                     {
-                        unsigned int u;
-                        ASSERT(sscanf((const char *)hex_byte, "%x", &u) == 1);
+                        uint8_t u;
+                        ASSERT(sscanf((const char *)hex_byte, "%" SCNx8, &u) == 1);
                         *out++ = u;
                         hb_index = 0;
                         if (++count == keylen)
@@ -1546,13 +1548,13 @@
     ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH);

-    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    const uint8_t cipher_length = (uint8_t)cipher_kt_key_size(kt->cipher);
     if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }

-    uint8_t hmac_length = md_kt_size(kt->digest);
+    uint8_t hmac_length = (uint8_t)md_kt_size(kt->digest);

     if (!buf_write(buf, &hmac_length, 1))
     {
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index a1acaf9..34088ab 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -28,12 +28,12 @@

 #ifndef htonll
 #define htonll(x) ((1==htonl(1)) ? (x) : \
-                   ((uint64_t)htonl((x) & 0xFFFFFFFF) << 32) | htonl((x) >> 32))
+                   ((uint64_t)htonl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | htonl((uint32_t)((x) >> 32)))
 #endif

 #ifndef ntohll
 #define ntohll(x) ((1==ntohl(1)) ? (x) : \
-                   ((uint64_t)ntohl((x) & 0xFFFFFFFF) << 32) | ntohl((x) >> 32))
+                   ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif

 static inline int
@@ -72,6 +72,19 @@
     }
 }

+static inline size_t
+min_size(size_t x, size_t y)
+{
+    if (x < y)
+    {
+        return x;
+    }
+    else
+    {
+        return y;
+    }
+}
+
 static inline int
 max_int(int x, int y)
 {
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 635557c..ebdec25 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -165,7 +165,7 @@
         return;
     }

-    for (olen = hlen - sizeof(struct openvpn_tcphdr),
+    for (olen = hlen - (int) sizeof(struct openvpn_tcphdr),
          opt = (uint8_t *)(tc + 1);
          olen > 1;
          olen -= optlen, opt += optlen)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index 3cde574..d77c99e 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -105,7 +105,7 @@
 /* format a time_t as ascii, or use current time if 0 */

 const char *
-time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc)
+time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc)
 {
     struct buffer out = alloc_buf_gc(64, gc);
     struct timeval tv;
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index c37673e..9543732 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -43,7 +43,7 @@
 bool frequency_limit_event_allowed(struct frequency_limit *f);

 /* format a time_t as ascii, or use current time if 0 */
-const char *time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc);
+const char *time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc);

 /* struct timeval functions */

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index be28999..fb962e4 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -588,14 +588,14 @@
         }
         else
         {
-            diff = (int) prev_now - v;
+            diff = (int)(prev_now - v);
             if (diff < 0)
             {
                 c = 'N';
             }
             else if (diff < 10)
             {
-                c = '0' + diff;
+                c = (char)('0' + diff);
             }
             else
             {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index a789990..019ec18 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -257,8 +257,7 @@
                    struct buffer *buf,
                    const struct session_id *sid, int max, bool prepend)
 {
-    int i, j;
-    uint8_t n;
+    int i, j, n;
     struct buffer sub;

     n = ack->len;
@@ -270,9 +269,9 @@
     copy_acks_to_mru(ack, ack_mru, n);

     /* Number of acks we can resend that still fit into the packet */
-    uint8_t total_acks = min_int(max, ack_mru->len);
+    uint8_t total_acks = (uint8_t)min_int(max, ack_mru->len);

-    sub = buf_sub(buf, ACK_SIZE(total_acks), prepend);
+    sub = buf_sub(buf, (int)ACK_SIZE(total_acks), prepend);
     if (!BDEF(&sub))
     {
         goto error;
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 90fe6e9..b8894db 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -634,7 +634,7 @@
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));

-    size_t wkc_len = ntohs(net_len);
+    uint16_t wkc_len = ntohs(net_len);
     if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len))
     {
         msg(D_TLS_ERRORS, "Can not locate tls-crypt-v2 client key");
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index b68fb43..10cdc0b 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -292,7 +292,7 @@
  * @return              false on error, true  on success
  *
  * On return enc_len is  set to actual size of the result.
- * enc is NULL or enc_len is not enough to store the result, it is set
+ * If enc is NULL or enc_len is not enough to store the result, it is set
  * to the required size and false is returned.
  */
 bool
@@ -337,8 +337,8 @@
                         MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
                         MAKE_DI(sha512_256), {0, NULL, 0}};

-    int out_len = 0;
-    int ret = 0;
+    size_t out_len = 0;
+    bool ret = false;

     int nid = OBJ_sn2nid(mdname);
     if (nid == NID_undef)
@@ -354,7 +354,7 @@

     if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
-        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;
     }

@@ -383,13 +383,13 @@

     out_len = tbslen + di->sz;

-    if (enc && (out_len <= (int) *enc_len))
+    if (enc && (out_len <= *enc_len))
     {
         /* combine header and digest */
         memcpy(enc, di->header, di->sz);
         memcpy(enc + di->sz, tbs, tbslen);
-        dmsg(D_XKEY, "encode_pkcs1: digest length = %d encoded length = %d",
-             (int) tbslen, (int) out_len);
+        dmsg(D_XKEY, "encode_pkcs1: digest length = %zu encoded length = %zu",
+             tbslen, out_len);
         ret = true;
     }

diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index 787b5e3..dfc7287 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -41,6 +41,6 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = rand();
+        output[i] = (uint8_t)rand();
     }
 }
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 9d3ea1a..fdc8fbd 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -97,8 +97,8 @@

     for (int i = 0; i < strlen(ciphername); i++)
     {
-        upper[i] = toupper(ciphername[i]);
-        lower[i] = tolower(ciphername[i]);
+        upper[i] = (char)toupper((unsigned char)ciphername[i]);
+        lower[i] = (char)tolower((unsigned char)ciphername[i]);
         if (rand() & 0x1)
         {
             random_case[i] = upper[i];
@@ -155,7 +155,7 @@


     uint8_t out[32];
-    bool ret = ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
+    bool ret = ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));

 #if defined(LIBRESSL_VERSION_NUMBER) || defined(ENABLE_CRYPTO_WOLFSSL)
     /* No TLS1 PRF support in these libraries */
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index ff3f788..a3567bc 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -93,7 +93,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -120,7 +120,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -151,7 +151,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index cfe9ac3..b92412d 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -368,7 +368,7 @@
     }

     /* return a predefined string as sig */
-    memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+    memcpy(sig, good_sig, min_size(sizeof(good_sig), *siglen));

     return 1;
 }
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index a01fbe5..4f12f88 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -137,7 +137,7 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = i;
+        output[i] = (uint8_t)i;
     }
     return true;
 }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 6
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: cron2 <gert@...1296...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-MessageType: newpatchset

[-- Attachment #2: Type: text/html, Size: 22920 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (5 preceding siblings ...)
  2024-09-10 12:14 ` flichtenheld (Code Review)
@ 2024-09-10 12:15 ` flichtenheld (Code Review)
  2024-09-10 12:20 ` [Openvpn-devel] [PATCH v6] " Gert Doering
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: flichtenheld (Code Review) @ 2024-09-10 12:15 UTC (permalink / raw)
  Cc: cron2 <gert@

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

Attention is currently required from: cron2, plaisthos.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/267?usp=email )

Change subject: Various fixes for -Wconversion errors
......................................................................


Patch Set 6:

(1 comment)

File src/openvpn/socket.h:

http://gerrit.openvpn.net/c/openvpn/+/267/comment/1402c05e_4280072e :
PS5, Line 1184: static inline size_t
> this looks funny. `ssize_t` and keep the `-1`? […]
Moved to http://gerrit.openvpn.net/c/openvpn/+/740



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 6
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: cron2 <gert@...1296...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Attention: cron2 <gert@...1296...>
Gerrit-Comment-Date: Tue, 10 Sep 2024 12:15:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <gert@...1296...>
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2593 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [PATCH v6] Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (6 preceding siblings ...)
  2024-09-10 12:15 ` flichtenheld (Code Review)
@ 2024-09-10 12:20 ` Gert Doering
  2024-09-10 12:52   ` [Openvpn-devel] [PATCH applied] " Gert Doering
  2024-09-10 12:53 ` [Openvpn-devel] [M] Change in openvpn[master]: " cron2 (Code Review)
  2024-09-10 12:53 ` cron2 (Code Review)
  9 siblings, 1 reply; 11+ messages in thread
From: Gert Doering @ 2024-09-10 12:20 UTC (permalink / raw)
  To: openvpn-devel

From: Frank Lichtenheld <frank@...2641...>

These are all fixes I considered "safe". They either

- Have sufficient checks/shifts for a cast to be safe
- Fix the type of a variable without requiring code changes
- Are in non-critical unittest code

v2:
 - add min_size instead of abusing min_int
v6:
 - remove change of return value of link_socket_write.
   Move to separate patch.

Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Signed-off-by: Frank Lichtenheld <frank@...2641...>
---

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/+/267
This mail reflects revision 6 of this Change.

Acked-by according to Gerrit (reflected above):

        
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index abe6a9c..9ee76aa 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -326,7 +326,7 @@
         return false;
     }
 
-    const int size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -863,7 +863,7 @@
         {
             break;
         }
-        line[n++] = c;
+        line[n++] = (char)c;
     }
     while (c);
 
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c226727..12ad0b9 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -26,6 +26,8 @@
 #include "config.h"
 #endif
 
+#include <inttypes.h>
+
 #include "syshead.h"
 #include <string.h>
 
@@ -1283,8 +1285,8 @@
                     hex_byte[hb_index++] = c;
                     if (hb_index == 2)
                     {
-                        unsigned int u;
-                        ASSERT(sscanf((const char *)hex_byte, "%x", &u) == 1);
+                        uint8_t u;
+                        ASSERT(sscanf((const char *)hex_byte, "%" SCNx8, &u) == 1);
                         *out++ = u;
                         hb_index = 0;
                         if (++count == keylen)
@@ -1546,13 +1548,13 @@
     ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH);
 
-    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    const uint8_t cipher_length = (uint8_t)cipher_kt_key_size(kt->cipher);
     if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }
 
-    uint8_t hmac_length = md_kt_size(kt->digest);
+    uint8_t hmac_length = (uint8_t)md_kt_size(kt->digest);
 
     if (!buf_write(buf, &hmac_length, 1))
     {
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index a1acaf9..34088ab 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -28,12 +28,12 @@
 
 #ifndef htonll
 #define htonll(x) ((1==htonl(1)) ? (x) : \
-                   ((uint64_t)htonl((x) & 0xFFFFFFFF) << 32) | htonl((x) >> 32))
+                   ((uint64_t)htonl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | htonl((uint32_t)((x) >> 32)))
 #endif
 
 #ifndef ntohll
 #define ntohll(x) ((1==ntohl(1)) ? (x) : \
-                   ((uint64_t)ntohl((x) & 0xFFFFFFFF) << 32) | ntohl((x) >> 32))
+                   ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif
 
 static inline int
@@ -72,6 +72,19 @@
     }
 }
 
+static inline size_t
+min_size(size_t x, size_t y)
+{
+    if (x < y)
+    {
+        return x;
+    }
+    else
+    {
+        return y;
+    }
+}
+
 static inline int
 max_int(int x, int y)
 {
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 635557c..ebdec25 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -165,7 +165,7 @@
         return;
     }
 
-    for (olen = hlen - sizeof(struct openvpn_tcphdr),
+    for (olen = hlen - (int) sizeof(struct openvpn_tcphdr),
          opt = (uint8_t *)(tc + 1);
          olen > 1;
          olen -= optlen, opt += optlen)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index 3cde574..d77c99e 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -105,7 +105,7 @@
 /* format a time_t as ascii, or use current time if 0 */
 
 const char *
-time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc)
+time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc)
 {
     struct buffer out = alloc_buf_gc(64, gc);
     struct timeval tv;
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index c37673e..9543732 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -43,7 +43,7 @@
 bool frequency_limit_event_allowed(struct frequency_limit *f);
 
 /* format a time_t as ascii, or use current time if 0 */
-const char *time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc);
+const char *time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc);
 
 /* struct timeval functions */
 
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index be28999..fb962e4 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -588,14 +588,14 @@
         }
         else
         {
-            diff = (int) prev_now - v;
+            diff = (int)(prev_now - v);
             if (diff < 0)
             {
                 c = 'N';
             }
             else if (diff < 10)
             {
-                c = '0' + diff;
+                c = (char)('0' + diff);
             }
             else
             {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index a789990..019ec18 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -257,8 +257,7 @@
                    struct buffer *buf,
                    const struct session_id *sid, int max, bool prepend)
 {
-    int i, j;
-    uint8_t n;
+    int i, j, n;
     struct buffer sub;
 
     n = ack->len;
@@ -270,9 +269,9 @@
     copy_acks_to_mru(ack, ack_mru, n);
 
     /* Number of acks we can resend that still fit into the packet */
-    uint8_t total_acks = min_int(max, ack_mru->len);
+    uint8_t total_acks = (uint8_t)min_int(max, ack_mru->len);
 
-    sub = buf_sub(buf, ACK_SIZE(total_acks), prepend);
+    sub = buf_sub(buf, (int)ACK_SIZE(total_acks), prepend);
     if (!BDEF(&sub))
     {
         goto error;
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 90fe6e9..b8894db 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -634,7 +634,7 @@
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));
 
-    size_t wkc_len = ntohs(net_len);
+    uint16_t wkc_len = ntohs(net_len);
     if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len))
     {
         msg(D_TLS_ERRORS, "Can not locate tls-crypt-v2 client key");
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index b68fb43..10cdc0b 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -292,7 +292,7 @@
  * @return              false on error, true  on success
  *
  * On return enc_len is  set to actual size of the result.
- * enc is NULL or enc_len is not enough to store the result, it is set
+ * If enc is NULL or enc_len is not enough to store the result, it is set
  * to the required size and false is returned.
  */
 bool
@@ -337,8 +337,8 @@
                         MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
                         MAKE_DI(sha512_256), {0, NULL, 0}};
 
-    int out_len = 0;
-    int ret = 0;
+    size_t out_len = 0;
+    bool ret = false;
 
     int nid = OBJ_sn2nid(mdname);
     if (nid == NID_undef)
@@ -354,7 +354,7 @@
 
     if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
-        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;
     }
 
@@ -383,13 +383,13 @@
 
     out_len = tbslen + di->sz;
 
-    if (enc && (out_len <= (int) *enc_len))
+    if (enc && (out_len <= *enc_len))
     {
         /* combine header and digest */
         memcpy(enc, di->header, di->sz);
         memcpy(enc + di->sz, tbs, tbslen);
-        dmsg(D_XKEY, "encode_pkcs1: digest length = %d encoded length = %d",
-             (int) tbslen, (int) out_len);
+        dmsg(D_XKEY, "encode_pkcs1: digest length = %zu encoded length = %zu",
+             tbslen, out_len);
         ret = true;
     }
 
diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index 787b5e3..dfc7287 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -41,6 +41,6 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = rand();
+        output[i] = (uint8_t)rand();
     }
 }
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 9d3ea1a..fdc8fbd 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -97,8 +97,8 @@
 
     for (int i = 0; i < strlen(ciphername); i++)
     {
-        upper[i] = toupper(ciphername[i]);
-        lower[i] = tolower(ciphername[i]);
+        upper[i] = (char)toupper((unsigned char)ciphername[i]);
+        lower[i] = (char)tolower((unsigned char)ciphername[i]);
         if (rand() & 0x1)
         {
             random_case[i] = upper[i];
@@ -155,7 +155,7 @@
 
 
     uint8_t out[32];
-    bool ret = ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
+    bool ret = ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));
 
 #if defined(LIBRESSL_VERSION_NUMBER) || defined(ENABLE_CRYPTO_WOLFSSL)
     /* No TLS1 PRF support in these libraries */
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index ff3f788..a3567bc 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -93,7 +93,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }
 
 static void
@@ -120,7 +120,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }
 
 static void
@@ -151,7 +151,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }
 
 static void
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index cfe9ac3..b92412d 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -368,7 +368,7 @@
     }
 
     /* return a predefined string as sig */
-    memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+    memcpy(sig, good_sig, min_size(sizeof(good_sig), *siglen));
 
     return 1;
 }
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index a01fbe5..4f12f88 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -137,7 +137,7 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = i;
+        output[i] = (uint8_t)i;
     }
     return true;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [PATCH applied] Re: Various fixes for -Wconversion errors
  2024-09-10 12:20 ` [Openvpn-devel] [PATCH v6] " Gert Doering
@ 2024-09-10 12:52   ` Gert Doering
  0 siblings, 0 replies; 11+ messages in thread
From: Gert Doering @ 2024-09-10 12:52 UTC (permalink / raw)
  To: Frank Lichtenheld <frank@; +Cc: openvpn-devel

I'm not a great fan of patches that do nothing more than "appease compilers",
and some of the conversions should really not be necessary - OTOH,
compilers have become better at spotting implicit conversions that *are*
not intended, losing precision and breaking things in the long run - so
better be explicit about int types...

I have stared-at-code (in addition to the ACK by Arne, recorded in Gerrit,
which somehow did not make it into the mail) - looks good.  Also, gave
it some basic tests on Linux and FreeBSD + GHA (-Werror builds), and
all fine..

Your patch has been applied to the master branch.

commit 53449cb61ff569c4862926c7999d50f634030fd9
Author: Frank Lichtenheld
Date:   Tue Sep 10 14:20:08 2024 +0200

     Various fixes for -Wconversion errors

     Signed-off-by: Frank Lichtenheld <frank@...2641...>
     Acked-by: Arne Schwabe <arne@...1227...>
     Message-Id: <20240910122008.23507-1-gert@...1296...>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29172.html
     Signed-off-by: Gert Doering <gert@...1296...>

--
kind regards,

Gert Doering

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (7 preceding siblings ...)
  2024-09-10 12:20 ` [Openvpn-devel] [PATCH v6] " Gert Doering
@ 2024-09-10 12:53 ` cron2 (Code Review)
  2024-09-10 12:53 ` cron2 (Code Review)
  9 siblings, 0 replies; 11+ messages in thread
From: cron2 (Code Review) @ 2024-09-10 12:53 UTC (permalink / raw)
  To: flichtenheld <frank@; +Cc: openvpn-devel

[-- Attachment #1: Type: text/plain, Size: 13318 bytes --]

cron2 has uploaded a new patch set (#7) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/267?usp=email )


Change subject: Various fixes for -Wconversion errors
......................................................................

Various fixes for -Wconversion errors

These are all fixes I considered "safe". They either

- Have sufficient checks/shifts for a cast to be safe
- Fix the type of a variable without requiring code changes
- Are in non-critical unittest code

v2:
 - add min_size instead of abusing min_int
v6:
 - remove change of return value of link_socket_write.
   Move to separate patch.

Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Signed-off-by: Frank Lichtenheld <frank@...2641...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20240910122008.23507-1-gert@...1296...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29172.html
Signed-off-by: Gert Doering <gert@...1296...>
---
M src/openvpn/buffer.c
M src/openvpn/crypto.c
M src/openvpn/integer.h
M src/openvpn/mss.c
M src/openvpn/otime.c
M src/openvpn/otime.h
M src/openvpn/packet_id.c
M src/openvpn/reliable.c
M src/openvpn/tls_crypt.c
M src/openvpn/xkey_helper.c
M tests/unit_tests/openvpn/mock_get_random.c
M tests/unit_tests/openvpn/test_crypto.c
M tests/unit_tests/openvpn/test_packet_id.c
M tests/unit_tests/openvpn/test_provider.c
M tests/unit_tests/openvpn/test_tls_crypt.c
15 files changed, 48 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/67/267/7

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index abe6a9c..9ee76aa 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -326,7 +326,7 @@
         return false;
     }

-    const int size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -863,7 +863,7 @@
         {
             break;
         }
-        line[n++] = c;
+        line[n++] = (char)c;
     }
     while (c);

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c226727..12ad0b9 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -26,6 +26,8 @@
 #include "config.h"
 #endif

+#include <inttypes.h>
+
 #include "syshead.h"
 #include <string.h>

@@ -1283,8 +1285,8 @@
                     hex_byte[hb_index++] = c;
                     if (hb_index == 2)
                     {
-                        unsigned int u;
-                        ASSERT(sscanf((const char *)hex_byte, "%x", &u) == 1);
+                        uint8_t u;
+                        ASSERT(sscanf((const char *)hex_byte, "%" SCNx8, &u) == 1);
                         *out++ = u;
                         hb_index = 0;
                         if (++count == keylen)
@@ -1546,13 +1548,13 @@
     ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH);

-    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    const uint8_t cipher_length = (uint8_t)cipher_kt_key_size(kt->cipher);
     if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }

-    uint8_t hmac_length = md_kt_size(kt->digest);
+    uint8_t hmac_length = (uint8_t)md_kt_size(kt->digest);

     if (!buf_write(buf, &hmac_length, 1))
     {
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index a1acaf9..34088ab 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -28,12 +28,12 @@

 #ifndef htonll
 #define htonll(x) ((1==htonl(1)) ? (x) : \
-                   ((uint64_t)htonl((x) & 0xFFFFFFFF) << 32) | htonl((x) >> 32))
+                   ((uint64_t)htonl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | htonl((uint32_t)((x) >> 32)))
 #endif

 #ifndef ntohll
 #define ntohll(x) ((1==ntohl(1)) ? (x) : \
-                   ((uint64_t)ntohl((x) & 0xFFFFFFFF) << 32) | ntohl((x) >> 32))
+                   ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif

 static inline int
@@ -72,6 +72,19 @@
     }
 }

+static inline size_t
+min_size(size_t x, size_t y)
+{
+    if (x < y)
+    {
+        return x;
+    }
+    else
+    {
+        return y;
+    }
+}
+
 static inline int
 max_int(int x, int y)
 {
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 635557c..ebdec25 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -165,7 +165,7 @@
         return;
     }

-    for (olen = hlen - sizeof(struct openvpn_tcphdr),
+    for (olen = hlen - (int) sizeof(struct openvpn_tcphdr),
          opt = (uint8_t *)(tc + 1);
          olen > 1;
          olen -= optlen, opt += optlen)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index 3cde574..d77c99e 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -105,7 +105,7 @@
 /* format a time_t as ascii, or use current time if 0 */

 const char *
-time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc)
+time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc)
 {
     struct buffer out = alloc_buf_gc(64, gc);
     struct timeval tv;
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index c37673e..9543732 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -43,7 +43,7 @@
 bool frequency_limit_event_allowed(struct frequency_limit *f);

 /* format a time_t as ascii, or use current time if 0 */
-const char *time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc);
+const char *time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc);

 /* struct timeval functions */

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index be28999..fb962e4 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -588,14 +588,14 @@
         }
         else
         {
-            diff = (int) prev_now - v;
+            diff = (int)(prev_now - v);
             if (diff < 0)
             {
                 c = 'N';
             }
             else if (diff < 10)
             {
-                c = '0' + diff;
+                c = (char)('0' + diff);
             }
             else
             {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index a789990..019ec18 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -257,8 +257,7 @@
                    struct buffer *buf,
                    const struct session_id *sid, int max, bool prepend)
 {
-    int i, j;
-    uint8_t n;
+    int i, j, n;
     struct buffer sub;

     n = ack->len;
@@ -270,9 +269,9 @@
     copy_acks_to_mru(ack, ack_mru, n);

     /* Number of acks we can resend that still fit into the packet */
-    uint8_t total_acks = min_int(max, ack_mru->len);
+    uint8_t total_acks = (uint8_t)min_int(max, ack_mru->len);

-    sub = buf_sub(buf, ACK_SIZE(total_acks), prepend);
+    sub = buf_sub(buf, (int)ACK_SIZE(total_acks), prepend);
     if (!BDEF(&sub))
     {
         goto error;
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 90fe6e9..b8894db 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -634,7 +634,7 @@
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));

-    size_t wkc_len = ntohs(net_len);
+    uint16_t wkc_len = ntohs(net_len);
     if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len))
     {
         msg(D_TLS_ERRORS, "Can not locate tls-crypt-v2 client key");
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index b68fb43..10cdc0b 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -292,7 +292,7 @@
  * @return              false on error, true  on success
  *
  * On return enc_len is  set to actual size of the result.
- * enc is NULL or enc_len is not enough to store the result, it is set
+ * If enc is NULL or enc_len is not enough to store the result, it is set
  * to the required size and false is returned.
  */
 bool
@@ -337,8 +337,8 @@
                         MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
                         MAKE_DI(sha512_256), {0, NULL, 0}};

-    int out_len = 0;
-    int ret = 0;
+    size_t out_len = 0;
+    bool ret = false;

     int nid = OBJ_sn2nid(mdname);
     if (nid == NID_undef)
@@ -354,7 +354,7 @@

     if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
-        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;
     }

@@ -383,13 +383,13 @@

     out_len = tbslen + di->sz;

-    if (enc && (out_len <= (int) *enc_len))
+    if (enc && (out_len <= *enc_len))
     {
         /* combine header and digest */
         memcpy(enc, di->header, di->sz);
         memcpy(enc + di->sz, tbs, tbslen);
-        dmsg(D_XKEY, "encode_pkcs1: digest length = %d encoded length = %d",
-             (int) tbslen, (int) out_len);
+        dmsg(D_XKEY, "encode_pkcs1: digest length = %zu encoded length = %zu",
+             tbslen, out_len);
         ret = true;
     }

diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index 787b5e3..dfc7287 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -41,6 +41,6 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = rand();
+        output[i] = (uint8_t)rand();
     }
 }
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 9d3ea1a..fdc8fbd 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -97,8 +97,8 @@

     for (int i = 0; i < strlen(ciphername); i++)
     {
-        upper[i] = toupper(ciphername[i]);
-        lower[i] = tolower(ciphername[i]);
+        upper[i] = (char)toupper((unsigned char)ciphername[i]);
+        lower[i] = (char)tolower((unsigned char)ciphername[i]);
         if (rand() & 0x1)
         {
             random_case[i] = upper[i];
@@ -155,7 +155,7 @@


     uint8_t out[32];
-    bool ret = ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
+    bool ret = ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));

 #if defined(LIBRESSL_VERSION_NUMBER) || defined(ENABLE_CRYPTO_WOLFSSL)
     /* No TLS1 PRF support in these libraries */
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index ff3f788..a3567bc 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -93,7 +93,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -120,7 +120,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -151,7 +151,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index cfe9ac3..b92412d 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -368,7 +368,7 @@
     }

     /* return a predefined string as sig */
-    memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+    memcpy(sig, good_sig, min_size(sizeof(good_sig), *siglen));

     return 1;
 }
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index a01fbe5..4f12f88 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -137,7 +137,7 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = i;
+        output[i] = (uint8_t)i;
     }
     return true;
 }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 7
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: cron2 <gert@...1296...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-MessageType: newpatchset

[-- Attachment #2: Type: text/html, Size: 22743 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors
       [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
                   ` (8 preceding siblings ...)
  2024-09-10 12:53 ` [Openvpn-devel] [M] Change in openvpn[master]: " cron2 (Code Review)
@ 2024-09-10 12:53 ` cron2 (Code Review)
  9 siblings, 0 replies; 11+ messages in thread
From: cron2 (Code Review) @ 2024-09-10 12:53 UTC (permalink / raw)
  To: flichtenheld <frank@; +Cc: plaisthos <arne-openvpn@

[-- Attachment #1: Type: text/plain, Size: 13183 bytes --]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/267?usp=email )

Change subject: Various fixes for -Wconversion errors
......................................................................

Various fixes for -Wconversion errors

These are all fixes I considered "safe". They either

- Have sufficient checks/shifts for a cast to be safe
- Fix the type of a variable without requiring code changes
- Are in non-critical unittest code

v2:
 - add min_size instead of abusing min_int
v6:
 - remove change of return value of link_socket_write.
   Move to separate patch.

Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Signed-off-by: Frank Lichtenheld <frank@...2641...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20240910122008.23507-1-gert@...1296...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29172.html
Signed-off-by: Gert Doering <gert@...1296...>
---
M src/openvpn/buffer.c
M src/openvpn/crypto.c
M src/openvpn/integer.h
M src/openvpn/mss.c
M src/openvpn/otime.c
M src/openvpn/otime.h
M src/openvpn/packet_id.c
M src/openvpn/reliable.c
M src/openvpn/tls_crypt.c
M src/openvpn/xkey_helper.c
M tests/unit_tests/openvpn/mock_get_random.c
M tests/unit_tests/openvpn/test_crypto.c
M tests/unit_tests/openvpn/test_packet_id.c
M tests/unit_tests/openvpn/test_provider.c
M tests/unit_tests/openvpn/test_tls_crypt.c
15 files changed, 48 insertions(+), 34 deletions(-)




diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index abe6a9c..9ee76aa 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -326,7 +326,7 @@
         return false;
     }

-    const int size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -863,7 +863,7 @@
         {
             break;
         }
-        line[n++] = c;
+        line[n++] = (char)c;
     }
     while (c);

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c226727..12ad0b9 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -26,6 +26,8 @@
 #include "config.h"
 #endif

+#include <inttypes.h>
+
 #include "syshead.h"
 #include <string.h>

@@ -1283,8 +1285,8 @@
                     hex_byte[hb_index++] = c;
                     if (hb_index == 2)
                     {
-                        unsigned int u;
-                        ASSERT(sscanf((const char *)hex_byte, "%x", &u) == 1);
+                        uint8_t u;
+                        ASSERT(sscanf((const char *)hex_byte, "%" SCNx8, &u) == 1);
                         *out++ = u;
                         hb_index = 0;
                         if (++count == keylen)
@@ -1546,13 +1548,13 @@
     ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH);

-    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    const uint8_t cipher_length = (uint8_t)cipher_kt_key_size(kt->cipher);
     if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }

-    uint8_t hmac_length = md_kt_size(kt->digest);
+    uint8_t hmac_length = (uint8_t)md_kt_size(kt->digest);

     if (!buf_write(buf, &hmac_length, 1))
     {
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index a1acaf9..34088ab 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -28,12 +28,12 @@

 #ifndef htonll
 #define htonll(x) ((1==htonl(1)) ? (x) : \
-                   ((uint64_t)htonl((x) & 0xFFFFFFFF) << 32) | htonl((x) >> 32))
+                   ((uint64_t)htonl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | htonl((uint32_t)((x) >> 32)))
 #endif

 #ifndef ntohll
 #define ntohll(x) ((1==ntohl(1)) ? (x) : \
-                   ((uint64_t)ntohl((x) & 0xFFFFFFFF) << 32) | ntohl((x) >> 32))
+                   ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif

 static inline int
@@ -72,6 +72,19 @@
     }
 }

+static inline size_t
+min_size(size_t x, size_t y)
+{
+    if (x < y)
+    {
+        return x;
+    }
+    else
+    {
+        return y;
+    }
+}
+
 static inline int
 max_int(int x, int y)
 {
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 635557c..ebdec25 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -165,7 +165,7 @@
         return;
     }

-    for (olen = hlen - sizeof(struct openvpn_tcphdr),
+    for (olen = hlen - (int) sizeof(struct openvpn_tcphdr),
          opt = (uint8_t *)(tc + 1);
          olen > 1;
          olen -= optlen, opt += optlen)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index 3cde574..d77c99e 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -105,7 +105,7 @@
 /* format a time_t as ascii, or use current time if 0 */

 const char *
-time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc)
+time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc)
 {
     struct buffer out = alloc_buf_gc(64, gc);
     struct timeval tv;
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index c37673e..9543732 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -43,7 +43,7 @@
 bool frequency_limit_event_allowed(struct frequency_limit *f);

 /* format a time_t as ascii, or use current time if 0 */
-const char *time_string(time_t t, int usec, bool show_usec, struct gc_arena *gc);
+const char *time_string(time_t t, long usec, bool show_usec, struct gc_arena *gc);

 /* struct timeval functions */

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index be28999..fb962e4 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -588,14 +588,14 @@
         }
         else
         {
-            diff = (int) prev_now - v;
+            diff = (int)(prev_now - v);
             if (diff < 0)
             {
                 c = 'N';
             }
             else if (diff < 10)
             {
-                c = '0' + diff;
+                c = (char)('0' + diff);
             }
             else
             {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index a789990..019ec18 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -257,8 +257,7 @@
                    struct buffer *buf,
                    const struct session_id *sid, int max, bool prepend)
 {
-    int i, j;
-    uint8_t n;
+    int i, j, n;
     struct buffer sub;

     n = ack->len;
@@ -270,9 +269,9 @@
     copy_acks_to_mru(ack, ack_mru, n);

     /* Number of acks we can resend that still fit into the packet */
-    uint8_t total_acks = min_int(max, ack_mru->len);
+    uint8_t total_acks = (uint8_t)min_int(max, ack_mru->len);

-    sub = buf_sub(buf, ACK_SIZE(total_acks), prepend);
+    sub = buf_sub(buf, (int)ACK_SIZE(total_acks), prepend);
     if (!BDEF(&sub))
     {
         goto error;
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 90fe6e9..b8894db 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -634,7 +634,7 @@
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));

-    size_t wkc_len = ntohs(net_len);
+    uint16_t wkc_len = ntohs(net_len);
     if (!buf_advance(&wrapped_client_key, BLEN(&wrapped_client_key) - wkc_len))
     {
         msg(D_TLS_ERRORS, "Can not locate tls-crypt-v2 client key");
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index b68fb43..10cdc0b 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -292,7 +292,7 @@
  * @return              false on error, true  on success
  *
  * On return enc_len is  set to actual size of the result.
- * enc is NULL or enc_len is not enough to store the result, it is set
+ * If enc is NULL or enc_len is not enough to store the result, it is set
  * to the required size and false is returned.
  */
 bool
@@ -337,8 +337,8 @@
                         MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
                         MAKE_DI(sha512_256), {0, NULL, 0}};

-    int out_len = 0;
-    int ret = 0;
+    size_t out_len = 0;
+    bool ret = false;

     int nid = OBJ_sn2nid(mdname);
     if (nid == NID_undef)
@@ -354,7 +354,7 @@

     if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
-        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+        msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;
     }

@@ -383,13 +383,13 @@

     out_len = tbslen + di->sz;

-    if (enc && (out_len <= (int) *enc_len))
+    if (enc && (out_len <= *enc_len))
     {
         /* combine header and digest */
         memcpy(enc, di->header, di->sz);
         memcpy(enc + di->sz, tbs, tbslen);
-        dmsg(D_XKEY, "encode_pkcs1: digest length = %d encoded length = %d",
-             (int) tbslen, (int) out_len);
+        dmsg(D_XKEY, "encode_pkcs1: digest length = %zu encoded length = %zu",
+             tbslen, out_len);
         ret = true;
     }

diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index 787b5e3..dfc7287 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -41,6 +41,6 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = rand();
+        output[i] = (uint8_t)rand();
     }
 }
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 9d3ea1a..fdc8fbd 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -97,8 +97,8 @@

     for (int i = 0; i < strlen(ciphername); i++)
     {
-        upper[i] = toupper(ciphername[i]);
-        lower[i] = tolower(ciphername[i]);
+        upper[i] = (char)toupper((unsigned char)ciphername[i]);
+        lower[i] = (char)tolower((unsigned char)ciphername[i]);
         if (rand() & 0x1)
         {
             random_case[i] = upper[i];
@@ -155,7 +155,7 @@


     uint8_t out[32];
-    bool ret = ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
+    bool ret = ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));

 #if defined(LIBRESSL_VERSION_NUMBER) || defined(ENABLE_CRYPTO_WOLFSSL)
     /* No TLS1 PRF support in these libraries */
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index ff3f788..a3567bc 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -93,7 +93,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -120,7 +120,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
@@ -151,7 +151,7 @@
     assert(data->pis.id == 1);
     assert(data->pis.time == now);
     assert_true(data->test_buf_data.buf_id == htonl(1));
-    assert_true(data->test_buf_data.buf_time == htonl(now));
+    assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
 }

 static void
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index cfe9ac3..b92412d 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -368,7 +368,7 @@
     }

     /* return a predefined string as sig */
-    memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+    memcpy(sig, good_sig, min_size(sizeof(good_sig), *siglen));

     return 1;
 }
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index a01fbe5..4f12f88 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -137,7 +137,7 @@
 {
     for (int i = 0; i < len; i++)
     {
-        output[i] = i;
+        output[i] = (uint8_t)i;
     }
     return true;
 }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6818b153bdeb1eed65870af99b0531e95807fe0f
Gerrit-Change-Number: 267
Gerrit-PatchSet: 7
Gerrit-Owner: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: cron2 <gert@...1296...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-MessageType: merged

[-- Attachment #2: Type: text/html, Size: 22580 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-09-10 12:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <gerrit.1683717398000.I6818b153bdeb1eed65870af99b0531e95807fe0f@...2715...>
2023-09-07  8:45 ` [Openvpn-devel] [M] Change in openvpn[master]: Various fixes for -Wconversion errors flichtenheld (Code Review)
2023-09-07  8:45 ` flichtenheld (Code Review)
2023-11-20 10:30 ` plaisthos (Code Review)
2023-11-21 10:10 ` [Openvpn-devel] [PATCH v5] " Frank Lichtenheld
2024-09-10 10:56 ` [Openvpn-devel] [M] Change in openvpn[master]: " cron2 (Code Review)
2024-09-10 12:14 ` flichtenheld (Code Review)
2024-09-10 12:15 ` flichtenheld (Code Review)
2024-09-10 12:20 ` [Openvpn-devel] [PATCH v6] " Gert Doering
2024-09-10 12:52   ` [Openvpn-devel] [PATCH applied] " Gert Doering
2024-09-10 12:53 ` [Openvpn-devel] [M] Change in openvpn[master]: " cron2 (Code Review)
2024-09-10 12:53 ` cron2 (Code Review)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.