| Message ID | 20210421134348.1950392-4-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,1/7] Remove --disable-multihome option | expand |
Hi, On 21/04/2021 15:43, Arne Schwabe wrote: > Move to definition inside the ifdef where they are used to avoid > unused warnings. > > Fix a few printf related warnings when DWORD is used as paramter and > the printf format should be %lu (long unsigned int) > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/route.c | 4 ++-- > src/openvpn/socket.c | 3 ++- > src/openvpn/tun.c | 2 +- > 3 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/src/openvpn/route.c b/src/openvpn/route.c > index c6b3dc584..c83bd9e2b 100644 > --- a/src/openvpn/route.c > +++ b/src/openvpn/route.c > @@ -2360,7 +2360,6 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, > #else > int metric; > #endif > - const char *device = tt->actual_name; > bool gateway_needed = false; > > if ((r6->flags & (RT_DEFINED|RT_ADDED)) != (RT_DEFINED|RT_ADDED)) > @@ -2369,6 +2368,7 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, > } > > #ifndef _WIN32 > + const char *device = tt->actual_name; This variable is not used in all cases embraced by "ifndef _WIN32". I.e. I think it is not used when any of the following is defined: TARGET_OPENBSD TARGET_NETBSD TARGET_AIX Therefore this change would fix *only* the warning on Windows. Do we want to go this way? or should we rather rearrange the ifdefs a bit so that we have a cleaner change/function? > if (r6->iface != NULL) /* vpn server special route */ > { > device = r6->iface; > @@ -2713,7 +2713,7 @@ get_default_gateway_row(const MIB_IPFORWARDTABLE *routes) > const DWORD index = row->dwForwardIfIndex; > const DWORD metric = row->dwForwardMetric1; > > - dmsg(D_ROUTE_DEBUG, "GDGR: route[%d] %s/%s i=%d m=%d", > + dmsg(D_ROUTE_DEBUG, "GDGR: route[%lu] %s/%s i=%d m=%d", shouldn't be enough to convert to %u ? (/me is not a DOWRD expert though) > i, > print_in_addr_t((in_addr_t) net, 0, &gc), > print_in_addr_t((in_addr_t) mask, 0, &gc), > diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c > index b13d2e0f1..8a6e42cc6 100644 > --- a/src/openvpn/socket.c > +++ b/src/openvpn/socket.c > @@ -2837,10 +2837,11 @@ print_link_socket_actual_ex(const struct link_socket_actual *act, > { > if (act) > { > - char ifname[IF_NAMESIZE] = "[undef]"; > struct buffer out = alloc_buf_gc(128, gc); > buf_printf(&out, "%s", print_sockaddr_ex(&act->dest.addr.sa, separator, flags, gc)); > #if ENABLE_IP_PKTINFO > + char ifname[IF_NAMESIZE] = "[undef]"; This makes sense. > + > if ((flags & PS_SHOW_PKTINFO) && addr_defined_ipi(act)) > { > switch (act->dest.addr.sa.sa_family) > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 2c1b270dd..4ef34e4eb 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -5606,7 +5606,7 @@ windows_set_mtu(const int iface_index, const short family, > > if (err != NO_ERROR) > { > - msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]", > + msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%lu if_index=%d]", same question as above. > family_name, strerror_win32(err, &gc), err, iface_index); > } > else > Regards,
>> #ifndef _WIN32 >> + const char *device = tt->actual_name; > > This variable is not used in all cases embraced by "ifndef _WIN32". > I.e. I think it is not used when any of the following is defined: > TARGET_OPENBSD > TARGET_NETBSD > TARGET_AIX > > Therefore this change would fix *only* the warning on Windows. > Do we want to go this way? or should we rather rearrange the ifdefs a > bit so that we have a cleaner change/function? No all other platforms. The variable is used just 2 lines below it definition to set gateway_needed which is used by all platforms apart from _WIN32. So only WIN32 had this warning. > > > >> if (r6->iface != NULL) /* vpn server special route */ >> { >> device = r6->iface; >> @@ -2713,7 +2713,7 @@ get_default_gateway_row(const MIB_IPFORWARDTABLE *routes) >> const DWORD index = row->dwForwardIfIndex; >> const DWORD metric = row->dwForwardMetric1; >> >> - dmsg(D_ROUTE_DEBUG, "GDGR: route[%d] %s/%s i=%d m=%d", >> + dmsg(D_ROUTE_DEBUG, "GDGR: route[%lu] %s/%s i=%d m=%d", > > shouldn't be enough to convert to %u ? (/me is not a DOWRD expert though) No. DWORD is a long unsigned int as mentioned in the commit message. It is 32 bit int since Windows always has long as 32 bit. It is a bit confusing when you are used to the long=64 bit :) >> index 2c1b270dd..4ef34e4eb 100644 >> --- a/src/openvpn/tun.c >> +++ b/src/openvpn/tun.c >> @@ -5606,7 +5606,7 @@ windows_set_mtu(const int iface_index, const short family, >> >> if (err != NO_ERROR) >> { >> - msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]", >> + msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%lu if_index=%d]", > > same question as above. > Same answer :) Arne
Hi, On 22/04/2021 14:39, Arne Schwabe wrote: > >>> #ifndef _WIN32 >>> + const char *device = tt->actual_name; >> >> This variable is not used in all cases embraced by "ifndef _WIN32". >> I.e. I think it is not used when any of the following is defined: >> TARGET_OPENBSD >> TARGET_NETBSD >> TARGET_AIX >> >> Therefore this change would fix *only* the warning on Windows. >> Do we want to go this way? or should we rather rearrange the ifdefs a >> bit so that we have a cleaner change/function? > > > No all other platforms. The variable is used just 2 lines below it > definition to set gateway_needed which is used by all platforms apart > from _WIN32. So only WIN32 had this warning. Well, it's assigned but not used on the platforms I mentioned. I guess the compiler isn't able to throw a warning at the moment though. Anyway, I am fine with keeping this change as it is. This code may require some bigger restructuring at some point. So fixing the usage of one variable only doesn't give us anything. But at least this changes gets rid of a warning :) > >> >> >> >>> if (r6->iface != NULL) /* vpn server special route */ >>> { >>> device = r6->iface; >>> @@ -2713,7 +2713,7 @@ get_default_gateway_row(const MIB_IPFORWARDTABLE *routes) >>> const DWORD index = row->dwForwardIfIndex; >>> const DWORD metric = row->dwForwardMetric1; >>> >>> - dmsg(D_ROUTE_DEBUG, "GDGR: route[%d] %s/%s i=%d m=%d", >>> + dmsg(D_ROUTE_DEBUG, "GDGR: route[%lu] %s/%s i=%d m=%d", >> >> shouldn't be enough to convert to %u ? (/me is not a DOWRD expert though) > > No. DWORD is a long unsigned int as mentioned in the commit message. It > is 32 bit int since Windows always has long as 32 bit. It is a bit > confusing when you are used to the long=64 bit :) > >>> index 2c1b270dd..4ef34e4eb 100644 >>> --- a/src/openvpn/tun.c >>> +++ b/src/openvpn/tun.c >>> @@ -5606,7 +5606,7 @@ windows_set_mtu(const int iface_index, const short family, >>> >>> if (err != NO_ERROR) >>> { >>> - msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]", >>> + msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%lu if_index=%d]", >> >> same question as above. >> Thanks for the explanation on DWORD. Compiled on my rig and no problem showed up. Acked-by: Antonio Quartulli <antonio@openvpn.net>
Your patch has been applied to the master branch.
commit 7890e51aab91b304045a0163462f334b8e5ae8e8
Author: Arne Schwabe
Date: Wed Apr 21 15:43:45 2021 +0200
Fix a number of mingw warnings
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210421134348.1950392-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22176.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/route.c b/src/openvpn/route.c index c6b3dc584..c83bd9e2b 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -2360,7 +2360,6 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, #else int metric; #endif - const char *device = tt->actual_name; bool gateway_needed = false; if ((r6->flags & (RT_DEFINED|RT_ADDED)) != (RT_DEFINED|RT_ADDED)) @@ -2369,6 +2368,7 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, } #ifndef _WIN32 + const char *device = tt->actual_name; if (r6->iface != NULL) /* vpn server special route */ { device = r6->iface; @@ -2713,7 +2713,7 @@ get_default_gateway_row(const MIB_IPFORWARDTABLE *routes) const DWORD index = row->dwForwardIfIndex; const DWORD metric = row->dwForwardMetric1; - dmsg(D_ROUTE_DEBUG, "GDGR: route[%d] %s/%s i=%d m=%d", + dmsg(D_ROUTE_DEBUG, "GDGR: route[%lu] %s/%s i=%d m=%d", i, print_in_addr_t((in_addr_t) net, 0, &gc), print_in_addr_t((in_addr_t) mask, 0, &gc), diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index b13d2e0f1..8a6e42cc6 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2837,10 +2837,11 @@ print_link_socket_actual_ex(const struct link_socket_actual *act, { if (act) { - char ifname[IF_NAMESIZE] = "[undef]"; struct buffer out = alloc_buf_gc(128, gc); buf_printf(&out, "%s", print_sockaddr_ex(&act->dest.addr.sa, separator, flags, gc)); #if ENABLE_IP_PKTINFO + char ifname[IF_NAMESIZE] = "[undef]"; + if ((flags & PS_SHOW_PKTINFO) && addr_defined_ipi(act)) { switch (act->dest.addr.sa.sa_family) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 2c1b270dd..4ef34e4eb 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -5606,7 +5606,7 @@ windows_set_mtu(const int iface_index, const short family, if (err != NO_ERROR) { - msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]", + msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%lu if_index=%d]", family_name, strerror_win32(err, &gc), err, iface_index); } else
Move to definition inside the ifdef where they are used to avoid unused warnings. Fix a few printf related warnings when DWORD is used as paramter and the printf format should be %lu (long unsigned int) Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/route.c | 4 ++-- src/openvpn/socket.c | 3 ++- src/openvpn/tun.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-)