[Openvpn-devel] Use proper print format/casting when converting msg_channel handle

Message ID 20230214134323.1033590-1-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel] Use proper print format/casting when converting msg_channel handle | expand

Commit Message

Arne Schwabe Feb. 14, 2023, 1:43 p.m. UTC
The current casting triggers a warning on 32bit:

    init.c:1842:66: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]

Use the proper printf format specifier for printing a pointer avoiding
the cast alltogether.

In options.c use a cast to intptr_t before converting to a handle to
avoid having to ifdef atoll/atol for 32/64 bit.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/init.c    | 3 ++-
 src/openvpn/options.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Lev Stipakov March 1, 2023, 1:16 p.m. UTC | #1
Hi,

Sadly MSVC doesn't show those warnings even on L4.

Change looks good. I couldn't (yet) test on x86 since I (hopefully not
for long) lost access to the x86 machine, but arm64 works just fine.

Acked-by: Lev Stipakov <lstipakov@gmail.com>

ti 14. helmik. 2023 klo 15.44 Arne Schwabe (arne@rfc2549.org) kirjoitti:
>
> The current casting triggers a warning on 32bit:
>
>     init.c:1842:66: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>
> Use the proper printf format specifier for printing a pointer avoiding
> the cast alltogether.
>
> In options.c use a cast to intptr_t before converting to a handle to
> avoid having to ifdef atoll/atol for 32/64 bit.
>
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/init.c    | 3 ++-
>  src/openvpn/options.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 0ad2c7e09..0d50d9189 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -1839,7 +1839,8 @@ do_open_tun(struct context *c, int *error_flags)
>  #ifdef _WIN32
>          /* store (hide) interactive service handle in tuntap_options */
>          c->c1.tuntap->options.msg_channel = c->options.msg_channel;
> -        msg(D_ROUTE, "interactive service msg_channel=%" PRIu64, (unsigned long long) c->options.msg_channel);
> +        msg(D_ROUTE, "interactive service msg_channel=%" PRIuPTR,
> +            (intptr_t) c->options.msg_channel);
>  #endif
>
>          /* allocate route list structure */
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index c1ddb0262..679528187 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -7882,7 +7882,7 @@ add_option(struct options *options,
>  #ifdef _WIN32
>          VERIFY_PERMISSION(OPT_P_GENERAL);
>          HANDLE process = GetCurrentProcess();
> -        HANDLE handle = (HANDLE) atoll(p[1]);
> +        HANDLE handle = (HANDLE) ((intptr_t) atoll(p[1]));
>          if (!DuplicateHandle(process, handle, process, &options->msg_channel, 0,
>                               FALSE, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS))
>          {
> --
> 2.37.1 (Apple Git-137.1)
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Gert Doering March 1, 2023, 1:30 p.m. UTC | #2
I have not tested this beyond "compile on mingw", which didn't warn
before the patch either.  But it looks innocent enough, and if Lev
confirms it actually works, good enough.

Your patch has been applied to the master and release/2.6 branch.

commit 9c52e0c610ef1229561c2d038ca41fe2cbefe8da (master)
commit 27dac5061cfeff75470dca11e07dadb1fb0ad180 (release/2.6)
Author: Arne Schwabe
Date:   Tue Feb 14 14:43:23 2023 +0100

     Use proper print format/casting when converting msg_channel handle

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Lev Stipakov <lstipakov@gmail.com>
     Message-Id: <20230214134323.1033590-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26255.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 0ad2c7e09..0d50d9189 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1839,7 +1839,8 @@  do_open_tun(struct context *c, int *error_flags)
 #ifdef _WIN32
         /* store (hide) interactive service handle in tuntap_options */
         c->c1.tuntap->options.msg_channel = c->options.msg_channel;
-        msg(D_ROUTE, "interactive service msg_channel=%" PRIu64, (unsigned long long) c->options.msg_channel);
+        msg(D_ROUTE, "interactive service msg_channel=%" PRIuPTR,
+            (intptr_t) c->options.msg_channel);
 #endif
 
         /* allocate route list structure */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index c1ddb0262..679528187 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7882,7 +7882,7 @@  add_option(struct options *options,
 #ifdef _WIN32
         VERIFY_PERMISSION(OPT_P_GENERAL);
         HANDLE process = GetCurrentProcess();
-        HANDLE handle = (HANDLE) atoll(p[1]);
+        HANDLE handle = (HANDLE) ((intptr_t) atoll(p[1]));
         if (!DuplicateHandle(process, handle, process, &options->msg_channel, 0,
                              FALSE, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS))
         {