Re: vasnprintf.c %n blocked under MSVS8

[Simon Josefsson]

Bruno Haible <address@hidden> writes:

> Hi Simon,
>
>> Hi!  I received a patch for vasnprintf with rationale:
>
> From whom, please? I would like to acknowledge the original reporter.

Hi!  It was Adam Strzelecki <address@hidden>, in:

http://article.gmane.org/gmane.comp.gnu.gsasl.general/122

>> >   - 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.

Yes.

> 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.

Thanks.  I think your patch is slightly better, since it fixes mingw
too.

>> 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.

Agreed.  In retrospect, my concern was because of the _MSC_VER test --
such a check is a compile-time version-based check, which doesn't handle
the situation when the binary is built on one host and then transferred
to another host with older/newer libraries.  Your patch, however, is a
compile-time but not version-based check, which seems more appropriate.

So, please install, although the attribution should probably be Adam
rather than me.

Thanks,
Simon