Skip to contentSkip to conversation

Fix windows-specific format specifiers

PATCH Superseded Steffan Karger 2 participants 5 messages Mar 9, 2017

lore
Share & tools
lore

Steffan Karger

author
#0 lore raw

PATCH 1/1 Fix windows-specific format specifiers

A number of printf-like functions in windows-specific code used incorrect format specifiers, which could potentially lead to incorrect values being printed.

Signed-off-by Steffan Karger <[email protected]...>
---
 src/openvpn/route.c  | 12 ++++++------
 src/openvpn/socket.c |  6 +++---
 src/openvpn/tun.c    |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
src/openvpn/route.c +6 -6
index fec1c25..25b5fce 100644
@@ -1967,12 +1967,12 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
         struct buffer out = alloc_buf_gc(64, &gc);
         if (r6->adapter_index)          /* vpn server special route */
         {
-            buf_printf(&out, "interface=%d", r6->adapter_index );
+            buf_printf(&out, "interface=%lu", r6->adapter_index );
             gateway_needed = true;
         }
         else
         {
-            buf_printf(&out, "interface=%d", tt->adapter_index );
+            buf_printf(&out, "interface=%lu", tt->adapter_index );
         }
         device = buf_bptr(&out);
 

@@ -2406,12 +2406,12 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, unsigned
         struct buffer out = alloc_buf_gc(64, &gc);
         if (r6->adapter_index)          /* vpn server special route */
         {
-            buf_printf(&out, "interface=%d", r6->adapter_index );
+            buf_printf(&out, "interface=%lu", r6->adapter_index );
             gateway_needed = true;
         }
         else
         {
-            buf_printf(&out, "interface=%d", tt->adapter_index );
+            buf_printf(&out, "interface=%lu", tt->adapter_index );
         }
         device = buf_bptr(&out);
 

@@ -2832,7 +2832,7 @@ get_default_gateway_ipv6(struct route_ipv6_gateway_info *rgi6,
         goto done;
     }
 
-    msg( D_ROUTE, "GDG6: II=%d DP=%s/%d NH=%s",
+    msg( D_ROUTE, "GDG6: II=%lu DP=%s/%u NH=%s",
          BestRoute.InterfaceIndex,
          print_in6_addr( BestRoute.DestinationPrefix.Prefix.Ipv6.sin6_addr, 0, &gc),
          BestRoute.DestinationPrefix.PrefixLength,

@@ -2993,7 +2993,7 @@ do_route_service(const bool add, const route_message_t *rt, const size_t size, H
 
     if (ack.error_number != NO_ERROR)
     {
-        msg(M_WARN, "ROUTE: route %s failed using service: %s [status=%u if_index=%lu]",
+        msg(M_WARN, "ROUTE: route %s failed using service: %s [status=%u if_index=%d]",
             (add ? "addition" : "deletion"), strerror_win32(ack.error_number, &gc),
             ack.error_number, rt->iface.index);
         goto out;
src/openvpn/socket.c +3 -3
index c1c0eaa..12085e6 100644
@@ -1119,7 +1119,7 @@ socket_do_accept(socket_descriptor_t sd,
 
     if (!socket_defined(new_sd))
     {
-        msg(D_LINK_ERRORS | M_ERRNO, "TCP: accept(%d) failed", sd);
+        msg(D_LINK_ERRORS | M_ERRNO, "TCP: accept(%"PRIuPTR") failed", sd);
     }
     /* only valid if we have remote_len_af!=0 */
     else if (remote_len_af && remote_len != remote_len_af)

@@ -1872,13 +1872,13 @@ phase2_inetd(struct link_socket *sock, const struct frame *frame,
             if (getsockname(sock->sd, &local_addr.addr.sa, &addrlen) == 0)
             {
                 sock->info.lsa->actual.dest.addr.sa.sa_family = local_addr.addr.sa.sa_family;
-                dmsg(D_SOCKET_DEBUG, "inetd(%s): using sa_family=%d from getsockname(%d)",
+                dmsg(D_SOCKET_DEBUG, "inetd(%s): using sa_family=%d from getsockname(%"PRIuPTR")",
                      proto2ascii(sock->info.proto, sock->info.af, false),
                      local_addr.addr.sa.sa_family, sock->sd);
             }
             else
             {
-                msg(M_WARN, "inetd(%s): getsockname(%d) failed, using AF_INET",
+                msg(M_WARN, "inetd(%s): getsockname(%"PRIuPTR") failed, using AF_INET",
                     proto2ascii(sock->info.proto, sock->info.af, false), sock->sd);
             }
         }
src/openvpn/tun.c +2 -2
index 3504fbf..0b13310 100644
@@ -125,7 +125,7 @@ do_address_service(const bool add, const short family, const struct tuntap *tt)
 
     if (ack.error_number != NO_ERROR)
     {
-        msg(M_WARN, "TUN: %s address failed using service: %s [status=%u if_index=%lu]",
+        msg(M_WARN, "TUN: %s address failed using service: %s [status=%u if_index=%d]",
             (add ? "adding" : "deleting"), strerror_win32(ack.error_number, &gc),
             ack.error_number, addr.iface.index);
         goto out;

@@ -3796,7 +3796,7 @@ get_panel_reg(struct gc_arena *gc)
 
             if (status != ERROR_SUCCESS || name_type != REG_SZ)
             {
-                dmsg(D_REGISTRY, "Error opening registry key: %s\\%s\\%s",
+                dmsg(D_REGISTRY, "Error opening registry key: %s\\%s\\%S",
                      NETWORK_CONNECTIONS_KEY, connection_string, name_string);
             }
             else
Signature · 1 line
2.7.4
author
#1 lore raw
Quoted 3 lines · Steffan Karger

A number of printf-like functions in windows-specific code used incorrect format specifiers, which could potentially lead to incorrect values being printed.

Hm, some of these are not windows-specific, and cause problems for non-windows builds. Please hold your breath for a V2.

-Steffan

author
#2 lore raw

PATCH v2 1/1 Fix windows-build format specifiers

A number of printf-like functions used incorrect format specifiers for Windows builds, which could potentially lead to incorrect values being printed / used when calling executables.

Signed-off-by Steffan Karger <[email protected]...>

---
v2: add OVPN_PRI_SKT define to print socket type across platforms

 src/openvpn/init.c    |  2 +-
 src/openvpn/route.c   | 12 ++++++------
 src/openvpn/socket.c  |  7 ++++---
 src/openvpn/syshead.h |  2 ++
 src/openvpn/tun.c     |  4 ++--
 5 files changed, 15 insertions(+), 12 deletions(-)
src/openvpn/init.c +1 -1
index 033318d..2a9a398 100644
@@ -1585,7 +1585,7 @@ do_open_tun(struct context *c)
 #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=%u", (unsigned int) c->options.msg_channel);
+    msg(D_ROUTE, "interactive service msg_channel=%"PRIuPTR, (uintptr_t) c->options.msg_channel);
 #endif
 
     /* allocate route list structure */
src/openvpn/route.c +6 -6
index fec1c25..25b5fce 100644
@@ -1967,12 +1967,12 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
         struct buffer out = alloc_buf_gc(64, &gc);
         if (r6->adapter_index)          /* vpn server special route */
         {
-            buf_printf(&out, "interface=%d", r6->adapter_index );
+            buf_printf(&out, "interface=%lu", r6->adapter_index );
             gateway_needed = true;
         }
         else
         {
-            buf_printf(&out, "interface=%d", tt->adapter_index );
+            buf_printf(&out, "interface=%lu", tt->adapter_index );
         }
         device = buf_bptr(&out);
 

@@ -2406,12 +2406,12 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, unsigned
         struct buffer out = alloc_buf_gc(64, &gc);
         if (r6->adapter_index)          /* vpn server special route */
         {
-            buf_printf(&out, "interface=%d", r6->adapter_index );
+            buf_printf(&out, "interface=%lu", r6->adapter_index );
             gateway_needed = true;
         }
         else
         {
-            buf_printf(&out, "interface=%d", tt->adapter_index );
+            buf_printf(&out, "interface=%lu", tt->adapter_index );
         }
         device = buf_bptr(&out);
 

@@ -2832,7 +2832,7 @@ get_default_gateway_ipv6(struct route_ipv6_gateway_info *rgi6,
         goto done;
     }
 
-    msg( D_ROUTE, "GDG6: II=%d DP=%s/%d NH=%s",
+    msg( D_ROUTE, "GDG6: II=%lu DP=%s/%u NH=%s",
          BestRoute.InterfaceIndex,
          print_in6_addr( BestRoute.DestinationPrefix.Prefix.Ipv6.sin6_addr, 0, &gc),
          BestRoute.DestinationPrefix.PrefixLength,

@@ -2993,7 +2993,7 @@ do_route_service(const bool add, const route_message_t *rt, const size_t size, H
 
     if (ack.error_number != NO_ERROR)
     {
-        msg(M_WARN, "ROUTE: route %s failed using service: %s [status=%u if_index=%lu]",
+        msg(M_WARN, "ROUTE: route %s failed using service: %s [status=%u if_index=%d]",
             (add ? "addition" : "deletion"), strerror_win32(ack.error_number, &gc),
             ack.error_number, rt->iface.index);
         goto out;
src/openvpn/socket.c +4 -3
index c1c0eaa..c854f90 100644
@@ -1119,7 +1119,7 @@ socket_do_accept(socket_descriptor_t sd,
 
     if (!socket_defined(new_sd))
     {
-        msg(D_LINK_ERRORS | M_ERRNO, "TCP: accept(%d) failed", sd);
+        msg(D_LINK_ERRORS | M_ERRNO, "TCP: accept(%"PRI_OVPN_SKT") failed", sd);
     }
     /* only valid if we have remote_len_af!=0 */
     else if (remote_len_af && remote_len != remote_len_af)

@@ -1872,13 +1872,14 @@ phase2_inetd(struct link_socket *sock, const struct frame *frame,
             if (getsockname(sock->sd, &local_addr.addr.sa, &addrlen) == 0)
             {
                 sock->info.lsa->actual.dest.addr.sa.sa_family = local_addr.addr.sa.sa_family;
-                dmsg(D_SOCKET_DEBUG, "inetd(%s): using sa_family=%d from getsockname(%d)",
+                dmsg(D_SOCKET_DEBUG,
+                     "inetd(%s): using sa_family=%d from getsockname(%"PRI_OVPN_SKT")",
                      proto2ascii(sock->info.proto, sock->info.af, false),
                      local_addr.addr.sa.sa_family, sock->sd);
             }
             else
             {
-                msg(M_WARN, "inetd(%s): getsockname(%d) failed, using AF_INET",
+                msg(M_WARN, "inetd(%s): getsockname(%"PRI_OVPN_SKT") failed, using AF_INET",
                     proto2ascii(sock->info.proto, sock->info.af, false), sock->sd);
             }
         }
src/openvpn/syshead.h +2 -0
index a1b6047..fcb9318 100644
@@ -482,9 +482,11 @@ typedef unsigned short sa_family_t;
 #ifdef _WIN32
 #define SOCKET_UNDEFINED (INVALID_SOCKET)
 typedef SOCKET socket_descriptor_t;
+#define PRI_OVPN_SKT PRIuPTR
 #else
 #define SOCKET_UNDEFINED (-1)
 typedef int socket_descriptor_t;
+#define PRI_OVPN_SKT "d"
 #endif
 
 static inline int
src/openvpn/tun.c +2 -2
index 3504fbf..0b13310 100644
@@ -125,7 +125,7 @@ do_address_service(const bool add, const short family, const struct tuntap *tt)
 
     if (ack.error_number != NO_ERROR)
     {
-        msg(M_WARN, "TUN: %s address failed using service: %s [status=%u if_index=%lu]",
+        msg(M_WARN, "TUN: %s address failed using service: %s [status=%u if_index=%d]",
             (add ? "adding" : "deleting"), strerror_win32(ack.error_number, &gc),
             ack.error_number, addr.iface.index);
         goto out;

@@ -3796,7 +3796,7 @@ get_panel_reg(struct gc_arena *gc)
 
             if (status != ERROR_SUCCESS || name_type != REG_SZ)
             {
-                dmsg(D_REGISTRY, "Error opening registry key: %s\\%s\\%s",
+                dmsg(D_REGISTRY, "Error opening registry key: %s\\%s\\%S",
                      NETWORK_CONNECTIONS_KEY, connection_string, name_string);
             }
             else
Signature · 1 line
2.7.4
#3 lore raw

Re: [Openvpn-devel] [PATCH v2] Fix windows-build format specifiers

Quoted 20 lines · Steffan Karger

A number of printf-like functions used incorrect format specifiers for Windows builds, which could potentially lead to incorrect values being printed / used when calling executables.

Signed-off-by Steffan Karger <[email protected]...>

---
v2: add OVPN_PRI_SKT define to print socket type across platforms

 src/openvpn/init.c    |  2 +-
 src/openvpn/route.c   | 12 ++++++------
 src/openvpn/socket.c  |  7 ++++---
 src/openvpn/syshead.h |  2 ++
 src/openvpn/tun.c     |  4 ++--
 5 files changed, 15 insertions(+), 12 deletions(-)
src/openvpn/init.c +0 -0
index 033318d..2a9a398 100644

src/openvpn/init.c do_open_tun(struct context *c) · line 1588

Quoted 5 lines
src/openvpn/init.c +1 -1
@@ -1585,7 +1585,7 @@ do_open_tun(struct context *c)
 #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=%u", (unsigned int) c->options.msg_channel);
+    msg(D_ROUTE, "interactive service msg_channel=%"PRIuPTR, (uintptr_t) c->options.msg_channel);

A little comment about the style (codestyle maniac here!). Should this:

msg_channel=%"PRIuPTR

be:

msg_channel=%" PRIuPTR

?

the " is closing the const char and therefore a space would be needed between it and the next constant.

This appears several times in the patch and therefore I thought it was worth mentioning it.

Cheers,

Signature · 1 line
Antonio Quartulli
author
#4 lore raw

Re: [Openvpn-devel] [PATCH v2] Fix windows-build format specifiers

Quoted 36 lines · Antonio Quartulli
Quoted 20 lines · Steffan Karger

A number of printf-like functions used incorrect format specifiers for Windows builds, which could potentially lead to incorrect values being printed / used when calling executables.

Signed-off-by Steffan Karger <[email protected]...>

---
v2: add OVPN_PRI_SKT define to print socket type across platforms

 src/openvpn/init.c    |  2 +-
 src/openvpn/route.c   | 12 ++++++------
 src/openvpn/socket.c  |  7 ++++---
 src/openvpn/syshead.h |  2 ++
 src/openvpn/tun.c     |  4 ++--
 5 files changed, 15 insertions(+), 12 deletions(-)
src/openvpn/init.c +0 -0
index 033318d..2a9a398 100644

src/openvpn/init.c do_open_tun(struct context *c) · line 1588

Quoted 5 lines
src/openvpn/init.c +1 -1
@@ -1585,7 +1585,7 @@ do_open_tun(struct context *c)
 #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=%u", (unsigned int) c->options.msg_channel);
+    msg(D_ROUTE, "interactive service msg_channel=%"PRIuPTR, (uintptr_t) c->options.msg_channel);

A little comment about the style (codestyle maniac here!). Should this:

msg_channel=%"PRIuPTR

be:

msg_channel=%" PRIuPTR

Quoted 7 lines

?

the " is closing the const char and therefore a space would be needed between it and the next constant.

This appears several times in the patch and therefore I thought it was worth mentioning it.

I don't have a strong preference about this. I tend to not add spaces for these preprocessor-format-specifiers, because I view them as part of the string. But I get your view too. So I'd be perfectly fine with adding spaces if that's the general preference.

-Steffan