Re: [HACKERS] snprintf() argument reordering not working under Windows

Nicolai Tufar wrote:
> Greetings,
>
> Last April we have made some changes to src/ports/snprintf.c so that it
> would support argument reordering like %2$s, %1$d and such on
> platforms where original snprintf() does not support it, like Windows,
> HP-UX or NetBSD.

Sure, I remember. So glad you returned at this time. I found a design
limitation in that file yesterday. It can not output more then 4096
characters, and there are some cases with NUMERIC that try to output
more than that. For example:

SELECT pow(10::numeric, 10000) + 1;

should show a '1' at the end of the number, but with the existing code
you will just see 4095 0's and no more.

I am attaching the new snprintf.c and the patch itself for your review.
The change is to pass 'stream' down into the routines and output to the
FILE* right from inside the routine, rather than using a string. This
fixes the problem.

I am also thinking of modifying the code so if we are using snprintf.c
only because we need positional parameter control, we check for '$' in
the string and only use snprintf.c in those cases.

> NLS messages of some languages, like Turkish, rely heavily on argument
> reordering because of the language structure. In 8.1 Turkish messages
> in Windows version are all broken because argument reordering is not there.

Really? I have not heard any report of that but this is new code in 8.1.

> I examined commit logs and came to conclusion that src/port/snprintf.c
> is not included in server when compiling under Windows because of change
> to src/port/Makefile made in revision 1.28 by Bruce Momjian. See here:
>
> http://developer.postgresql.org/cvsweb.cgi/pgsql/src/port/Makefile
>
> Comment to the commit says:
> `No server version of snprintf needed, so remove Makefile rule.'
> In fact I think we need snprintf in server because NLS messages are
> printed by the server.

Actually, that changes means that there was nothing backend-specific in
snprintf.c so we don't need a _special_ version for the backend. The
real change not to use snprintf.c on Win32 is in configure.in with this
comment:

# Force use of our snprintf if system's doesn't do arg control
# This feature is used by NLS
if test "$enable_nls" = yes &&
test $pgac_need_repl_snprintf = no &&
# On Win32, libintl replaces snprintf() with its own version that
# understands arg control, so we don't need our own. In fact, it
# also uses macros that conflict with ours, so we _can't_ use
# our own.
test "$PORTNAME" != "win32"; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
pgac_need_repl_snprintf=yes
fi
fi

Here is the commit:

revision 1.409
date: 2005/05/05 19:15:54; author: momjian; state: Exp; lines: +8 -2
On Win32, libintl replaces snprintf() with its own version that
understands arg control, so we don't need our own. In fact, it
also uses macros that conflict with ours, so we _can't_ use
our own.

So, I think it was Magnus who said that Win32 didn' need and couldn't
use our snprintf. Magnus, any ideas?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073