Skip site navigation (1) Skip section navigation (2)

Re: [HACKERS] snprintf causes regression tests to fail

From: Nicolai Tufar <ntufar(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Joerg Hessdoerfer <Joerg(dot)Hessdoerfer(at)sea-gmbh(dot)com>,Magnus Hagander <mha(at)sollentuna(dot)net>,Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>,pgsql-hackers-win32(at)postgresql(dot)org
Subject: Re: [HACKERS] snprintf causes regression tests to fail
Date: 2005-03-01 22:57:46
Message-ID: d80929390503011457274e0849@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-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

In response to

Responses

pgsql-hackers by date

Next:From: Nicolai TufarDate: 2005-03-01 23:09:41
Subject: Re: [pgsql-hackers-win32] snprintf causes regression tests to fail
Previous:From: Tom LaneDate: 2005-03-01 22:45:31
Subject: Re: [pgsql-hackers-win32] snprintf causes regression tests to fail

pgsql-hackers-win32 by date

Next:From: Nicolai TufarDate: 2005-03-01 23:09:41
Subject: Re: [pgsql-hackers-win32] snprintf causes regression tests to fail
Previous:From: Tom LaneDate: 2005-03-01 22:45:31
Subject: Re: [pgsql-hackers-win32] snprintf causes regression tests to fail

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group