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

Re: [HACKERS] snprintf causes regression tests to fail

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joerg Hessdoerfer <Joerg(dot)Hessdoerfer(at)sea-gmbh(dot)com>
Cc: "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,"Nicolai Tufar" <ntufar(at)gmail(dot)com>
Subject: Re: [HACKERS] snprintf causes regression tests to fail
Date: 2005-03-01 22:13:06
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackerspgsql-hackers-win32
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

In response to


pgsql-hackers by date

Next:From: Joshua D. DrakeDate: 2005-03-01 22:15:39
Subject: logging as inserts
Previous:From: Mark WongDate: 2005-03-01 22:06:42
Subject: Re: 8.0.X and the ARC patent

pgsql-hackers-win32 by date

Next:From: Nicolai TufarDate: 2005-03-01 22:26:37
Subject: Re: [pgsql-hackers-win32] snprintf causes regression
Previous:From: Nicolai TufarDate: 2005-03-01 21:59:10
Subject: Re: [HACKERS] snprintf causes regression tests to fail

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