Re: [HACKERS] snprintf causes regression tests to fail

Joerg Hessdoerfer <Joerg(dot)Hessdoerfer(at)sea-gmbh(dot)com> writes:
> Some stupid idea just crossed my mind: what if the /ports version just
> re-arranges the va_list according to the positional args and calls
> vsnprintf()?

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.

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.

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.

3. Rescan format string and do the outputting.

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.

regards, tom lane