Avoid format specifier %zu for Windows compatibility
PATCH 1/1 Avoid format specifier %zu for Windows compatibility
--- src/openvpn/crypto.c | 4 ++-- src/openvpn/options.c | 4 ++-- src/openvpn/ssl_mbedtls.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
src/openvpn/crypto.c +2 -2
@@ -738,8 +738,8 @@ crypto_adjust_frame_parameters(struct frame *frame,frame_add_to_extra_frame (frame, crypto_overhead); -msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %zu bytes" , - __func__, crypto_overhead); +msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %d bytes" , + __func__, (int ) crypto_overhead); }size_t
src/openvpn/options.c +2 -2
@@ -2992,7 +2992,7 @@ calc_options_string_link_mtu(const struct options *o, const struct frame *frame) o->replay, cipher_kt_mode_ofb_cfb (fake_kt.cipher));frame_finalize (&fake_frame, o->ce.link_mtu_defined, o->ce.link_mtu, o->ce.tun_mtu_defined, o->ce.tun_mtu); -msg (D_MTU_DEBUG ,"%s: link-mtu %zu -> %d" , __func__, link_mtu, +msg (D_MTU_DEBUG ,"%s: link-mtu %d -> %d" , __func__, (int ) link_mtu,EXPANDED_SIZE (&fake_frame)); link_mtu =EXPANDED_SIZE (&fake_frame); }
@@ -3061,7 +3061,7 @@ options_string (const struct options *o,*/ buf_printf (&out,",dev-type %s" ,dev_type_string (o->dev, o->dev_type)); -buf_printf (&out,",link-mtu %zu" ,calc_options_string_link_mtu (o, frame)); +buf_printf (&out,",link-mtu %d" , (int )calc_options_string_link_mtu (o, frame));buf_printf (&out,",tun-mtu %d" ,PAYLOAD_SIZE (frame));buf_printf (&out,",proto %s" ,proto_remote (o->ce.proto, remote));
src/openvpn/ssl_mbedtls.c +2 -2
@@ -1111,8 +1111,8 @@ print_details (struct key_state_ssl * ks_ssl, const char *prefix) cert =mbedtls_ssl_get_peer_cert (ks_ssl->ctx);if (cert !=NULL ) { -openvpn_snprintf (s2,sizeof (s2),", %zu bit key" , -mbedtls_pk_get_bitlen (&cert->pk)); +openvpn_snprintf (s2,sizeof (s2),", %d bit key" , + (int )mbedtls_pk_get_bitlen (&cert->pk)); }msg (D_HANDSHAKE ,"%s%s" , s1, s2);
Signature · 1 line
1.7.10.4
Hi,
src/openvpn/crypto.c crypto_adjust_frame_parameters(struct frame *frame, · line 742
Quoted 4 lines · Selva Nair
src/openvpn/crypto.c +2
-2-msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %zu bytes" , - __func__, crypto_overhead); +msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %d bytes" , + __func__, (int ) crypto_overhead);
Just for clarification: how does this break windows? mingw builds, or msvc builds? Will it print "nothing" ("zu bytes") or wrong values?
I had the suspicion that %zu wasn't as portable as Steffan hoped (and I seem to remember we've been there before :) ) but was too lazy to actually go out and research it.
(Going to apply later today, so "ACK!", just curious)
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Hi,
Quoted 6 lines · Gert Doering
src/openvpn/crypto.c crypto_adjust_frame_parameters(struct frame *frame, · line 742
Quoted 4 lines · Selva Nair
src/openvpn/crypto.c +2
-2-msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %zu bytes" , - __func__, crypto_overhead); +msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %d bytes" , + __func__, (int ) crypto_overhead);
Quoted 8 lines
Just for clarification: how does this break windows? mingw builds, or msvc builds? Will it print "nothing" ("zu bytes") or wrong values?
I had the suspicion that %zu wasn't as portable as Steffan hoped (and I seem to remember we've been there before :) ) but was too lazy to actually go out and research it.
(Going to apply later today, so "ACK!", just curious)
MSVC 2015 claims to support C99 now (took them 16 years!), and it should at least support %zu (which is part of C99). Shouldn't we just move (at least the master branch) to C99? Or are there good reasons to keep punishing ourselves with having to support C89?
-Steffan
Hi,
Quoted 4 lines · Steffan Karger
MSVC 2015 claims to support C99 now (took them 16 years!), and it should at least support %zu (which is part of C99). Shouldn't we just move (at least the master branch) to C99? Or are there good reasons to keep punishing ourselves with having to support C89?
Well, I'd assume that %zu is more a libc thing (as printf() format parsing happens at runtime) than what the C compiler understands - and if, say, Win7 libc.dll (or however it's called) doesn't grok %zu, we can't use it...
This is why I was asking what breaks exactly :-)
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 14 lines · Steffan Karger
Hi,
Quoted 2 lines · Gert Doering
Quoted 1 line · Selva Nair
- msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %zu
bytes",
Quoted 2 lines
Review
Quoted 2 lines
(quoted diff) +1
-1- __func__, crypto_overhead); + msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %dbytes",
Quoted 4 lines
Quoted 1 line
+
__func__, (int)crypto_overhead);Just for clarification: how does this break windows? mingw builds, or msvc builds? Will it print "nothing" ("zu bytes") or wrong values?
mingw (my version is gcc 4.6.3) prints zu bytes. I think this could be also fixed by defining __USE_MINGW_ANSI_STDIO=1
Quoted 5 lines
Quoted 5 lines
I had the suspicion that %zu wasn't as portable as Steffan hoped (and I seem to remember we've been there before :) ) but was too lazy to actually go out and research it.
Quoted 7 lines
Quoted 1 line
(Going to apply later today, so "ACK!", just curious)
MSVC 2015 claims to support C99 now (took them 16 years!), and it should at least support %zu (which is part of C99). Shouldn't we just move (at least the master branch) to C99? Or are there good reasons to keep punishing ourselves with having to support C89?
I am totally in support of deprecating arcane MSVC (pre 2015?) support. MS way of printing size_t has been to use "%Iu" (with an upper case i, not 'ell'), which indicates the required run-time support is there. But I am not sure whether MSVC 2015 translates zu to Iu or only works on platforms with a new runtime library.
Selva
Quoted 10 lines · Selva Nair
Quoted 9 lines · Steffan Karger
Quoted 8 lines · Gert Doering
Quoted 6 lines · Selva Nair
- msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %zu bytes",
(quoted diff) +1
-1- __func__, crypto_overhead); + msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %dbytes",
+__func__, (int)crypto_overhead);
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
Just for clarification: how does this break windows? mingw builds, or msvc builds? Will it print "nothing" ("zu bytes") or wrong values?
mingw (my version is gcc 4.6.3) prints zu bytes. I think this could be also fixed by defining __USE_MINGW_ANSI_STDIO=1
I looked into this a bit, and it should indeed fix the printing of %zu on Windows. I would prefer to teach autotools to include this flag on Windows builds, instead of the suggested casts in the code. (But if you both prefer the casts, just ignore my rambling.)
-Steffan
Hi,
Quoted 16 lines · Steffan Karger
Quoted 1 line · Selva Nair
On Fri, Aug 5, 2016 at 3:32 AM, Steffan Karger <[email protected]...>
Quoted 1 line · wrote:
Quoted 1 line
On Fri, Aug 5, 2016 at 8:14 AM, Gert Doering <[email protected]...>
Quoted 11 lines · wrote:
Quoted 10 lines
Quoted 10 lines · Selva Nair
Quoted 6 lines
- msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %zu bytes",
(quoted diff) +1
-1- __func__, crypto_overhead); + msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %dbytes",
+__func__, (int)crypto_overhead);Just for clarification: how does this break windows? mingw builds, or msvc builds? Will it print "nothing" ("zu bytes") or wrong values?
Quoted 9 lines
Quoted 1 line
mingw (my version is gcc 4.6.3) prints zu bytes. I think this could be
also
Quoted 1 line
fixed by defining __USE_MINGW_ANSI_STDIO=1
I looked into this a bit, and it should indeed fix the printing of %zu on Windows. I would prefer to teach autotools to include this flag on Windows builds, instead of the suggested casts in the code. (But if you both prefer the casts, just ignore my rambling.)
I don't like casts and agree with Steffan. That is, assuming MSVC support is only for developers, we need not worry about the runtime and could require MSVC 2015. I suppose we only distribute mingw cross-compiled binaries.
I think, with __USE_MINGW_ANSI_STDIO=1 mingw will use a custom implementation I/O routines, so the binary size may increase a bit (by ~50 kB or ~5% in my tests).
Also we currently use MS-specific format specs like %I64u %I64x for counter_format and ptr_format. Those will have to be changed once the above define is in place. Its only a couple of instances, though.
I am looking at this as an opportunity to stop coding for VS2011 and 2013 and feel liberated :) If Gert is fine with this I'll make a new patch.
Selva
Quoted 41 lines · Selva Nair
Hi,
Quoted 18 lines · Steffan Karger
Quoted 1 line · Selva Nair
On Fri, Aug 5, 2016 at 3:32 AM, Steffan Karger <[email protected]...>
Quoted 1 line · wrote:
Quoted 1 line
On Fri, Aug 5, 2016 at 8:14 AM, Gert Doering <[email protected]...>
Quoted 2 lines · wrote:
Quoted 2 lines
Quoted 2 lines · Selva Nair
Quoted 1 line
- msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by
%zu
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
bytes",
(quoted diff) +1
-1- __func__, crypto_overhead); + msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %dbytes",
+__func__, (int)crypto_overhead);Just for clarification: how does this break windows? mingw builds,
or
Quoted 2 lines
Quoted 1 line
Quoted 1 line
msvc builds? Will it print "nothing" ("zu bytes") or wrong values?
Quoted 9 lines
Quoted 1 line
mingw (my version is gcc 4.6.3) prints zu bytes. I think this could be
also
Quoted 1 line
fixed by defining __USE_MINGW_ANSI_STDIO=1
I looked into this a bit, and it should indeed fix the printing of %zu on Windows. I would prefer to teach autotools to include this flag on Windows builds, instead of the suggested casts in the code. (But if you both prefer the casts, just ignore my rambling.)
I don't like casts and agree with Steffan. That is, assuming MSVC support is only for developers, we need not worry about the runtime and could require MSVC 2015. I suppose we only distribute mingw cross-compiled binaries.
I think, with __USE_MINGW_ANSI_STDIO=1 mingw will use a custom implementation I/O routines, so the binary size may increase a bit (by ~50 kB or ~5% in my tests).
Quoted 7 lines
Also we currently use MS-specific format specs like %I64u %I64x for
counter_formatandptr_format. Those will have to be changed once the above define is in place. Its only a couple of instances, though.I am looking at this as an opportunity to stop coding for VS2011 and 2013 and feel liberated :) If Gert is fine with this I'll make a new patch.
So finally, what is the consensus on this?
Selva
Hi,
sorry for being a bit lazy in answering. This is the sort of threads that lead to endless wars ... :-)
Quoted 10 lines · Selva Nair
Quoted 4 lines
I looked into this a bit, and it should indeed fix the printing of %zu on Windows. I would prefer to teach autotools to include this flag on Windows builds, instead of the suggested casts in the code. (But if you both prefer the casts, just ignore my rambling.)
I don't like casts and agree with Steffan. That is, assuming MSVC support is only for developers, we need not worry about the runtime and could require MSVC 2015. I suppose we only distribute mingw cross-compiled binaries.
Quoted 7 lines
I think, with __USE_MINGW_ANSI_STDIO=1 mingw will use a custom implementation I/O routines, so the binary size may increase a bit (by ~50 kB or ~5% in my tests).
Also we currently use MS-specific format specs like %I64u %I64x for
counter_formatandptr_format. Those will have to be changed once the above define is in place. Its only a couple of instances, though.
Mmmh. Trading off 3 ("easy") casts vs. 50 kB extra code for stdio instead of using the platform-provided stdio library *plus* having to change (and possibly review) quite a few other parts of the code sounds like a fairly easy trade-off to me - and my conclusion would be "drop %zu, just cast to (unsigned int)".
Just to make this clear: I do not like sprinkling casts across the code "to silence the compiler" at all. It quite often hides bugs (especially when dealing with pointers).
Now *these* are all very simple - talking about numbers in the numerical range "0..2000", so "%u" plus (unsigned int) would be totally safe here, no matter what platform used.
Quoted 2 lines
I am looking at this as an opportunity to stop coding for VS2011 and 2013 and feel liberated :) If Gert is fine with this I'll make a new patch.
This is somewhat tangential from not using %zu :) - I think regarding "which VS versions do we support", we should at least check with James and Lev, as these two seem to be the last ones to (regularily) build with MSVC...
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 3 lines · Gert Doering
sorry for being a bit lazy in answering. This is the sort of threads that lead to endless wars ... :-)
Oh, no, this one is not worth even a fist fight, forget wars :)
Quoted 19 lines · Selva Nair
Quoted 5 lines
Quoted 4 lines
I looked into this a bit, and it should indeed fix the printing of %zu on Windows. I would prefer to teach autotools to include this flag on Windows builds, instead of the suggested casts in the code. (But if you both prefer the casts, just ignore my rambling.)
Quoted 7 lines
I don't like casts and agree with Steffan. That is, assuming MSVC support is only for developers, we need not worry about the runtime and could require MSVC 2015. I suppose we only distribute mingw cross-compiled binaries.
I think, with __USE_MINGW_ANSI_STDIO=1 mingw will use a custom implementation I/O routines, so the binary size may increase a bit (by
~50
Quoted 4 lines
kB or ~5% in my tests).
Also we currently use MS-specific format specs like %I64u %I64x for
counter_formatandptr_format. Those will have to be changed once the
Quoted 10 lines
above
Quoted 1 line
define is in place. Its only a couple of instances, though.
Mmmh. Trading off 3 ("easy") casts vs. 50 kB extra code for stdio instead of using the platform-provided stdio library *plus* having to change (and possibly review) quite a few other parts of the code sounds like a fairly easy trade-off to me - and my conclusion would be "drop %zu, just cast to (unsigned int)".
So, in hindsight, that "full disclosure" about 50kB + some lines was not in the best interests
of the case I was trying to make. Anyway... :)
Quoted 8 lines
Quoted 2 lines
I am looking at this as an opportunity to stop coding for VS2011 and 2013 and feel liberated :) If Gert is fine with this I'll make a new patch.
This is somewhat tangential from not using %zu :) - I think regarding "which VS versions do we support", we should at least check with James and Lev, as these two seem to be the last ones to (regularily) build with MSVC...
I'll submit a new patch.
Selva
PATCH 1/1 Avoid format specifier %zu for Windows compatibility
In reply to Selva Nair- Replace %zu by %u and cast the size_t variable to (unsigned int). The cast should be safe as in all instances the number involved is small.
Note: mingw64 targets msvcrt.dll runtime that doesn't support %zu and
print "zu" instead of the number. With -Wformat the compiler does warn that z is an unknown conversion type.
v2: Cast to (unsigned int) instead of (int).
--- src/openvpn/crypto.c | 4 ++-- src/openvpn/options.c | 4 ++-- src/openvpn/ssl_mbedtls.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
src/openvpn/crypto.c +2 -2
@@ -738,8 +738,8 @@ crypto_adjust_frame_parameters(struct frame *frame,frame_add_to_extra_frame (frame, crypto_overhead); -msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %zu bytes" , - __func__, crypto_overhead); +msg (D_MTU_DEBUG ,"%s: Adjusting frame parameters for crypto by %u bytes" , + __func__, (unsigned int ) crypto_overhead); }size_t
src/openvpn/options.c +2 -2
@@ -2992,7 +2992,7 @@ calc_options_string_link_mtu(const struct options *o, const struct frame *frame) o->replay, cipher_kt_mode_ofb_cfb (fake_kt.cipher));frame_finalize (&fake_frame, o->ce.link_mtu_defined, o->ce.link_mtu, o->ce.tun_mtu_defined, o->ce.tun_mtu); -msg (D_MTU_DEBUG ,"%s: link-mtu %zu -> %d" , __func__, link_mtu, +msg (D_MTU_DEBUG ,"%s: link-mtu %u -> %d" , __func__, (unsigned int ) link_mtu,EXPANDED_SIZE (&fake_frame)); link_mtu =EXPANDED_SIZE (&fake_frame); }
@@ -3061,7 +3061,7 @@ options_string (const struct options *o,*/ buf_printf (&out,",dev-type %s" ,dev_type_string (o->dev, o->dev_type)); -buf_printf (&out,",link-mtu %zu" ,calc_options_string_link_mtu (o, frame)); +buf_printf (&out,",link-mtu %u" , (unsigned int )calc_options_string_link_mtu (o, frame));buf_printf (&out,",tun-mtu %d" ,PAYLOAD_SIZE (frame));buf_printf (&out,",proto %s" ,proto_remote (o->ce.proto, remote));
src/openvpn/ssl_mbedtls.c +2 -2
@@ -1111,8 +1111,8 @@ print_details (struct key_state_ssl * ks_ssl, const char *prefix) cert =mbedtls_ssl_get_peer_cert (ks_ssl->ctx);if (cert !=NULL ) { -openvpn_snprintf (s2,sizeof (s2),", %zu bit key" , -mbedtls_pk_get_bitlen (&cert->pk)); +openvpn_snprintf (s2,sizeof (s2),", %u bit key" , + (unsigned int )mbedtls_pk_get_bitlen (&cert->pk)); }msg (D_HANDSHAKE ,"%s%s" , s1, s2);
Signature · 1 line
1.7.10.4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 8 lines · Selva Nair
- Replace %zu by %u and cast the
size_tvariable to (unsigned int). The cast should be safe as in all instances the number involved is small.Note: mingw64 targets msvcrt.dll runtime that doesn't support %zu and print "zu" instead of the number. With -Wformat the compiler does warn that z is an unknown conversion type.
Quoted 6 lines
v2: Cast to (unsigned int) instead of (int).
Signed-off-by Selva Nair <[email protected]...> ---
src/openvpn/crypto.c| 4 ++--src/openvpn/options.c|
4 ++--src/openvpn/ssl_mbedtls.c| 4 ++-- 3 files changed, 6
insertions(+), 6 deletions(-)
ACK.
I've double checked that all places converted from %zu to %u are do only the size_t type (which on at least Linux map to unsigned int - sorry, don't have Windows handy). But given that these values are mostly MTU, crypto packet overhead length or crypto key length related, they shouldn't easily overrun (that would be quite a special network packet or crypto key).
Just done a simple build on my Linux box (again, sorry, no cross compiler handy) ... but, last famous words: This change looks sane and safe.
I'll need to update my git ack-am scripts slightly now that gmane web is currently dead. But I'll commit it asap.
- --
kind regards,
David Sommerseth
OpenVPN Technologies, Inc
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iEYEARECAAYFAlet4U4ACgkQDC186MBRfrpohwCfS6auj2tZvjunstoCp9unaUDc OKgAni/nqugO2abrUHLaZrqeHGlx8O95
=DYTu
-----END PGP SIGNATURE-----
Your patch has been applied to the master branch.
commit fff2b444ae8344c063ff819074a355e0a5aa47df
Author: Selva Nair Date: Thu Aug 11 18:58:29 2016 -0400
Avoid format specifier %zu for Windows compatibility
Signed-off-by: Selva Nair <[email protected]...>
Acked-by: David Sommerseth <[email protected]...>
Message-Id: <[email protected]...>
Signed-off-by: David Sommerseth <[email protected]...>--
kind regards,
David Sommerseth
Hi,
Quoted 6 lines · David Sommerseth
Your patch has been applied to the master branch.
commit
fff2b444ae8344c063ff819074a355e0a5aa47dfAuthor: Selva Nair Date: Thu Aug 11 18:58:29 2016 -0400
Quoted 8 lines
Avoid format specifier %zu for Windows compatibility
Signed-off-by: Selva Nair <[email protected]...>
Acked-by: David Sommerseth <[email protected]...>
Message-Id: <[email protected]...>
Signed-off-by: David Sommerseth <[email protected]...>Built snapshot on Linux .. Run on W10p server4+6
Appears to work fine
(some other errors .. presume to be mine)
regards