4 hour countdown

Sorry for such a late submission.

I just downloaded the latest postgresql-8.0.0-rc5-3.zip installer
for windows and it appears that Windows' printf() does not
support placeholder replacement as described in
http://developer.postgresql.org/docs/postgres/nls.html#AEN57284

I searched list archives but found no explanation for this.
Original string is <<%s at or near \"%s\">> and we
replaced it in Turkish for <<\"%2$s\" yerinde %1$s>>.

It works perfectly fine on Linux but fails miserably under
Windows with something like:
<<HATA: "$s" yerinde $sat character 1>>
while on Linux the same version works fine:
<<HATA: "sdfassfsdfasd" yerinde söz dizim hatasý at character 1 >>

What is it? I suppose that Windows' printf() does
not support changing the order of placeholder
characters with %2$ .... %1$. Did some one else
deal with it? Is it because of compilation flags?

In any case PostgreSQL version in current pginstaller
is broken.

Best regards,
Nicolai

unix/win32 libc doesnt support "$n" variables..

On Mon, 17 Jan 2005 21:03:56 +0200, Nicolai Tufar <ntufar@gmail.com> wrote:

Sorry for such a late submission.

I just downloaded the latest postgresql-8.0.0-rc5-3.zip installer
for windows and it appears that Windows' printf() does not
support placeholder replacement as described in
http://developer.postgresql.org/docs/postgres/nls.html#AEN57284

I searched list archives but found no explanation for this.
Original string is <<%s at or near \"%s\">> and we
replaced it in Turkish for <<\"%2$s\" yerinde %1$s>>.

It works perfectly fine on Linux but fails miserably under
Windows with something like:
<<HATA: "$s" yerinde $sat character 1>>
while on Linux the same version works fine:
<<HATA: "sdfassfsdfasd" yerinde söz dizim hatasý at character 1 >>

What is it? I suppose that Windows' printf() does
not support changing the order of placeholder
characters with %2$ .... %1$. Did some one else
deal with it? Is it because of compilation flags?

In any case PostgreSQL version in current pginstaller
is broken.

Best regards,
Nicolai

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

--
/* Jumping Flowers */

On Mon, 17 Jan 2005 16:17:33 -0300, Fx <weimaraner@gmail.com> wrote:

unix/win32 libc doesnt support "$n" variables..

What can we do then?

Nicolai Tufar <ntufar@gmail.com> writes:

Sorry for such a late submission.
I just downloaded the latest postgresql-8.0.0-rc5-3.zip installer
for windows and it appears that Windows' printf() does not
support placeholder replacement as described in
http://developer.postgresql.org/docs/postgres/nls.html#AEN57284

Original string is <<%s at or near \"%s\">> and we
replaced it in Turkish for <<\"%2$s\" yerinde %1$s>>.

Hmm. Looking around, it seems that %n$ support is required by the
Single Unix Spec:
http://www.opengroup.org/onlinepubs/007908799/xsh/fprintf.html
but it is *not* required by C99 as far as I can tell. I don't see any
mention of support for it in my HPUX fprintf man page, either. So this
construct may not be as portable as we could wish.

There appear to be about 150 affected messages, in these files:

src/backend/po/pt_BR.po
src/backend/po/de.po
src/backend/po/es.po
src/backend/po/zh_CN.po
src/backend/po/tr.po
src/bin/pg_dump/po/zh_CN.po
src/bin/pg_dump/po/tr.po
src/bin/psql/po/zh_CN.po
src/bin/psql/po/zh_TW.po
src/bin/psql/po/tr.po
src/bin/scripts/po/zh_CN.po

I don't think we'll hold up release to fix this, but the affected
translators may want to think about whether they can avoid the problem
or not.

regards, tom lane

I wrote:

I don't think we'll hold up release to fix this, but the affected
translators may want to think about whether they can avoid the problem
or not.

Also, it looks like src/port/snprintf.c is not %n$ capable either.
I'm not sure which platforms that affects.

A possible route to a solution is to upgrade snprintf.c and then use
it on platforms that don't have this support. This only fixes those
cases where we go through snprintf, which is probably not all of the
affected messages, but it might be enough. It's not happening for
8.0.0 though.

regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On Mon, 17 Jan 2005, Tom Lane wrote:

<snip>

I don't think we'll hold up release to fix this,

:-) It seems nothing seems to stop you from holding up this release
anymore: Neither ARC problem nor this one :)

Regards,
- --
Devrim GUNDUZ
devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.tdmsoft.com http://www.gunduz.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQFB7B0Ktl86P3SPfQ4RAqCFAJ9YLdaUP8ALAetKHQcpPxHfrlcXcQCgpKmX
gKsYQYdu8nh4rGQOo2DQl3c=
=g6q/
-----END PGP SIGNATURE-----

On Mon, 17 Jan 2005 14:54:44 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Also, it looks like src/port/snprintf.c is not %n$ capable either.
I'm not sure which platforms that affects.

A possible route to a solution is to upgrade snprintf.c and then use
it on platforms that don't have this support. This only fixes those
cases where we go through snprintf, which is probably not all of the
affected messages, but it might be enough. It's not happening for
8.0.0 though.

Bad news for the Turks. Turkish language syntax
is lake Latin: the verb is always at the end. Because
of that almost all messages that have two or more
placeholders are broken under Windows.

The solution you propose seem to be a trivial exercise:
1. write a replacement function, say, pg_snprintf() that
would shuffle input arguments, replace %x$ with %
and then call libc's snprintf();
2. replace snprintf() in all source files with pg_snprintf();
3. Update Makefiles to use our custom function on broken
platforms or default to snprinf(). (Or maybe pg_snprintf()
should be a #define);
4. Tell everyone to use pg_snprintf() instead of snprintf()
from now on.

I would like volunteer to implement this for 8.1 and
would be honoured if I get a chance to contribute.

regards, tom lane

Best regards,
Nicolai Tufar

I don't think we'll hold up release to fix this, but the affected
translators may want to think about whether they can avoid

the problem

or not.

Also, it looks like src/port/snprintf.c is not %n$ capable either.
I'm not sure which platforms that affects.

A possible route to a solution is to upgrade snprintf.c and then use
it on platforms that don't have this support. This only fixes those
cases where we go through snprintf, which is probably not all of the
affected messages, but it might be enough. It's not happening for
8.0.0 though.

Might want to track down which platforms are affected by the file in
port/, and then add win32 to it, and put it up somewhere on a list of
known issues in 8.0?

//Magnus

Greetings,
for a couple of days I have been hacking on src/port/snprintf.c.
With Magnus' help I have managed to implement argument replacement
in snprintf(). The code is very crude and not quite optimised,
any suggestions will be more than welcome.

Here is what I did:

1. I renamed snprintf() to pg_snprintf(), vsnprintf() to pg_vsnprintf()
and introduced pg_printf() that calls pg_vsnprintf().

2. After running configure I manually added snprintf.o to src/Makefile.global's
LIBOBJ declaration and -lpgport to Makefile.shlib's DLLWRAP declaration

3. To make sure these functions are used everywhere I introduced the following
lines at the end of src/include/c.h:

#define snprintf pg_snprintf
#define vsnprintf pg_vsnprintf
#define printf pg_printf

4. I introduced a volatile static char[] variable in snprintf.c code so I can
grep executables for this string and be sure that it is included.

5. Before running regression test I always ran make install, apparently because
libpq is read from /usr/local/.

During compilation the following warnings were reported:
../../../src/include/utils/elog.h:121: warning: `pg_printf' is an
unrecognized format function type

which is perfectly fine because we replace printf with pg_printf and
gcc's format() does
know anything about it.

On Linux, PostgreSQL passed regression tests with flying colours and
prints messages
with %n$ just fine. On win32: int8, timestamp, timestamptz, abstime, horology,
constraints, vacuum, and many others failed. To check my code, I
reverted snprintf.c
to the original one from CVS and forced win32 port to use these
functions and it
fails in same places. After examining regression tests diff I came to
conclusion that problem is in fmtnum() function when it operates with
particularly
long integers.

In snprintf() file I changed only and only dopr() function, neve touching
fmtnum(), fmtstr() or fmtfolat().

I would like to kindly ask these questions:

1. Am I on the right to implement %n$ ? Can it be accepted?
2. Why would we not just take FreeBSD's vfprintf() and use it instead?
3. What is wrong with fmtnum() on Win32?
4. What %m format string is used for? And where is it handled. Do I
need to implement it?

I am attaching my version if snprintf.c (because it is too different
from the original to
make a patch) and regression.diff of a failed Win32 regression test
produced wither with
my or with original snprintf.c.

Best regards,
Nicolai Tufar

Nicolai Tufar wrote:

1. I renamed snprintf() to pg_snprintf(), vsnprintf() to
pg_vsnprintf() and introduced pg_printf() that calls pg_vsnprintf().

This is not necessary. You will notice that we already have configure
tests of snprintf format specifiers (%lld etc.), so using the original
function names is OK even in that case.

4. I introduced a volatile static char[] variable in snprintf.c code
so I can grep executables for this string and be sure that it is
included.

Remove that when finalizing the code.

5. Before running regression test I always ran make install,
apparently because libpq is read from /usr/local/.

That's because of the -rpath.

2. Why would we not just take FreeBSD's vfprintf() and use it
instead?

Try it. It's painful.

4. What %m format string is used for? And where is it handled. Do I
need to implement it?

It's only used in the error reporting functions in the server and is
handled there. You don't need to worry about it.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

On Sat, 22 Jan 2005 15:31:39 +0100, Peter Eisentraut <peter_e@gmx.net> wrote:

Nicolai Tufar wrote:

1. I renamed snprintf() to pg_snprintf(), vsnprintf() to
pg_vsnprintf() and introduced pg_printf() that calls pg_vsnprintf().

This is not necessary. You will notice that we already have configure
tests of snprintf format specifiers (%lld etc.), so using the original
function names is OK even in that case.

how about a test for the thing like:
printf("%2$s %1$s!\n", "world", "Hello");
It is what I am trying to solve :(

5. Before running regression test I always ran make install,
apparently because libpq is read from /usr/local/.

That's because of the -rpath.

I see. Thanks for information.

2. Why would we not just take FreeBSD's vfprintf() and use it
instead?

Try it. It's painful.

src/port/snprintf.c is not not a particularly pretty. And
after my rework it got even uglier. I have looked at
FreeBDS's one and it is not *that* complicated. Would
you like me to try to incorporate into PostgreSQL.
NetBSD's one is somewhat simplier but does not
support %n$ feature by the way.

4. What %m format string is used for? And where is it handled. Do I
need to implement it?

It's only used in the error reporting functions in the server and is
handled there. You don't need to worry about it.

Oh, that is a relief.

Peter Eisentraut

Nicolai

Nicolai Tufar <ntufar@gmail.com> writes:

On Sat, 22 Jan 2005 15:31:39 +0100, Peter Eisentraut <peter_e@gmx.net> wrote:

Nicolai Tufar wrote:

2. Why would we not just take FreeBSD's vfprintf() and use it
instead?

Try it. It's painful.

src/port/snprintf.c is not not a particularly pretty. And
after my rework it got even uglier. I have looked at
FreeBDS's one and it is not *that* complicated.

If you can get it to work then it'd be a better bet than writing (and
having to maintain) our own.

regards, tom lane

On Sat, 22 Jan 2005 17:05:22 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote:

src/port/snprintf.c is not not a particularly pretty. And
after my rework it got even uglier. I have looked at
FreeBDS's one and it is not *that* complicated.

If you can get it to work then it'd be a better bet than writing (and
having to maintain) our own.

Very well, I am starting to work on it.
I will put everything necessary in on file.
So far it is 1500-odd lines and will probably
grow a little bit. Plus we would probably need to
include Double-to-ASCII package too:
http://cvsup.pt.freebsd.org/cgi-bin/cvsweb/cvsweb.cgi/src/contrib/gdtoa/
A daunting task indeed. I will do it but I am afraid
it would not be too portable.

I was wandering if reimplementing print formatting
is the right thing to do. The problem I have is
in fmtnum() and probably in fmtfloat() functions
which are very platform-dependent.

The code to shuffle xxprinf() arguments based on
%n$ formatting stirngs is ready why don't shuffle
formatting placeholders too and call OS's vsnprintf()
with modified formatting formatting strings and
va_list?

Or, a similar solution, for every parameter
extract formatting placeholder and value,
call snprintf() and the combine the resulting string?

The first solution requires doing nasty manipulations
with va_list, the second will be slower because of
multiple calls to snprintf().

Which one is better?

Best regards,
Nicolai Tufar

Greetings,

I would like to submit a new version of src/port/snprintf.c
It passes regression tests on Linux and Win32 and
prints all of %n$ messages finely.

I added printf() function too because --help usage-type
output is printed with printf().

I have no experience with autoconf so I would ask you
for help on what changes to do to include libpgport to
src/Makefile.global and src/Makefile.shlib.

PostgreSQL uses snprintf() in more places than I expected.
So these changes need a through testing. I have no 64-bit
to able to run regression test on it would be nice to run
it on a 64-bit platform too.

Best regards,
Nicolai Tufar