MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
PATCH 1/2 MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
In MSVC2015 the behavior of vsnprintf was changed. W/o this fix there is one character missing at the end.
--- compat/snprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
compat/snprintf.c +1 -1
@@ -9,7 +9,7 @@* always have room for a trailing NUL byte. */ #ifndef SNPRINTF_SIZE_CORR -#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) +#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) && (!defined(_MSC_VER) || _MSC_VER <1900 )#define SNPRINTF_SIZE_CORR 1#else #define SNPRINTF_SIZE_CORR 0
Signature · 1 line
2.7.4.windows.1
Quoted 5 lines · Sven Strickroth
In MSVC2015 the behavior of vsnprintf was changed. W/o this fix there is one character missing at the end.
Signed-off-by Sven Strickroth <[email protected]>---
Thanks.
I am not qualified to judge the correctness of the assertion that MSVC at or more recent than version 1900 does not need the correction and will wait for Windows folks to Ack.
Thanks.
Quoted 16 lines
compat/snprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)compat/snprintf.c +1
-1@@ -9,7 +9,7 @@* always have room for a trailing NUL byte. */ #ifndef SNPRINTF_SIZE_CORR -#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) +#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) && (!defined(_MSC_VER) || _MSC_VER <1900 )#define SNPRINTF_SIZE_CORR 1#else #define SNPRINTF_SIZE_CORR 0
Quoted 2 lines · Sven Strickroth
In MSVC2015 the behavior of vsnprintf was changed. W/o this fix there is one character missing at the end.
How about adding a link to [1] in the commit message and quoting the central "Beginning with the UCRT in Visual Studio 2015 and Windows 10, vsnprintf is no longer identical to _vsnprintf. The vsnprintf function complies with the C99 standard; _vnsprintf is retained for backward compatibility" statement?
[1] https://msdn.microsoft.com/en-us/library/1kt27hek.aspx
Signature · 1 line
Sebastian Schuberth
"Beginning with the UCRT in Visual Studio 2015 and Windows 10, vsnprintf is no longer identical to _vsnprintf. The vsnprintf function complies with the C99 standard; _vnsprintf is retained for backward compatibility" [1]
W/o this fix there is one character missing at the end.
[1] https://msdn.microsoft.com/en-us/library/1kt27hek.aspx
--- compat/snprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
compat/snprintf.c +1 -1
@@ -9,7 +9,7 @@* always have room for a trailing NUL byte. */ #ifndef SNPRINTF_SIZE_CORR -#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) +#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) && (!defined(_MSC_VER) || _MSC_VER <1900 )#define SNPRINTF_SIZE_CORR 1#else #define SNPRINTF_SIZE_CORR 0
Signature · 1 line
2.7.4.windows.1
compat/snprintf.c line 15
Quoted 13 lines · Sven Strickroth
compat/snprintf.c +1
-1@@ -9,7 +9,7 @@* always have room for a trailing NUL byte. */ #ifndef SNPRINTF_SIZE_CORR -#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) +#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) && (!defined(_MSC_VER) || _MSC_VER <1900 )#define SNPRINTF_SIZE_CORR 1#else #define SNPRINTF_SIZE_CORR 0
I wonder if the logic is (and was) sensible here. We assume that every non-__GNUC__ and non-_MSC_VER compiler on Windows requires the correction. Wouldn't it make sense to not assume requiring the correction unless we know the compiler has this bug? That is, shouldn't this better say
#if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) || (defined(_MSC_VER) && _MSC_VER < 1900))
#define SNPRINTF_SIZE_CORR 1
#else
#define SNPRINTF_SIZE_CORR 0
Signature · 1 line
Sebastian Schuberth
Hi Sven & Sebastian,
Quoted 1 line · Sebastian Schuberth
On Tue, Mar 29, 2016 at 9:13 PM, Sven Strickroth <[email protected]> wrote:
ACK on the patch.
Quoted 20 lines
compat/snprintf.c line 15
Quoted 13 lines
compat/snprintf.c +1
-1@@ -9,7 +9,7 @@* always have room for a trailing NUL byte. */ #ifndef SNPRINTF_SIZE_CORR -#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) +#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) && (!defined(_MSC_VER) || _MSC_VER <1900 )#define SNPRINTF_SIZE_CORR 1#else #define SNPRINTF_SIZE_CORR 0I wonder if the logic is (and was) sensible here. We assume that every non-__GNUC__ and non-_MSC_VER compiler on Windows requires the correction. Wouldn't it make sense to not assume requiring the correction unless we know the compiler has this bug? That is, shouldn't this better say
Quoted 5 lines
#if defined(WIN32) && (
defined(__GNUC__)&&__GNUC__< 4) || (defined(_MSC_VER)&&_MSC_VER< 1900))
#defineSNPRINTF_SIZE_CORR1
#else
#defineSNPRINTF_SIZE_CORR0
Since the standard on Windows always was MS Visual C, it should be assumed that compilers *other* than GCC followed Visual C's lead.
Of course, evidence speaks louder than assumptions.
Therefore I would prefer to keep the current version, at least until we encounter a case where it is incorrect.
Thanks,
Johannes
Quoted 19 lines · Johannes Schindelin
Quoted 13 lines
compat/snprintf.c line 15
Quoted 6 lines
#ifndef
SNPRINTF_SIZE_CORRcompat/snprintf.c +1
-1-#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) +#if defined(WIN32 ) && (!defined(__GNUC__) || __GNUC__ <4 ) && (!defined(_MSC_VER) || _MSC_VER <1900 )#define SNPRINTF_SIZE_CORR 1#else #define SNPRINTF_SIZE_CORR 0I wonder if the logic is (and was) sensible here. We assume that every non-__GNUC__ and non-_MSC_VER compiler on Windows requires the correction. Wouldn't it make sense to not assume requiring the correction unless we know the compiler has this bug? That is, shouldn't this better say
Quoted 5 lines
#if defined(WIN32) && (
defined(__GNUC__)&&__GNUC__< 4) || (defined(_MSC_VER)&&_MSC_VER< 1900))
#defineSNPRINTF_SIZE_CORR1
#else
#defineSNPRINTF_SIZE_CORR0
Quoted 7 lines
Since the standard on Windows always was MS Visual C, it should be assumed that compilers *other* than GCC followed Visual C's lead.
Of course, evidence speaks louder than assumptions.
Therefore I would prefer to keep the current version, at least until we encounter a case where it is incorrect.
Fine with me. It's probably better not to change the logic as we wouldn't know whether this would break things for some exotic compiler currently in use to compile Git.
Also ACK from my side on the path then.
Signature · 1 line
Sebastian Schuberth