Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Wed, 16 Mar 2005 01:00:21 -0500 (EST), Bruce Momjian wrote: > > I have applied a modified version of your patch, attached. I am so sorry, I sent untested patch again. Thank you very much for patience in fixing it. The patch looks perfectly fine and works under Solaris. Under win32 I am still struggling with build environment. In many directories link fails with "undefined reference to `pg_snprintf'" in other it fails with "undefined reference to `_imp__libintl_sprintf'". In yet another directory it fails with both, like in src/interfaces/ecpg/pgtypeslib: dlltool --export-all --output-def pgtypes.def numeric.o datetime.o common.o dt_common.o timestamp.o interval.o pgstrcasecmp.o dllwrap -o libpgtypes.dll --dllname libpgtypes.dll --def pgtypes.def numeric.o datetime.o common.o dt_common.o timestamp.o interval.o pgstrcasecmp.o -L../../../../src/port -lm numeric.o(.text+0x19ea):numeric.c: undefined reference to `_imp__libintl_sprintf' datetime.o(.text+0x476):datetime.c: undefined reference to `pg_snprintf' common.o(.text+0x1cd):common.c: undefined reference to `pg_snprintf' common.o(.text+0x251):common.c: undefined reference to `pg_snprintf' dt_common.o(.text+0x538):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x553):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x597):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x5d5):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x628):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x7e8):dt_common.c: more undefined references to `_imp__libintl_sprintf' follow c:\MinGW\bin\dllwrap.exe: c:\MinGW\bin\gcc exited with status 1 make: *** [libpgtypes.a] Error 1 Could someone with a better grasp of configure and win32 environment check it? Aparently no one regularily compiles source code under win32 during development cycle these days. Best regards, Nicolai ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > Is there no way to create a va_arg va_list structure in C? Exactly. The standard lets you *read out* from such a structure, but there's no provision for creating one on-the-fly. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > I am wondering if we should just process long long args and %$ args, > and pass everything else to the native snprintf. AFAICS this is a non-starter --- how will you construct the call to snprintf? Or even vsnprintf? C doesn't provide the tools you need to make it happen. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Resubmission of yesterday's patch so that it would
cont conflict with Bruce's cvs commit. Pleas apply.
Best regards,
Nicolai.
On Sat, 12 Mar 2005 01:58:15 +0200, Nicolai Tufar <[EMAIL PROTECTED]> wrote:
> On Thu, 10 Mar 2005 19:21:41 -0500 (EST), Bruce Momjian
> wrote:
> > > The CVS-tip implementation is fundamentally broken and won't work even
> > > for our internal uses. I've not wasted time complaining about it
> > > because I thought we were going to replace it. If we can't find a
> > > usable replacement then we're going to have to put a lot of effort
> > > into fixing what's there. On the whole I think the effort would be
> > > better spent importing someone else's solution.
> >
> > Oh, so our existing implementation doesn't even meet our needs. OK.
>
> Which made me wander why did I not aggree with
> Tom Lane's suggestion to make do three passes
> instead of two. Tom was right, as usual. It happened to
> be much easier than I expected. The patch is attached.
> Please apply.
>
> Tom, what do you think? Will it be fine with you?
>
> Best regards,
> Nicolai
>
>
>
*** ./src/port/snprintf.c.orig Sat Mar 12 09:13:43 2005
--- ./src/port/snprintf.c Sat Mar 12 10:01:44 2005
***
*** 195,200
--- 195,202
int pointflag;
char func;
int realpos;
+ int longflag;
+ int longlongflag;
} *fmtpar, **fmtparptr;
/* Create enough structures to hold all arguments */
***
*** 264,275
--- 266,279
realpos = position;
len = 0;
goto nextch;
+ /*
case '*':
if (pointflag)
maxwidth = va_arg(args, int);
else
len = va_arg(args, int);
goto nextch;
+ */
case '.':
pointflag = 1;
goto nextch;
***
*** 301,316
#endif
case 'u':
case 'U':
! /* fmtnum(value,base,dosign,ljust,len,zpad,&output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 305,312
#endif
case 'u':
case 'U':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 325,340
break;
case 'o':
case 'O':
! /* fmtnum(value,base,dosign,ljust,len,zpad,&output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 321,328
break;
case 'o':
case 'O':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 349,365
break;
case 'd':
case 'D':
! if (longflag)
! {
! if (longlongflag)
! {
! value = va_arg(args, int64);
! }
! else
! value = va_arg(args, long);
! }
! else
! value = va_arg(args, int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 337,344
break;
case 'd':
case 'D':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 373,387
fmtpos++;
break;
case 'x':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 352,359
fmtpos++;
break;
case 'x':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 395,409
fmtpos++;
break;
case 'X':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, u
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Thu, 10 Mar 2005 19:21:41 -0500 (EST), Bruce Momjian
wrote:
> > The CVS-tip implementation is fundamentally broken and won't work even
> > for our internal uses. I've not wasted time complaining about it
> > because I thought we were going to replace it. If we can't find a
> > usable replacement then we're going to have to put a lot of effort
> > into fixing what's there. On the whole I think the effort would be
> > better spent importing someone else's solution.
>
> Oh, so our existing implementation doesn't even meet our needs. OK.
Which made me wander why did I not aggree with
Tom Lane's suggestion to make do three passes
instead of two. Tom was right, as usual. It happened to
be much easier than I expected. The patch is attached.
Please apply.
Tom, what do you think? Will it be fine with you?
Best regards,
Nicolai
*** ./src/port/snprintf.c.orig Sat Mar 12 01:28:49 2005
--- ./src/port/snprintf.c Sat Mar 12 01:08:30 2005
***
*** 195,200
--- 195,202
int pointflag;
char func;
int realpos;
+ int longflag;
+ int longlongflag;
} *fmtpar, **fmtparptr;
/* Create enough structures to hold all arguments */
***
*** 263,274
--- 265,278
realpos = position;
len = 0;
goto nextch;
+ /*
case '*':
if (pointflag)
maxwidth = va_arg(args, int);
else
len = va_arg(args, int);
goto nextch;
+ */
case '.':
pointflag = 1;
goto nextch;
***
*** 300,315
#endif
case 'u':
case 'U':
! /* fmtnum(value,base,dosign,ljust,len,zpad,&output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 304,311
#endif
case 'u':
case 'U':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 324,339
break;
case 'o':
case 'O':
! /* fmtnum(value,base,dosign,ljust,len,zpad,&output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 320,327
break;
case 'o':
case 'O':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 348,364
break;
case 'd':
case 'D':
! if (longflag)
! {
! if (longlongflag)
! {
! value = va_arg(args, int64);
! }
! else
! value = va_arg(args, long);
! }
! else
! value = va_arg(args, int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 336,343
break;
case 'd':
case 'D':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 372,386
fmtpos++;
break;
case 'x':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 351,358
fmtpos++;
break;
case 'x':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 394,408
fmtpos++;
break;
case 'X':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 366,373
fmtpos++;
b
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Fri, 11 Mar 2005 01:14:31 -0500, Tom Lane wrote: > Nicolai Tufar <[EMAIL PROTECTED]> writes: > > Very well, I too, tend to think that importing some else's solution > > is the way to go. Tom, have you looked at Trio? > > I have not seen it ... but if it looks reasonable to you, have a go > at it. It looks reasonable to me. It claims to: fully implement C99 (ISO/IEC 9899:1999) and UNIX98 (the Single Unix Specification, Version 2) standards, as well as many features from other implementations, e.g. the GNU libc and BSD4. I compiled and run regression tests with it used instead of our current implementation and it worked fine under win32 and Solaris x86. The only problem is that it is 11000 lines as oposed to our curent implementation's 600. I will remove everything unnecessary and submit a patch for consideration. Regards, Nicolai Tufar ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Nicolai Tufar <[EMAIL PROTECTED]> writes: > Very well, I too, tend to think that importing some else's solution > is the way to go. Tom, have you looked at Trio? I have not seen it ... but if it looks reasonable to you, have a go at it. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Tom Lane wrote: > The CVS-tip implementation is fundamentally broken and won't work even > for our internal uses. I've not wasted time complaining about it > because I thought we were going to replace it. If we can't find a > usable replacement then we're going to have to put a lot of effort > into fixing what's there. On the whole I think the effort would be > better spent importing someone else's solution. Bruce Momjian wrote: > Oh, so our existing implementation doesn't even meet our needs. OK. Very well, I too, tend to think that importing some else's solution is the way to go. Tom, have you looked at Trio? If it is fine with you too, I will strip it to the bare minimum needed for snprintf(), vsnprintf() and printf() by Saturday. Regards, Nicolai Tufar ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > Please see my posting about using a macro for snprintf. If the current > implementation of snprintf is enough for our existing translation users > we probably don't need to add anything more to it because snprintf will > not be exported to client applications. The CVS-tip implementation is fundamentally broken and won't work even for our internal uses. I've not wasted time complaining about it because I thought we were going to replace it. If we can't find a usable replacement then we're going to have to put a lot of effort into fixing what's there. On the whole I think the effort would be better spent importing someone else's solution. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Wed, 9 Mar 2005 22:51:27 -0500 (EST), Bruce Momjian wrote: > > What do you think about it? Shall I abandon FreeBSD and go ahead > > Incorporating Trio? > > Yes, maybe just add the proper %$ handling from Trio to what we have > now. Adding proper %$ from Trio will require too much effort. I would rather not do it. Not because I am lazy but because: 1) Trio team seem to be very serious about standards, update the library as soon as new standards come out: Trio fully implements the C99 (ISO/IEC 9899:1999) and UNIX98 (the Single Unix Specification, Version 2) standards, as well as many features from other implementations, e.g. the GNU libc and BSD4. 2) If we integrate the whole library in source code we will not have to maintain it and will rely on Trio team for bug fixes and updates. Integrating it will be very easy since all of the functions begin with "trio_". I used it instead of the src/port/snrpintf.c one and it passes regression tests under Win32 just fine. The downside is that Trio library is rather big. It is 3 .c and 6 .h files totalling 11556 lines. Compiled it is 71224 bytes not stripped and 56204 bytes stripped on Solaris 10 for x86, 32-bit. Even for a shared library it will probably be too much. Trio has a lot of string handling functions which are probably not necessary. Would you like me to try to remove everything unnecessary from it or we will settle with the full version? Regards, Nicolai Tufar ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Dear all, After struggling for one week to to integrate FreeBSD's vfprintf.c into PostgreSQL I finally gave up. It is too dependent on underlying FreeBSD system functions. To incorporate it into PostgreSQL we need to move vfprintf.c file itself, two dozen files form gdtoa and a half a dozen __XXtoa.c files scattered in apparently random fashion all around FreeBSD source tree. Instead I researched some other implementations of snprintf on the web released under a license compatible with PostgreSQL's. The most suitable one I have come upon is Trio [http://daniel.haxx.se/projects/trio/]. It is distributed under a MIT-like license which, I think will be compatible with us. What do you think about it? Shall I abandon FreeBSD and go ahead ÄncorporatÄng TrÄo? And by the way, what Äs the conclusÄon of snprÄntf() vs. pg_snprintf() and UNIX libraries discussion a week ago? Which one shall I implement? Regards, Nicolai Tufar ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > Tom Lane wrote: >> First line of thought: we surely must not insert a snprintf into >> libpq.so unless it is 100% up to spec *and* has no performance issues >> ... neither of which can be claimed of the CVS-tip version. > Agreed, and we have to support all the 64-bit specifications a port > might support like %qd and %I64d as well as %lld. I have added that to > our current CVS version. I really dislike that idea and request that you revert it. > Is there any way we can have just gettext() call our snprintf under a > special name? The issue only comes up in libpq --- in the backend there is no reason that snprintf can't be our snprintf, and likewise in self-contained programs like psql. It might be worth the pain-in-the-neck quality to have libpq refer to the functions as pq_snprintf etc. Perhaps we could do this via macros #define snprintfpq_snprintf and not have to uglify the source code. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] snprintf causes regression tests to fail
> > The big question is why our own vsnprintf() is not being called from > snprintf() in our port file. > I have seen this "problem" before, well, it isn't really a problem I guess. I'm not sure of the gcc compiler options, but On the Microsoft compiler if you specify the option "/Gy" it separates the functions within the object file, that way you don't load all the functions from the object if they are not needed. If, however, you create a function with the same name as another function, and one is declared in an object compiled with the "/Gy" option, and the other's object file is not, then if you also use a different function or reference variable in the object file compiled without the "/Gy" option, then the conflicting function will probably be used. Make sense? I would suggest using macro to redefine snprintf and vnsprintf to avoid the issue: #define snprintf pg_snprintf #define vnsprintf pg_vnsprintf ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Wed, 2 Mar 2005 09:24:32 +0100, Joerg Hessdoerfer <[EMAIL PROTECTED]> wrote: > don't know if PG borrowed the code from here, but according to the manpage > FreeBSD 5.3 seems to have a quite complete implementation, see > http://www.freebsd.org/cgi/man.cgi?query=snprintf&apropos=0&sektion=3&manpath=FreeBSD+5.3-RELEASE+and+Ports&format=html > > Would this help? Yes, it would. With Tom Lane's blessing I am starting on incorporating it into PG now. ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] snprintf causes regression tests to fail
Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian writes: > > > Tom Lane wrote: > > >> Does it help if you flip the order of the snprintf and vsnprintf > > >> functions in snprintf.c? > > > > > Yes, it fixes the problem and I have applied the reordering with a > > > comment. > > > > Fascinating. > > > > > I will start working on fixing the large fmtpar allocations now. > > > > Quite honestly, this code is not worth micro-optimizing because it > > is fundamentally broken. See my other comments in this thread. > > I am working on something that just counts the '%' characters in the > format string and allocates an array that size. > > > Can we find a BSD-license implementation that follows the spec? > > I would think NetBSD would be our best bet. I will download it and take > a look. Oops, I remember now that NetBSD doesn't support it. I think FreeBSD does. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] snprintf causes regression tests to fail
Tom Lane wrote: > Bruce Momjian writes: > > Tom Lane wrote: > >> Does it help if you flip the order of the snprintf and vsnprintf > >> functions in snprintf.c? > > > Yes, it fixes the problem and I have applied the reordering with a > > comment. > > Fascinating. > > > I will start working on fixing the large fmtpar allocations now. > > Quite honestly, this code is not worth micro-optimizing because it > is fundamentally broken. See my other comments in this thread. I am working on something that just counts the '%' characters in the format string and allocates an array that size. > Can we find a BSD-license implementation that follows the spec? I would think NetBSD would be our best bet. I will download it and take a look. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > Tom Lane wrote: >> Does it help if you flip the order of the snprintf and vsnprintf >> functions in snprintf.c? > Yes, it fixes the problem and I have applied the reordering with a > comment. Fascinating. > I will start working on fixing the large fmtpar allocations now. Quite honestly, this code is not worth micro-optimizing because it is fundamentally broken. See my other comments in this thread. Can we find a BSD-license implementation that follows the spec? regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] snprintf causes regression tests to fail
Tom Lane wrote: > Bruce Momjian writes: > > I think this means it is finding our /port/snprintf(), but when it calls > > vsnprintf, it must be using some other version, probably the operating > > system version that doesn't support %lld. > > Ya know, I was wondering about that but dismissed it because the > routines were all declared in the same file. Windows' linker must > behave very oddly to do this. > > Does it help if you flip the order of the snprintf and vsnprintf > functions in snprintf.c? Yes, it fixes the problem and I have applied the reordering with a comment. I will start working on fixing the large fmtpar allocations now. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] snprintf causes regression tests to fail
Tom Lane wrote: > Bruce Momjian writes: > > I think this means it is finding our /port/snprintf(), but when it calls > > vsnprintf, it must be using some other version, probably the operating > > system version that doesn't support %lld. I can confirm that using "%I64d" for the printf format allows the regression tests to pass for int8. > Ya know, I was wondering about that but dismissed it because the > routines were all declared in the same file. Windows' linker must > behave very oddly to do this. Yep, quite unusual. > Does it help if you flip the order of the snprintf and vsnprintf > functions in snprintf.c? Testing now. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > I think this means it is finding our /port/snprintf(), but when it calls > vsnprintf, it must be using some other version, probably the operating > system version that doesn't support %lld. Ya know, I was wondering about that but dismissed it because the routines were all declared in the same file. Windows' linker must behave very oddly to do this. Does it help if you flip the order of the snprintf and vsnprintf functions in snprintf.c? regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] snprintf causes regression tests to fail
I have some new information. If I add the attached patch to snprintf.c,
I should see see snprintf() being called printing "0", vsnprintf()
printing "1" and dopr(), "2". However, I only see "0" printing in the
server logs.
I think this means it is finding our /port/snprintf(), but when it calls
vsnprintf, it must be using some other version, probably the operating
system version that doesn't support %lld.
I am also attaching the 'nm' output from libpgport_srv.a which does show
vsnprintf as being defined.
Win32 doesn't like multiply defined symbols so we use
-Wl,--allow-multiple-definition to allow multiple symbols.
I bet if I define LONG_LONG_INT_FORMAT as '%I64d' it would pass the
regression tests. (I will test now.) Our config/c-library.m4 file
confirms that format for MinGW:
# MinGW uses '%I64d', though gcc throws an warning with -Wall,
The big question is why our own vsnprintf() is not being called from
snprintf() in our port file.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
*** snprintf.c Tue Mar 1 19:02:13 2005
--- /laptop/tmp/snprintf.c Tue Mar 1 20:27:21 2005
***
*** 96,101
--- 96,102
int len;
va_list args;
+ puts("0");
va_start(args, fmt);
len = vsnprintf(str, count, fmt, args);
va_end(args);
***
*** 109,114
--- 110,116
char *end;
str[0] = '\0';
end = str + count - 1;
+ puts("1");
dopr(str, fmt, args, end);
if (count > 0)
end[0] = '\0';
***
*** 178,183
--- 180,186
int realpos;
} fmtpar[NL_ARGMAX+1], *fmtparptr[NL_ARGMAX+1];
+ puts("2");
format_save = format;
output = buffer;
crypt.o:
b .bss
d .data
t .text
0060 b _a64toi
2ce0 b _CF6464
03c0 t _CIFP
34e0 b _constdatablock
0441 T _crypt
34f0 b _cryptresult
0760 t _des_cipher
d _des_ready
06c2 t _des_setkey
00c0 t _ExpandTr
18e0 b _IE3264
0a84 t _init_des
0f0f t _init_perm
0080 t _IP
0400 t _itoa64
3510 b _KS
03a0 t _P32Tr
0100 t _PC1
00e0 b _PC1ROT
0160 t _PC2
08e0 b _PC2ROT
b _perm.0
t _permute
0138 t _Rotates
01a0 t _S
1ce0 b _SPE
0040 b _tmp32.1
fseeko.o:
b .bss
d .data
t .text
getrusage.o:
b .bss
d .data
t .text
U ___udivdi3
U ___umoddi3
U __dosmaperr
U __errno
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
T _getrusage
inet_aton.o:
b .bss
d .data
t .text
U __impmb_cur_max
U __imp___pctype
U __isctype
U [EMAIL PROTECTED]
T _inet_aton
random.o:
b .bss
d .data
t .text
U _lrand48
T _random
srandom.o:
b .bss
d .data
t .text
U _srand48
T _srandom
unsetenv.o:
b .bss
d .data
t .text
U _getenv
U _malloc
U _putenv
U _sprintf
0004 T _unsetenv
getaddrinfo_srv.o:
b .bss
d .data
t .text
U _atoi
U _free
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
U _inet_aton
U [EMAIL PROTECTED]
U _malloc
U _memcpy
U [EMAIL PROTECTED]
025a T _pg_freeaddrinfo
02ca T _pg_gai_strerror
T _pg_getaddrinfo
02f6 T _pg_getnameinfo
U _snprintf
U [EMAIL PROTECTED]
copydir.o:
b .bss
d .data
t .text
t ___func__.0
U _AllocateDir
00a5 T _copydir
U [EMAIL PROTECTED]
U _errcode_for_file_access
U _errfinish
U _errmsg
U _errstart
U _FreeDir
U _mkdir
U _readdir
U _snprintf
gettimeofday.o:
b .bss
d .data
t .text
U ___udivdi3
t _epoch
U [EMAIL PROTECTED]
0008 T _gettimeofday
U [EMAIL PROTECTED]
kill.o:
b .bss
d .data
t .text
U __errno
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
0015 T _pgkill
U _wsprintfA
open.o:
b .bss
d .data
t .text
U __assert
U __errno
U __open_osfhandle
U __setmode
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
U [EMAIL PROTECTED]
t _openFlagsToCreateFileFlags
0160 T _win32_open
rand.o:
b .bss
d .data
t .text
t __dorand4
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
[EMAIL PROTECTED] writes: > Well, here is a stupid question, do we even know which snprintf we are > using on Win32? May it be possible that we are using the MingW version > which may be broken? The regression tests were not failing until we started using the port/ version ... regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Tue, 1 Mar 2005 15:38:58 -0500 (EST), [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > Is there a reason why we don't use the snprintf that comes with the > various C compilers? snprintf() is usually buried in OS libraries. We implement our own snprintf to make things like this: snprintf(buf,"%2$s %1$s","world","Hello"); which is not supported on some platforms work. We do it for national language translation of messages. In some languages you may need to change order of parameters to make a meaningful sentence. Another question is why we are using it for printing values from database. I am not too good on function overriding in standard C but maybe there is a way to usage of standard snprintf() in a particular place. ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
>> I spent all day debugging it. Still have absolutely >> no idea what could possibly go wrong. Does >> anyone have a slightest clue what can it be and >> why it manifests itself only on win32? > >It may be that the CLIB has badly broken support for 64bit >integers on 32 >bit platforms. Does anyone know of any Cygwin/Ming issues? > >Is this only with the new snprintf code in Win32? Yes. >Is this a problem with snprintf as implemented in src/port? Yes. Only. It works with the snprintf() in the runtime (this particular part). >Is there a reason why we don't use the snprintf that comes with the >various C compilers? It does not support "positional parameters" (I think it's called) which is required for proper translations. We do use that one when it works... //Magnus ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
I spent all day debugging it. Still have absolutely no idea what could possibly go wrong. Does anyone have a slightest clue what can it be and why it manifests itself only on win32? On Tue, 1 Mar 2005 09:29:07 -0500 (EST), Bruce Momjian wrote: > Nicolai Tufar wrote: > > On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian > > > My next guess > > > is that Win32 isn't handling va_arg(..., long long int) properly. > > > > > > > I am trying various combination of number and types > > of parameters in my test program and everything prints fine. > > When it comes to pg, it fails :( > > > > > > template1=# select * from test where x > 1000::int8; > > > >x > > > > > > > >-869367531 > > > > (1 row) > > > > I am not too fluent in source code, could someone > > point me to there actual call to snprintf() is being done > > when a query like this is executed. I could not find it myslef > > Sure, in src/backend/utils/adt/int8.c, there is a call in int8out(): > > if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0) > > and that calls port/snprintf.c. > > I have added a puts() in snprintf.c to make sure it is getting the > long/long specifier. > > -- > Bruce Momjian| http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup.| Newtown Square, Pennsylvania 19073 > ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian > My next guess > is that Win32 isn't handling va_arg(..., long long int) properly. > I am trying various combination of number and types of parameters in my test program and everything prints fine. When it comes to pg, it fails :( > > template1=# select * from test where x > 1000::int8; > >x > > > >-869367531 > > (1 row) I am not too fluent in source code, could someone point me to there actual call to snprintf() is being done when a query like this is executed. I could not find it myslef :( Regards, Nick ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] snprintf causes regression tests to fail
Bruce Momjian wrote:
> I can confirm your failure in current sources on Win32:
>
> template1=# create table test(x int8);
> CREATE TABLE
> template1=# insert into test values ('4567890123456789');
> INSERT 17235 1
> template1=# select * from test;
>x
>
>-869367531
> (1 row)
>
> and it knows it is a large number:
>
> template1=# select * from test where x > 1000::int8;
>x
>
>-869367531
> (1 row)
> template1=# select * from test where x < 1000::int8;
>x
> ---
> (0 rows)
>
> I am going to add some debugs to see what is being passed to *printf().
I have not found the solution yet but am heading to bed. My next guess
is that Win32 isn't handling va_arg(..., long long int) properly.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 8: explain analyze is your friend
Re: [HACKERS] snprintf causes regression tests to fail
Nicolai Tufar wrote:
> Regression test diff is attached.
> It fails on the following tests:
>int8
>subselect
>union
>sequence
>
> It fails to display correctly number "4567890123456789".
> In output is shows "-869367531". Apparent overflow or
> interpreting int8 as int4.
>
> while rewriting snprintf() I did not touch the actual functions
> that convert number to ASCII digit string. In truth, if you
> force PostgreSQL to use snprintf.c without my patch applied
> it produces the same errors.
>
> What can be wrong? GCC bug? The one I use is:
> gcc.exe (GCC) 3.3.1 (mingw special 20030804-1)
I can confirm your failure in current sources on Win32:
template1=# create table test(x int8);
CREATE TABLE
template1=# insert into test values ('4567890123456789');
INSERT 17235 1
template1=# select * from test;
x
-869367531
(1 row)
and it knows it is a large number:
template1=# select * from test where x > 1000::int8;
x
-869367531
(1 row)
template1=# select * from test where x < 1000::int8;
x
---
(0 rows)
I am going to add some debugs to see what is being passed to *printf().
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] snprintf causes regression tests to fail
Nicolai Tufar wrote: > Neither Bruce's nor subsequent Tom's patch did not fix > the issue. The command used is: > > make maintainer-clean && ./configure && make && make install && make check > > It should have be fine to recompile the source code > completely. I attach the resulting config.log. May be it > will give a clue. Regression test failure are in the > same places as previous ones. No log attached. Please email the log to me privately. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] [HACKERS] snprintf causes regression tests to fail
Neither Bruce's nor subsequent Tom's patch did not fix the issue. The command used is: make maintainer-clean && ./configure && make && make install && make check It should have be fine to recompile the source code completely. I attach the resulting config.log. May be it will give a clue. Regression test failure are in the same places as previous ones. Best regards, Nicolai On Mon, 28 Feb 2005 16:29:57 -0500 (EST), Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian writes: > > > Ah, the problem was introduced here: > > > > Right, it was my fault. > > > > > The problem is that the PGAC_FUNC_PRINTF_ARG_CONTROL call was moved > > > below the printf 64-bit tests. This commited patch moves > > > PGAC_FUNC_PRINTF_ARG_CONTROL which is after we know AC_TRY_RUN works and > > > just before printf 64-bit args are tested. > > > > This patch breaks things in a different way: you should not have moved > > the AC_LIBOBJ(snprintf) step. Also you randomly placed the arg-control > > test between a chunk of code and the comment describing same. > > Thanks. > -- > Bruce Momjian| http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup.| Newtown Square, Pennsylvania 19073 > ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] snprintf causes regression tests to fail
[EMAIL PROTECTED] writes: > Do we have any idea about what format string causes the regression failure? I'll bet the problem is that configure.in is doing things in the wrong order: it computes INT64_FORMAT against the system printf before deciding we should use our own printf. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] snprintf causes regression tests to fail
After some extensive debugging with Magnus's
help we finally managed to a kind of isolate the
problem. We placed snprintf.c in a separate
file, added necessary #includes and wrote
a simple main() function:
main()
{
unsigned long long ull=4567890123456789ULL;
static char buf[1024];
mysnprintf(buf,1024,"%lld\n",ull);
printf(buf);
}
When compiled with -D HAVE_LONG_LONG_INT_64=1
which declares long_long and ulong_long like:
typedef long long long_long;
typedef unsigned long long ulong_long;
It compiles fine and produces desired result. If not,
it produces "-869367531" as in regression tests.
Amazingly enough HAVE_LONG_LONG_INT_64 is
defined when compilation comes to src/port/snprintf.c
but the result is still wrong. I looked into configure.in
but the check for HAVE_LONG_LONG_INT_64 is too
complicated for me to understand. Bruce, could you
take a look at this? I am 90% sure it is an issue with
some configure definitions.
Best regards,
Nicolai
On Mon, 28 Feb 2005 19:58:15 +0200, Nicolai Tufar <[EMAIL PROTECTED]> wrote:
> Regression test diff is attached.
> It fails on the following tests:
>int8
>subselect
>union
>sequence
>
> It fails to display correctly number "4567890123456789".
> In output is shows "-869367531". Apparent overflow or
> interpreting int8 as int4.
>
> while rewriting snprintf() I did not touch the actual functions
> that convert number to ASCII digit string. In truth, if you
> force PostgreSQL to use snprintf.c without my patch applied
> it produces the same errors.
>
> What can be wrong? GCC bug? The one I use is:
> gcc.exe (GCC) 3.3.1 (mingw special 20030804-1)
>
> Any thoughts?
>
>
> On Mon, 28 Feb 2005 09:17:20 -0500 (EST), Bruce Momjian
> wrote:
> > Nicolai Tufar wrote:
> > > Linux and Solaris 10 x86 pass regression tests fine when I force the use
> > > of new
> > > snprintf(). The problem should be win32 - specific. I will
> > > investigate it throughly
> > > tonight. Can someone experienced in win32 what can possibly be the
> > > problem?
> >
> > Yea, I am confused too because my BSD uses the new snprintf.c code was
> > well. Magnus, what failures are you seeing on Win32?
> >
> > --
> > Bruce Momjian| http://candle.pha.pa.us
> > pgman@candle.pha.pa.us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup.| Newtown Square, Pennsylvania 19073
> >
>
>
>
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [HACKERS] snprintf causes regression tests to fail
> Linux and Solaris 10 x86 pass regression tests fine when I force the use > of new > snprintf(). The problem should be win32 - specific. I will > investigate it throughly > tonight. Can someone experienced in win32 what can possibly be the > problem? Do we have any idea about what format string causes the regression failure? It may be that certain integer types are not promoted uniformly when pushed on the stack. > > Nick > > On Sun, 27 Feb 2005 19:07:16 +0100, Magnus Hagander <[EMAIL PROTECTED]> > wrote: >> Hi! >> >> The new snpritnf code appears not to deal with 64-bit ints. I'm getting >> failures on win32 for int8 as well as all the time related tests (win32 >> uses int8 for tinmestamps). Removing the snprintf code and falling back >> to the OS code makes everything pass again. >> >> I would guess this affects int8 etc on other platforms as well (assuming >> they use our snprintf and not the libc one), but I haven't checked it. >> >> //Magnus >> >> > > ---(end of broadcast)--- > TIP 9: the planner will ignore your desire to choose an index scan if your > joining column's datatypes do not match > ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] snprintf causes regression tests to fail
Linux and Solaris 10 x86 pass regression tests fine when I force the use of new snprintf(). The problem should be win32 - specific. I will investigate it throughly tonight. Can someone experienced in win32 what can possibly be the problem? Nick On Sun, 27 Feb 2005 19:07:16 +0100, Magnus Hagander <[EMAIL PROTECTED]> wrote: > Hi! > > The new snpritnf code appears not to deal with 64-bit ints. I'm getting > failures on win32 for int8 as well as all the time related tests (win32 > uses int8 for tinmestamps). Removing the snprintf code and falling back > to the OS code makes everything pass again. > > I would guess this affects int8 etc on other platforms as well (assuming > they use our snprintf and not the libc one), but I haven't checked it. > > //Magnus > > ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] snprintf causes regression tests to fail
Nicolai Tufar wrote: > Linux and Solaris 10 x86 pass regression tests fine when I force the use of > new > snprintf(). The problem should be win32 - specific. I will > investigate it throughly > tonight. Can someone experienced in win32 what can possibly be the problem? Yea, I am confused too because my BSD uses the new snprintf.c code was well. Magnus, what failures are you seeing on Win32? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[HACKERS] snprintf causes regression tests to fail
Hi! The new snpritnf code appears not to deal with 64-bit ints. I'm getting failures on win32 for int8 as well as all the time related tests (win32 uses int8 for tinmestamps). Removing the snprintf code and falling back to the OS code makes everything pass again. I would guess this affects int8 etc on other platforms as well (assuming they use our snprintf and not the libc one), but I haven't checked it. //Magnus ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
