Hi Simon, > Hi! I received a patch for vasnprintf with rationale:
From whom, please? I would like to acknowledge the original reporter. > > - fixes vasnprintf for MSVC8 or higher that has also %n blocked I understand that it depends on the MSVCRT.DLL, that is, it will also happen with mingw. But essentially, the patch is right. I verified that it does not cause testsuite failures, and added some comments mentioning why this change is right. Patch appended below. My earlier statement that nothing needs to be done in gnulib <http://lists.gnu.org/archive/html/bug-gnulib/2007-06/msg00066.html> was wrong; I had not understood that %n makes a program crash on Vista or that mingw has snprintf(). > Looking at this, I'm not sure I understand the intention. Why isn't a > configure test used to determine whether %n works in snprintf or not (if > that is indeed what is relevant), rather than version-based checks? Ad-hoc tests are used here for two reasons: 1) autoconf tests are ok when the user wants to run the built executables on the build machine only. But on Woe32 and Linux systems, users often want to run the executables also on newer versions of the OS. And here it's the newer versions of the OS which introduce the bug. (Whereas usually, with autoconf, you test for a bug that exists in old versions only.) So, if we build binaries on Windows XP, we need to take into account the bug that Windows Vista has introduced. 2) Here the logic of autoconf tests would be far more complicated than the ad-hoc #if tests. Looking at the matrix in m4/printf.m4, there are 4 types of systems on which gl_SNPRINTF_DIRECTIVE_N fails, and they are treated differently: - glibc with FORTIFY. Here we exploit the fact that gl_SNPRINTF_TRUNCATION_C99 and gl_SNPRINTF_RETVAL_C99 succeed. - mingw. Here we exploit the fact that gl_SNPRINTF_TRUNCATION_C99 and gl_SNPRINTF_RETVAL_C99 "nearly" succeed: snprintf does not overwrite more memory than allowed, and its return value allows to know when the buffer was not large enough. (There are also snprintf implementations which overwrite memory, and others where the return value does not indicate an insufficient buffer size.) - HP-UX. Here we exploit the fact that gl_SNPRINTF_TRUNCATION_C99 succeeds and gl_SNPRINTF_RETVAL_C99 "nearly" succeeds. - Solaris 2.5.1 and OSF/1 4.0. Here the system has no snprintf() at all, hence vasnprintf() computes an upper bound on the needed memory size and uses sprintf. Adding autoconf tests for this case would IMO increase the complexity even more, rather than reducing it. 2008-02-07 Bruno Haible <[EMAIL PROTECTED]> * lib/vasnprintf.c (VASNPRINTF): Don't use %n on native Woe32 systems. Avoids a crash on Windows Vista. Reported by Simon Josefsson <[EMAIL PROTECTED]>. *** lib/vasnprintf.c.orig 2008-02-08 01:57:43.000000000 +0100 --- lib/vasnprintf.c 2008-02-08 01:55:39.000000000 +0100 *************** *** 4010,4016 **** #endif *fbp = dp->conversion; #if USE_SNPRINTF ! # if !(__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3)) fbp[1] = '%'; fbp[2] = 'n'; fbp[3] = '\0'; --- 4010,4016 ---- #endif *fbp = dp->conversion; #if USE_SNPRINTF ! # if !(__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3) || ((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)) fbp[1] = '%'; fbp[2] = 'n'; fbp[3] = '\0'; *************** *** 4023,4028 **** --- 4023,4043 ---- in format strings in writable memory may crash the program (if compiled with _FORTIFY_SOURCE=2), so we should avoid it in this situation. */ + /* On native Win32 systems (such as mingw), we can avoid using + %n because: + - Although the gl_SNPRINTF_TRUNCATION_C99 test fails, + snprintf does not write more than the specified number + of bytes. (snprintf (buf, 3, "%d %d", 4567, 89) writes + '4', '5', '6' into buf, not '4', '5', '\0'.) + - Although the gl_SNPRINTF_RETVAL_C99 test fails, snprintf + allows us to recognize the case of an insufficient + buffer size: it returns -1 in this case. + On native Win32 systems (such as mingw) where the OS is + Windows Vista, the use of %n in format strings by default + crashes the program. See + <http://gcc.gnu.org/ml/gcc/2007-06/msg00122.html> and + <http://msdn2.microsoft.com/en-us/library/ms175782(VS.80).aspx> + So we should avoid %n in this situation. */ fbp[1] = '\0'; # endif #else
