Re: [PATCHES] [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 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [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: [PATCHES] [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
> Hmm ... > > 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. > > Second line of thought: libpq already feels free to insert > allegedly up-to-spec versions of a number of things, and no > one has complained. > Maybe the linker prevents problems by not linking these > versions to any calls from outside libpq? > > Third thought: Windows' linker seems to be broken enough that > it may create problems of this ilk that exist on no other platform. If you're takling the combination of libpq and Windows, we are definitly safe for dynamic linking, which is what most ppl will use. Because the DLL will only export the entrypoitns that we explicitly define in the DEF files, and those are also the only ones that are present in the "import library". //Magnus ---(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: [PATCHES] [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Nicolai Tufar <[EMAIL PROTECTED]> writes: > I started with FreeBSD's vsnprintf() at first > but was set back by it's complexity and decided to > modify the port/snprintf.c code. Now would you like me > to incorporate FreeBSD's one into the code. > Give me a week and I will come with the patch. It's all yours ... regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Tom lane wrote: > With CVS-tip snprintf I get > result = '3 42' > result = '3 3505' I get similar results: result = '3 42' result = '9e-313 1413754129' Now I agree with you, it is fundamentally broken. We need to replace this implementation. Bruce Momjian wrote: > I can confirm that using "%I64d" for the printf format allows the > regression tests to pass for int8. But snprintf.c code does not support "%I64d" construct. It must be picking OS's vsnprintf() Bruce Momjian wrote: > I think FreeBSD does. I started with FreeBSD's vsnprintf() at first but was set back by it's complexity and decided to modify the port/snprintf.c code. Now would you like me to incorporate FreeBSD's one into the code. Give me a week and I will come with the patch. Best regards, Nicolai Tufar ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian writes: > Have we considered what is going to happen to applications when they use > our snprintf instead of the one from the operating system? Hmm ... 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. Second line of thought: libpq already feels free to insert allegedly up-to-spec versions of a number of things, and no one has complained. Maybe the linker prevents problems by not linking these versions to any calls from outside libpq? Third thought: Windows' linker seems to be broken enough that it may create problems of this ilk that exist on no other platform. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster