Re: [HACKERS] snprintf causes regression tests to fail - Mailing list pgsql-hackers-win32

> Having looked at the current snprintf.c, I don't actually believe that
> it works at all in the presence of positional parameter specs.  It picks
> up the arguments in the order they are mentioned in the format string,
> which is exactly not what it's supposed to do, if I understand the
> concept of positional specifiers properly.  This is certain to give the
> wrong answers when the arguments are of different widths.

It picks up arguments in order of appearance, places them in
array then shuffles them according to %n$ positional parameter.
I checked it with in many different combinations, it works!

> I'm also less than thrilled with the 300K+ local array ;-).  I don't see
> any point in saving the width specs from the first pass to the second
> when they are not needed in the first pass.  NL_ARGMAX could have a more
> sane value (surely not more than a couple hundred) and we need some
> checks that it's not exceeded in any case.  On the other side of the
> coin, the hardwired 4K limit in printf() is certainly *not* enough.

How would one solve this issue. Calling malloc() from a print function
would be rather expensive. Maybe call snprintf() from printf() and
see if resulting string is 4K long then allocate a new buffer and
call snprintf() again? And by the way, what should I use malloc()
or palloc()?

What would you think will be a good value for NL_ARGMAX?

> I think a correct implementation is probably a three-pass affair:
>
> 1. Scan format string to determine the basic datatypes (int, float, char*,
> etc) of each actual parameter and their positions.  Note that runtime
> width and precision specs have to be counted as actual parameters.
>
> 2. Perform the va_arg() fetches in position order, and stash the actual
> parameter values into a local array.

I actually do it. But in one step. I left the scanning loop in place
but replaced calls to actual printing functions with code to fill
the array, preserving width and precision. Plus I store pointers
to beginning and endings of format placeholders to not to have
to scan format string again in the next step.

> 3. Rescan format string and do the outputting.

My version: reorder the array and do the outputting.
/* shuffle pointers */ comment in source is misleading,
it should have been /* reorder pointers */.

> I don't offhand see what is making the regression tests fail (it's not
> the above because the problem occurs with a nonpositional format string);
> but there's not much point in trying to get rid of porting issues when
> the fundamental algorithm is wrong.

I believe my algorithm is working and it is faster than your proposed algorithm.
But if it is not acceptable I am willing to change as deemed necessary. I tested
the function separately and it passed regression tests on many platforms.
Situation with win32 is really unusual. We may need a win32 and MinGG internals
guru to have a look a the issue.

>                       regards, tom lane
Nicolai Tufar