Re: [HACKERS] snprintf causes regression tests to fail
От | Nicolai Tufar |
---|---|
Тема | Re: [HACKERS] snprintf causes regression tests to fail |
Дата | |
Msg-id | d80929390503011457274e0849@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] snprintf causes regression tests to fail (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: [HACKERS] snprintf causes regression tests to fail
(Tom Lane <tgl@sss.pgh.pa.us>)
|
Список | 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
В списке pgsql-hackers-win32 по дате отправления: