| Message ID | 20230207134333.52221-1-frank@lichtenheld.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
Looks good, looked at the code test-compiled in MSVC. Acked-by: Lev Stipakov <lstipakov@gmail.com> ti 7. helmik. 2023 klo 15.44 Frank Lichtenheld (frank@lichtenheld.com) kirjoitti: > > Relevant defines/typedefs: > typedef UINT_PTR SOCKET; > if defined(_WIN64) > typedef unsigned __int64 UINT_PTR; > else > typedef unsigned int UINT_PTR; > endif > ifdef _WIN64 > define PRIuPTR PRIu64 > else > define PRIuPTR PRIu32 > endif > > Remove duplicated include of inttypes.h > > Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> > --- > src/openvpn/error.c | 4 ++-- > src/openvpn/syshead.h | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > v2: > - use PRIuPTR as discussed on IRC (added relevant defines to > commit message) > - remove redundant include for inttypes.h from syshead.h > > diff --git a/src/openvpn/error.c b/src/openvpn/error.c > index 89a08cec..a2c9aa4c 100644 > --- a/src/openvpn/error.c > +++ b/src/openvpn/error.c > @@ -695,14 +695,14 @@ x_check_status(int status, > { > if (extended_msg) > { > - msg(x_cs_info_level, "%s %s [%s]: %s (fd=%d,code=%d)", description, > + msg(x_cs_info_level, "%s %s [%s]: %s (fd=" SOCKET_PRINTF ",code=%d)", description, > sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", > extended_msg, openvpn_strerror(my_errno, crt_error, &gc), > sock ? sock->sd : -1, my_errno); > } > else > { > - msg(x_cs_info_level, "%s %s: %s (fd=%d,code=%d)", description, > + msg(x_cs_info_level, "%s %s: %s (fd=" SOCKET_PRINTF ",code=%d)", description, > sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", > openvpn_strerror(my_errno, crt_error, &gc), > sock ? sock->sd : -1, my_errno); > diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h > index fe91bc11..12ccf2f4 100644 > --- a/src/openvpn/syshead.h > +++ b/src/openvpn/syshead.h > @@ -48,7 +48,6 @@ > #ifdef _MSC_VER /* Visual Studio */ > #define __func__ __FUNCTION__ > #define __attribute__(x) > -#include <inttypes.h> > #endif > > #if defined(__APPLE__) > @@ -442,9 +441,11 @@ typedef unsigned short sa_family_t; > */ > #ifdef _WIN32 > #define SOCKET_UNDEFINED (INVALID_SOCKET) > +#define SOCKET_PRINTF "%" PRIuPTR > typedef SOCKET socket_descriptor_t; > #else > #define SOCKET_UNDEFINED (-1) > +#define SOCKET_PRINTF "%d" > typedef int socket_descriptor_t; > #endif > > -- > 2.34.1 > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Test built on MinGW and Linux. Haven't actually tried to trigger an error
message which would contain a fd=nnn output, but for "non windows" this
is a no-op change, and "for windows" it looks correct.
Your patch has been applied to the master and release/2.6 branch.
commit a95705be85cb1d3a5868efaeb960ec5d625d2f11 (master)
commit 37e23c9816754ceeb67d5365b639f0b37f804591 (release/2.6)
Author: Frank Lichtenheld
Date: Tue Feb 7 14:43:33 2023 +0100
Windows: fix wrong printf format in x_check_status
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230207134333.52221-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26166.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
CC: list ---------- Forwarded message --------- From: Selva Nair <selva.nair@gmail.com> Date: Tue, Feb 7, 2023 at 11:57 AM Subject: Re: [Openvpn-devel] [PATCH v2 3/5] Windows: fix wrong printf format in x_check_status To: Frank Lichtenheld <frank@lichtenheld.com> Nitpicking: > - use PRIuPTR as discussed on IRC (added relevant defines to > commit message) > The most interesting value to log for SOCKET on Windows is INVALID_SOCKET = ~0 (i.e, -1). It will print with PRIuPTR as 18446744073709551615. PRIdPTR which will give -1 or PRIxPTR (ffff....ffff) may be better in this case? Though socket is a handle on Windows, I think it never gets too large to be -ve except for this special value. I'm fine with this too as unsigned decimals for -1 in both 32 bit and 64 bit are not hard to spot. Selva
diff --git a/src/openvpn/error.c b/src/openvpn/error.c index 89a08cec..a2c9aa4c 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -695,14 +695,14 @@ x_check_status(int status, { if (extended_msg) { - msg(x_cs_info_level, "%s %s [%s]: %s (fd=%d,code=%d)", description, + msg(x_cs_info_level, "%s %s [%s]: %s (fd=" SOCKET_PRINTF ",code=%d)", description, sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", extended_msg, openvpn_strerror(my_errno, crt_error, &gc), sock ? sock->sd : -1, my_errno); } else { - msg(x_cs_info_level, "%s %s: %s (fd=%d,code=%d)", description, + msg(x_cs_info_level, "%s %s: %s (fd=" SOCKET_PRINTF ",code=%d)", description, sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", openvpn_strerror(my_errno, crt_error, &gc), sock ? sock->sd : -1, my_errno); diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h index fe91bc11..12ccf2f4 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -48,7 +48,6 @@ #ifdef _MSC_VER /* Visual Studio */ #define __func__ __FUNCTION__ #define __attribute__(x) -#include <inttypes.h> #endif #if defined(__APPLE__) @@ -442,9 +441,11 @@ typedef unsigned short sa_family_t; */ #ifdef _WIN32 #define SOCKET_UNDEFINED (INVALID_SOCKET) +#define SOCKET_PRINTF "%" PRIuPTR typedef SOCKET socket_descriptor_t; #else #define SOCKET_UNDEFINED (-1) +#define SOCKET_PRINTF "%d" typedef int socket_descriptor_t; #endif
Relevant defines/typedefs: typedef UINT_PTR SOCKET; if defined(_WIN64) typedef unsigned __int64 UINT_PTR; else typedef unsigned int UINT_PTR; endif ifdef _WIN64 define PRIuPTR PRIu64 else define PRIuPTR PRIu32 endif Remove duplicated include of inttypes.h Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> --- src/openvpn/error.c | 4 ++-- src/openvpn/syshead.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) v2: - use PRIuPTR as discussed on IRC (added relevant defines to commit message) - remove redundant include for inttypes.h from syshead.h