Re: [HACKERS] snprintf()

2007-02-02 Thread Kate F
On Fri, Feb/ 2/07 11:20:07PM -0500, Tom Lane wrote:
> Kate F <[EMAIL PROTECTED]> writes:
> > On Fri, Feb/ 2/07 10:52:28PM -0500, Tom Lane wrote:
> >> I wouldn't really have expected that to happen on any *BSD, but you
> >> could look into the generated Makefile.global to find out.
> 
> > I don't see anything that looks relevant for my Makefile.global; I
> > would be surprised if NetBSD's were overridden too!
> 
> Sorry for not being specific: the thing to check is whether that
> file's definition for LIBOBJS includes snprintf.o.  If it does,
> the code in src/port/snprintf.c would get sucked in.
> 
> If it doesn't, then I confess bafflement as to why snprintf isn't
> acting as you'd expect on your machine.

Just these:

LIBOBJS =  copydir.o dirmod.o exec.o noblock.o path.o pipe.o pgsleep.o
pgstrcasecmp.o qsort.o qsort_arg.o sprompt.o thread.o

(More than I expected, actually)

I am imagining the compiler (gcc, here) has some flags to explicitly
select if C99 (which is what I tested my stand-alone example with)
or SUS behaviour is to be used. I'm not really sure how I'd set that,
though - I imagine I'd need to recompile the backend with -std=C99?

Regards,

-- 
Kate

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [HACKERS] snprintf()

2007-02-02 Thread Tom Lane
Kate F <[EMAIL PROTECTED]> writes:
> On Fri, Feb/ 2/07 10:52:28PM -0500, Tom Lane wrote:
>> I wouldn't really have expected that to happen on any *BSD, but you
>> could look into the generated Makefile.global to find out.

> I don't see anything that looks relevant for my Makefile.global; I
> would be surprised if NetBSD's were overridden too!

Sorry for not being specific: the thing to check is whether that
file's definition for LIBOBJS includes snprintf.o.  If it does,
the code in src/port/snprintf.c would get sucked in.

If it doesn't, then I confess bafflement as to why snprintf isn't
acting as you'd expect on your machine.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [HACKERS] snprintf()

2007-02-02 Thread Kate F
On Fri, Feb/ 2/07 10:52:28PM -0500, Tom Lane wrote:
> Kate F <[EMAIL PROTECTED]> writes:
> > ... does PostgreSQL replace my system's snprintf() prototype with
> > its own implementation's?
> 
> We do on some platforms where configure decides the system version
> is deficient ... I don't recall the exact conditions at the moment.
> I wouldn't really have expected that to happen on any *BSD, but you
> could look into the generated Makefile.global to find out.

I don't see anything that looks relevant for my Makefile.global; I
would be surprised if NetBSD's were overridden too!


> > For reference, the relevant part of C99:
> >   7.19.6.5 2 If n is zero, nothing is written, and s may be a null
> >   pointer.
> 
> For reference, the relevant part of the Single Unix Spec:
> 
>   If the value of n is zero on a call to snprintf(), an
>   unspecified value less than 1 is returned.

Aha! I do recall either POSIX or SUS defers to C on conflicts... I
can't find which, though. If this snprintf() is following SUS
behaviour, that's fine. Thank you!


> So the behavior you'd like to depend on is unportable anyway, and
> that coding will get rejected if submitted as a Postgres patch.

Absolutley (and I assume you target C89, too, which does not provide
snprintf()). This was just something personal where I happened to use
it for convenience.

Thank you for checking that - and appologies for posting to the wrong
list; that should have been to -bugs!

Regards,

-- 
Kate

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [HACKERS] snprintf()

2007-02-02 Thread Tom Lane
Kate F <[EMAIL PROTECTED]> writes:
> ... does PostgreSQL replace my system's snprintf() prototype with
> its own implementation's?

We do on some platforms where configure decides the system version
is deficient ... I don't recall the exact conditions at the moment.
I wouldn't really have expected that to happen on any *BSD, but you
could look into the generated Makefile.global to find out.

> For reference, the relevant part of C99:
>   7.19.6.5 2 If n is zero, nothing is written, and s may be a null
>   pointer.

For reference, the relevant part of the Single Unix Spec:

If the value of n is zero on a call to snprintf(), an
unspecified value less than 1 is returned.

So the behavior you'd like to depend on is unportable anyway, and
that coding will get rejected if submitted as a Postgres patch.

regards, tom lane

---(end of broadcast)---
TIP 1: 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()

2007-02-02 Thread Kate F
Hello,

I've been implementing a type I needed, and happened to be using
snprintf(), since I have C99 available.

ereport(NOTICE,
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("%d", snprintf(NULL, 0, "abc";

For me, this reports "0". I beieve it should report 3. My system's
snprintf() returns 3. I'm using NetBSD. By including postgres.h and
fmgr.h, does PostgreSQL replace my system's snprintf() prototype with
its own implementation's?

Placing stdio.h above those includes appears to have no effect.

For reference, the relevant part of C99:

  7.19.6.5 2 If n is zero, nothing is written, and s may be a null
  pointer.

  7.19.6.5 3 The snprintf function returns the number of characters
  that would have been written had n been sufficiently large, not
  counting the terminating null character, or a neg ative value if an
  encoding error occurred.

Regards,

-- 
Kate

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-06 Thread Andrew Dunstan



Andrew Dunstan wrote:





I committed the pg_regress change back in Nov but didn't change 
buildfarm to use it. And now I look at it more closely I think it 
won't work. We have:


/   # locale
/   NOLOCALE :=
  ifdef NO_LOCALE
  NOLOCALE += --no-locale
  endif

I think instead of the += line we need:

  override NOLOCALE := --nolocale




and if I look after I have had some coffee I will see the underscore I 
missed that makes it make sense. We now return you to your regular viewing.


cheers

andrew

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-06 Thread Andrew Dunstan



Andrew Dunstan wrote:


Tom Lane wrote:


Andrew Dunstan <[EMAIL PROTECTED]> writes:
 

Which raises another question: can we force the locale on Windows, 
or are we stuck with the locale that the machine is set to? But 
maybe that belongs in another thread.
  


I thought we'd put in some sort of "no-locale" switch specifically for
the buildfarm to use on Windows?  I recall talking about it anyway ...



Yeah, but I'm not sure it's working. I will look into it.



*sheepish look*

I committed the pg_regress change back in Nov but didn't change 
buildfarm to use it. And now I look at it more closely I think it won't 
work. We have:


/   # locale
/   NOLOCALE :=
  ifdef NO_LOCALE
  NOLOCALE += --no-locale
  endif

I think instead of the += line we need:

  override NOLOCALE := --nolocale

The intended effect is that if any NOLOCALE arg is used in invoking 
make, --no-locale gets passed to pg_regress.


cheers

andrew



---(end of broadcast)---
TIP 1: 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] [HACKERS] snprintf() argument reordering not working

2005-12-06 Thread Andrew Dunstan



Tom Lane wrote:


Andrew Dunstan <[EMAIL PROTECTED]> writes:
 

Which raises another question: can we force the locale on Windows, or 
are we stuck with the locale that the machine is set to? But maybe that 
belongs in another thread.
   



I thought we'd put in some sort of "no-locale" switch specifically for
the buildfarm to use on Windows?  I recall talking about it anyway ...


 



Yeah, but I'm not sure it's working. I will look into it.

cheers

andrew

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-06 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> Which raises another question: can we force the locale on Windows, or 
> are we stuck with the locale that the machine is set to? But maybe that 
> belongs in another thread.

I thought we'd put in some sort of "no-locale" switch specifically for
the buildfarm to use on Windows?  I recall talking about it anyway ...

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-06 Thread Andrew Dunstan



Tom Lane wrote:


Please test ...
 



Well, if you look here you'll see a bunch of Turkish messages, because I 
forgot to change the locale back ;-)


http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=loris&dt=2005-12-06%2011:57:20

Which raises another question: can we force the locale on Windows, or 
are we stuck with the locale that the machine is set to? But maybe that 
belongs in another thread.


cheers

andrew



---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-06 Thread Dave Page
 

> -Original Message-
> From: [EMAIL PROTECTED] 
> [mailto:[EMAIL PROTECTED] On Behalf Of Bruce Momjian
> Sent: 06 December 2005 04:40
> To: Andrew Dunstan
> Cc: Tom Lane; [EMAIL PROTECTED]; [EMAIL PROTECTED]; 
> [EMAIL PROTECTED]; pgsql-hackers@postgresql.org
> Subject: Re: [PATCHES] [HACKERS] snprintf() argument 
> reordering not working
> 
>  We hope to put out a new pginstaller in the next few days
> for testing to make sure this has been resolve before releasing 8.1.1.

http://developer.postgresql.org/~dpage/postgresql-8.1t1.zip

DO NOT use in production as it got virtually no testing. I regenerated
all the GUIDs, so it will install alongside an existing installation.

Regards, Dave.

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working under Windows in 8.1

2005-12-06 Thread Nicolai Tufar
2005/12/6, Nicolai Tufar <[EMAIL PROTECTED]>:
> >
> > IIRC last time I tried this it didn't work too well ;-( I will have
> > another go. I think it's the best way to go.
>
> Very well, I will try to put up a patch to implement it in a couple of days.

Oh boy, it is already fixed. Sorry folks, my error.
Many thanks to Bruce, Tom and Andrew!
Turksh Windows user can breathe easier now.

Sincerely,
Nicolai Tufar

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working under Windows in 8.1

2005-12-06 Thread Nicolai Tufar
2005/12/4, Andrew Dunstan <[EMAIL PROTECTED]>:
> Tom said:
>
> >Would it work to modify c.h so that it #include's libintl.h, then #undefs
> >these macros, then #includes port.h to define 'em the way we want?
> >Some or all of this might need to be #ifdef WIN32, but that seems like
> >a reasonably noninvasive solution if it can work.
> >
>
> IIRC last time I tried this it didn't work too well ;-( I will have
> another go. I think it's the best way to go.

Very well, I will try to put up a patch to implement it in a couple of days.

>
> cheers
>
> andrew
>

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> If we don't do this then we need to link snprintf into libpgtypes just 
> like we do for ecpg, but it seems like overkill.

It might be overkill today, but what about tomorrow when someone decides
to internationalize libpgtypes?  I made it link in there too.  Please
test ...

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Tom Lane wrote:


Not sure why my build didn't show the problem in pgtypeslib, though.
That should have failed with or without libintl macros.


 



On *nix it probably just thinks it's an external symbol to be resolved 
later.


cheers

andrew

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> Also, we need a way to stop this from happening all over the build:

> In file included from ../../../../../../src/include/c.h:820,
>  from ../../../../../../src/include/postgres.h:48,
>  from utf8_and_sjis.c:14:
> ../../../../../../src/include/port.h:121: warning: `libintl_printf' is an 
> unrecognized format function type

Argh, I ordered things wrong: should undef the old macros before
declaring the new functions.

Not sure why my build didn't show the problem in pgtypeslib, though.
That should have failed with or without libintl macros.

regards, tom lane

---(end of broadcast)---
TIP 1: 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] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Bruce Momjian

I did some research and can report what was happening with *printf and
libintl.

Basically, there are two versions of libintl.  Versions before 0.13
(November 2003) use the libc version of *printf to handle printing of
translation strings.  Version 0.13 and after provide their own *printf
functions which understand %$ functionality.  The 0.13 change is:

  - C format strings with positions, as they arise when a translator needs to
reorder a sentence, are now supported on all platforms. On those few
platforms (NetBSD and Woe32) for which the native printf()/fprintf()/...
functions don't support such format strings, replacements are provided
through .

In addition, reports in April 2003 that libintl did not compile with our
custom pg *printf functions on Win32 caused us to disable pg *printf
functions on Win32.  The assumption was that libintl had a special
*printf version to handle %$, but in fact only post-0.13 had that
feature.

If we had built pginstaller with a post-0.13 libintl, pginstaller would
have handled %$ translation strings fine.  The problem was that
pginstaller was built using pre-0.13 libintl, meaning it was using the
Win32 *printf, which doesn't understand %$.

Added to this was that our *printf functions had a bug that made %$ not
work.

Aside from fixing our own *printf, we have two ways of fixing this,
either use a post-0.13 version of libintl, or use the pre-0.13 libintl.

Based on risk analysis, we have chosen to continue using the same
pre-0.13 version of libintl, and to rely on our pg *printf functions to
handle %$.  We hope to put out a new pginstaller in the next few days
for testing to make sure this has been resolve before releasing 8.1.1.

---

Andrew Dunstan wrote:
> 
> 
> Tom Lane wrote:
> 
> >Andrew Dunstan <[EMAIL PROTECTED]> writes:
> >  
> >
> >>However, a very simple test shows that the libintl printf does indeed do 
> >>%m$ processing:
> >>...
> >>So the next question is why isn't it working in the build.
> >>
> >>
> >
> >Is it possible that the build that was being complained of was using our
> >previous, very-broken snprintf.c?
> >
> >
> >  
> >
> 
> There's currently a config setting that is supposed to inhibit its use 
> on Windows. I am quite confused.
> 
> What is more, when I set the locale of my machine to Turkish and run the 
> installer project's 8.1_RC1 which I happen to have installed there, and 
> set lc_messages to tr_TR.UTF-8, I don't see lines like Nicolai reported:
> 
>   LOG:  "$s" veritaban?n transaction ID warp limiti $u
> 
> I see this:
>   
>   LOG:  "2147484146" veritabanin transaction ID warp limiti postgres
> 
> So I'm inclined to think there might be something odd about his setup and 
> maybe we aren't quite so broken after all.
> 
> cheers
> 
> andrew
> 
> ---(end of broadcast)---
> TIP 2: Don't 'kill -9' the postmaster
> 

-- 
  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: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Tom Lane wrote:


Andrew Dunstan <[EMAIL PROTECTED]> writes:
 

OK, eyeball this for the REL8_1_STABLE branch, please. It seems to 
work for me. No exports necessary.
 



 


er try this instead. I missed a line from configure.in
   



I cleaned this up slightly and committed it into HEAD and 8.1 branches.
It appears to work in that I can force pgac_need_repl_snprintf to "yes"
on a Linux machine and get a working build.  But we need to verify that
things are OK on Windows, both with the old libintl that the installer
is using and with current libintl.  Please build and test ...
 



the cleanup seems to have omitted the change I had to 
src/interfaces/ecpg/pgtypeslib/extern.h, which causes a build failure - see


http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=loris&dt=2005-12-06%2003:30:36

If we don't do this then we need to link snprintf into libpgtypes just 
like we do for ecpg, but it seems like overkill.


cheers

andrew



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
>> OK, eyeball this for the REL8_1_STABLE branch, please. It seems to 
>> work for me. No exports necessary.

> er try this instead. I missed a line from configure.in

I cleaned this up slightly and committed it into HEAD and 8.1 branches.
It appears to work in that I can force pgac_need_repl_snprintf to "yes"
on a Linux machine and get a working build.  But we need to verify that
things are OK on Windows, both with the old libintl that the installer
is using and with current libintl.  Please build and test ...

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> OK, eyeball this for the REL8_1_STABLE branch, please. It seems to work 
> for me. No exports necessary.

OK, I see a few cleanups I want to make, but given the knowledge that
this patch does work on Win32, I should be able to get it done tonight.
Thanks for doing the legwork!

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Andrew Dunstan wrote:




Tom Lane wrote:

I'm coming around to thinking that the simple solution is just to 
use it unconditionally on Windows.
  



I agree that that's what we should do, but it should be done the same
way we handle other routines from libpgport.  None of those are exported
to our client-side programs via libpq.


 



OK, eyeball this for the REL8_1_STABLE branch, please. It seems to 
work for me. No exports necessary.





er try this instead. I missed a line from configure.in

cheers

andrew
? autom4te.cache
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.461
diff -c -r1.461 configure
*** configure	5 Nov 2005 04:01:38 -	1.461
--- configure	5 Dec 2005 22:39:43 -
***
*** 13851,13858 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
  
! pgac_need_repl_snprintf=no
  
  for ac_func in snprintf
  do
--- 13851,13862 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
+ # Win32 gets this built unconditionally
  
! if test "$PORTNAME" = "win32"; then
!   pgac_need_repl_snprintf=yes
! else
!   pgac_need_repl_snprintf=no
  
  for ac_func in snprintf
  do
***
*** 14061,14066 
--- 14065,14071 
  fi
  done
  
+ fi
  
  
  # Check whether  declares snprintf() and vsnprintf(); if not,
***
*** 17111,17123 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that
! # understands arg control, so we don't need our own.  In fact, it
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
--- 17116,17122 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no ; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
Index: configure.in
===
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.431
diff -c -r1.431 configure.in
*** configure.in	5 Nov 2005 04:01:41 -	1.431
--- configure.in	5 Dec 2005 22:39:44 -
***
*** 849,858 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
  
! pgac_need_repl_snprintf=no
! AC_CHECK_FUNCS(snprintf, [], pgac_need_repl_snprintf=yes)
! AC_CHECK_FUNCS(vsnprintf, [], pgac_need_repl_snprintf=yes)
  
  
  # Check whether  declares snprintf() and vsnprintf(); if not,
--- 849,863 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
+ # Win32 gets this built unconditionally
  
! if test "$PORTNAME" = "win32"; then
!   pgac_need_repl_snprintf=yes
! else
!   pgac_need_repl_snprintf=no
!   AC_CHECK_FUNCS(snprintf, [], pgac_need_repl_snprintf=yes)
!   AC_CHECK_FUNCS(vsnprintf, [], pgac_need_repl_snprintf=yes)
! fi
  
  
  # Check whether  declares snprintf() and vsnprintf(); if not,
***
*** 1046,1058 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that 
! # understands arg control, so we don't need our own.  In fact, it 
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
  pgac_need_repl_snprintf=yes
--- 1051,1057 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no ; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
  pgac_need_repl_snprintf=yes
Index: src/include/c.h
=

Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Tom Lane wrote:

I'm coming around to thinking that the simple solution is just to use it 
unconditionally on Windows.
   



I agree that that's what we should do, but it should be done the same
way we handle other routines from libpgport.  None of those are exported
to our client-side programs via libpq.


 



OK, eyeball this for the REL8_1_STABLE branch, please. It seems to work 
for me. No exports necessary.


cheers

andrew
? autom4te.cache
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.461
diff -c -r1.461 configure
*** configure	5 Nov 2005 04:01:38 -	1.461
--- configure	5 Dec 2005 22:22:11 -
***
*** 13851,13858 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
  
! pgac_need_repl_snprintf=no
  
  for ac_func in snprintf
  do
--- 13851,13861 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
+ # Win32 gets this built unconditionally
  
! if test "$PORTNAME" = "win32"; then
!   pgac_need_repl_snprintf=yes
! else
  
  for ac_func in snprintf
  do
***
*** 14061,14066 
--- 14064,14070 
  fi
  done
  
+ fi
  
  
  # Check whether  declares snprintf() and vsnprintf(); if not,
***
*** 17111,17123 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that
! # understands arg control, so we don't need our own.  In fact, it
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
--- 17115,17121 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no ; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
Index: configure.in
===
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.431
diff -c -r1.431 configure.in
*** configure.in	5 Nov 2005 04:01:41 -	1.431
--- configure.in	5 Dec 2005 22:22:12 -
***
*** 849,858 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
  
! pgac_need_repl_snprintf=no
! AC_CHECK_FUNCS(snprintf, [], pgac_need_repl_snprintf=yes)
! AC_CHECK_FUNCS(vsnprintf, [], pgac_need_repl_snprintf=yes)
  
  
  # Check whether  declares snprintf() and vsnprintf(); if not,
--- 849,862 
  # is missing.  Yes, there are machines that have only one.  We may
  # also decide to use snprintf.c if snprintf() is present but does not
  # have all the features we need --- see below.
+ # Win32 gets this built unconditionally
  
! if test "$PORTNAME" = "win32"; then
!   pgac_need_repl_snprintf=yes
! else
!   AC_CHECK_FUNCS(snprintf, [], pgac_need_repl_snprintf=yes)
!   AC_CHECK_FUNCS(vsnprintf, [], pgac_need_repl_snprintf=yes)
! fi
  
  
  # Check whether  declares snprintf() and vsnprintf(); if not,
***
*** 1046,1058 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that 
! # understands arg control, so we don't need our own.  In fact, it 
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
  pgac_need_repl_snprintf=yes
--- 1050,1056 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no ; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
  pgac_need_repl_snprintf=yes
Index: src/include/c.h
===
RCS file: /cvsroot/pgsql/src/include/c.h,v
retrieving revision 1.190
diff -c -r1.190 c.h
*** src/include/c.h	15 Oct 2005 02:49:41 -

Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Andrew Dunstan <[EMAIL PROTECTED]> writes:
>>> The problem is that the alias will be picked up by every libpq client.
>> 
>> Not at all; libpq clients do not import c.h.

> Well, all the programs that use postgres-fe.h do.

Sure, but all of them do (or should) include libpgport and can get at
the functions from that.

> I'm coming around to thinking that the simple solution is just to use it 
> unconditionally on Windows.

I agree that that's what we should do, but it should be done the same
way we handle other routines from libpgport.  None of those are exported
to our client-side programs via libpq.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> What about Plan B?  Per Bruce's comment, it should really only be ecpg
>> that needs an extra copy of snprintf.o, and it's not like ecpg doesn't
>> already pull in various port/ files for itself.

> The problem is that the alias will be picked up by every libpq client.

Not at all; libpq clients do not import c.h.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Dave Page

-Original Message-
From: "Andrew Dunstan"<[EMAIL PROTECTED]>
Sent: 05/12/05 19:03:17
To: "Tom Lane"<[EMAIL PROTECTED]>
Cc: "Bruce Momjian", "[EMAIL PROTECTED]"<[EMAIL 
PROTECTED]>, "[EMAIL PROTECTED]"<[EMAIL PROTECTED]>, "[EMAIL PROTECTED]"<[EMAIL 
PROTECTED]>, "pgsql-hackers@postgresql.org"
Subject: Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

> I'm not sure I see the objection to stripping these out of the *.def files.

It will be a recipe for disaster if different builds of the same dll have 
different exports - apps that pick up the wrong one from a shared dir for 
example are likely to crash at startup. We went to some effort to prevent this 
for 8.0, for example, by not having separate (and different) .def files for 
each compiler, but by building them all from exports.txt.

Regards, Dave

-Unmodified Original Message-


Tom Lane wrote:

>Andrew Dunstan <[EMAIL PROTECTED]> writes:
>  
>
>>The bad news: if we aren't compiling with NLS enabled, having those 
>>entries in exports.txt makes the libpq build blow up. So either we need 
>>to use pg_*printf unconditionally on Windows, or we need a little 
>>Makefile + sed magic to strip those entries out of exports.txt when it 
>>is used, if we're not doing NLS, or something of that kind.
>>
>>
>
>I think it's a bad idea for exports.txt not to be the same in all
>builds.  So yeah, if we export these names at all then it has to be
>unconditional.
>
>What about Plan B?  Per Bruce's comment, it should really only be ecpg
>that needs an extra copy of snprintf.o, and it's not like ecpg doesn't
>already pull in various port/ files for itself.
>
>
>  
>

The problem is that the alias will be picked up by every libpq client. I 
got around the problem with ecpg's libpgtypes by unaliasing  sprintf and 
snprintf. But we can't do that everywhere.

I'm not sure I see the objection to stripping these out of the *.def files.

I can't spend any more time on this now - I have spent far too much on 
it already. My working patch is attached. Maybe I can look at it again 
in a few days.

cheers

andrew

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Tom Lane wrote:


Andrew Dunstan <[EMAIL PROTECTED]> writes:
 

The bad news: if we aren't compiling with NLS enabled, having those 
entries in exports.txt makes the libpq build blow up. So either we need 
to use pg_*printf unconditionally on Windows, or we need a little 
Makefile + sed magic to strip those entries out of exports.txt when it 
is used, if we're not doing NLS, or something of that kind.
   



I think it's a bad idea for exports.txt not to be the same in all
builds.  So yeah, if we export these names at all then it has to be
unconditional.

What about Plan B?  Per Bruce's comment, it should really only be ecpg
that needs an extra copy of snprintf.o, and it's not like ecpg doesn't
already pull in various port/ files for itself.


 



The problem is that the alias will be picked up by every libpq client. I 
got around the problem with ecpg's libpgtypes by unaliasing  sprintf and 
snprintf. But we can't do that everywhere.


I'm not sure I see the objection to stripping these out of the *.def files.

I can't spend any more time on this now - I have spent far too much on 
it already. My working patch is attached. Maybe I can look at it again 
in a few days.


cheers

andrew
Index: configure
===
RCS file: /projects/cvsroot/pgsql/configure,v
retrieving revision 1.461
diff -c -r1.461 configure
*** configure	5 Nov 2005 04:01:38 -	1.461
--- configure	5 Dec 2005 18:56:04 -
***
*** 17111,17123 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that
! # understands arg control, so we don't need our own.  In fact, it
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
--- 17111,17117 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no ; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
Index: src/include/c.h
===
RCS file: /projects/cvsroot/pgsql/src/include/c.h,v
retrieving revision 1.190
diff -c -r1.190 c.h
*** src/include/c.h	15 Oct 2005 02:49:41 -	1.190
--- src/include/c.h	5 Dec 2005 18:56:23 -
***
*** 96,101 
--- 96,122 
  
  #ifdef ENABLE_NLS
  #include 
+ #ifdef WIN32
+ #ifdef USE_SNPRINTF
+ 
+ #ifdef printf
+ #undef printf
+ #endif
+ #ifdef fprintf
+ #undef fprintf
+ #endif
+ #ifdef sprintf
+ #undef sprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
+ #ifdef vsnprintf
+ #undef vsnprintf
+ #endif
+ 
+ #endif
+ #endif
  #else
  #define gettext(x) (x)
  #endif
Index: src/interfaces/ecpg/pgtypeslib/extern.h
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/pgtypeslib/extern.h,v
retrieving revision 1.7
diff -c -r1.7 extern.h
*** src/interfaces/ecpg/pgtypeslib/extern.h	15 Oct 2005 02:49:47 -	1.7
--- src/interfaces/ecpg/pgtypeslib/extern.h	5 Dec 2005 18:56:24 -
***
*** 1,6 
--- 1,14 
  #ifndef __PGTYPES_COMMON_H__
  #define __PGTYPES_COMMON_H__
  
+ 
+ #ifdef sprintf
+ #undef sprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
+ 
  #include "pgtypes_error.h"
  
  /* These are the constants that decide which printf() format we'll use in
Index: src/interfaces/libpq/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v
retrieving revision 1.138
diff -c -r1.138 Makefile
*** src/interfaces/libpq/Makefile	29 Aug 2005 00:47:35 -	1.138
--- src/interfaces/libpq/Makefile	5 Dec 2005 18:56:24 -
***
*** 25,30 
--- 25,34 
  override CFLAGS += $(PTHREAD_CFLAGS)
  endif
  
+ ifneq ($(enable_nls), yes)
+ NONLS = -e '/^pg_.*printf/d' 
+ endif
+ 
  # Need to recomple any libpgport object files
  LIBS := $(patsubst -lpgport,, $(LIBS))
  
***
*** 103,126 
  	echo 'LIBRARY LIBPQ' >> $@
  	echo 'DESCRIPTION "PostgreSQL Client Library"' >> $@
  	echo 'EXPORTS' >> $@
! 	sed -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/\1@ \2/' < $< >> $@
  
  $(srcdir)/libpqddll.def: exports.txt
  	echo '; DEF file for MS VC++' > $@
  	echo 'LIBRARY LIBPQD' >> $@
  	echo 'DESCRIPTION "PostgreSQL Client Library"' >> $@
  	echo 'EXPORTS' >> $@
! 	sed -e '/^#/d' 

Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> With luck I can probably wrap this up today for the 8.1 stable branch - 
> it would be nice if the new release actually did NLS right.

BTW, core has already agreed to postpone the releases a couple days
while we make sure we have this problem nailed down.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> The bad news: if we aren't compiling with NLS enabled, having those 
> entries in exports.txt makes the libpq build blow up. So either we need 
> to use pg_*printf unconditionally on Windows, or we need a little 
> Makefile + sed magic to strip those entries out of exports.txt when it 
> is used, if we're not doing NLS, or something of that kind.

I think it's a bad idea for exports.txt not to be the same in all
builds.  So yeah, if we export these names at all then it has to be
unconditional.

What about Plan B?  Per Bruce's comment, it should really only be ecpg
that needs an extra copy of snprintf.o, and it's not like ecpg doesn't
already pull in various port/ files for itself.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Bruce Momjian wrote:


Was the last patch you sent in ready for application, or are you still
fooling with it?
   



He is still working on it.  It did not handle all *printf functions, as
he mentioned, and he might have other changes.

 



Yeah.

The good news: the new pg_*printf does the right thing for the %m$ 
parameters in the Turkish locale.


The bad news: if we aren't compiling with NLS enabled, having those 
entries in exports.txt makes the libpq build blow up. So either we need 
to use pg_*printf unconditionally on Windows, or we need a little 
Makefile + sed magic to strip those entries out of exports.txt when it 
is used, if we're not doing NLS, or something of that kind.


Question: do the entries in exports.txt have to be numbered 
consecutively, or just uniquely?


With luck I can probably wrap this up today for the 8.1 stable branch - 
it would be nice if the new release actually did NLS right.


cheers

andrew

---(end of broadcast)---
TIP 4: Have you searched our list archives?

  http://archives.postgresql.org


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Bruce Momjian
Tom Lane wrote:
> Andrew Dunstan <[EMAIL PROTECTED]> writes:
> > With 8.1_RC1 I *do* get the results Nicolai reported. With the changes I 
> > made yesterday, I see the result above, i.e. what we expect from our own 
> > breakage of sprintf (i haven't yet updated the snapshot I took).
> 
> Ah.  OK, that makes sense.
> 
> > But the simple fix seems to be to use our version of printf and friends. 
> > The changes requires are not too invasive.
> 
> I agree with doing this even if we weren't faced with (apparently)
> multiple versions of libintl that don't all work alike.  My thought is
> that running our own version of snprintf on a heavily used port like
> Windows is exactly what is needed to flush out any remaining bugs.
> It's obviously not gotten enough field usage yet ...
> 
> Was the last patch you sent in ready for application, or are you still
> fooling with it?

He is still working on it.  It did not handle all *printf functions, as
he mentioned, and he might have other changes.

-- 
  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: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> With 8.1_RC1 I *do* get the results Nicolai reported. With the changes I 
> made yesterday, I see the result above, i.e. what we expect from our own 
> breakage of sprintf (i haven't yet updated the snapshot I took).

Ah.  OK, that makes sense.

> But the simple fix seems to be to use our version of printf and friends. 
> The changes requires are not too invasive.

I agree with doing this even if we weren't faced with (apparently)
multiple versions of libintl that don't all work alike.  My thought is
that running our own version of snprintf on a heavily used port like
Windows is exactly what is needed to flush out any remaining bugs.
It's obviously not gotten enough field usage yet ...

Was the last patch you sent in ready for application, or are you still
fooling with it?

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Tom Lane wrote:


Andrew Dunstan <[EMAIL PROTECTED]> writes:
 

What is more, when I set the locale of my machine to Turkish and run the 
installer project's 8.1_RC1 which I happen to have installed there, and 
set lc_messages to tr_TR.UTF-8, I don't see lines like Nicolai reported:

 LOG:  "$s" veritaban?n transaction ID warp limiti $u
I see this:
 LOG:  "2147484146" veritabanin transaction ID warp limiti postgres
   



Well, that's pretty broken too :-(.  The tr.po file entry is

msgid "transaction ID wrap limit is %u, limited by database \"%s\""
msgstr "\"%2$s\" veritabanın transaction ID warp limiti %1$u"

and if I'm not completely confused, correct translated output would be

"postgres" veritabanın transaction ID warp limiti 2147484146

Nicolai's report looks a bit like what you would expect from an sprintf
implementation that hadn't heard of %n$ specs at all.  Your report looks
suspiciously like what our broken version of sprintf was producing last
week --- see
http://archives.postgresql.org/pgsql-hackers/2005-12/msg00194.php

How certain are you that that config setting is inhibiting use of
port/snprintf.c?  It seems unlikely that any other implementation would
have duplicated our bug.
 




Sorry ... I got into a muddle. I have rerun the tests.

With 8.1_RC1 I *do* get the results Nicolai reported. With the changes I 
made yesterday, I see the result above, i.e. what we expect from our own 
breakage of sprintf (i haven't yet updated the snapshot I took). I will 
now try to verify that the changes you made in pg_sprintf do the right 
thing.


We could ask why it appears that one version of libintl works (the one I 
got the other day from gnuwin32) and one doesn't (the one that is in the 
installer, apparently).


But the simple fix seems to be to use our version of printf and friends. 
The changes requires are not too invasive.



cheers

andrew

---(end of broadcast)---
TIP 1: 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] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> What is more, when I set the locale of my machine to Turkish and run the 
> installer project's 8.1_RC1 which I happen to have installed there, and 
> set lc_messages to tr_TR.UTF-8, I don't see lines like Nicolai reported:
>   LOG:  "$s" veritaban?n transaction ID warp limiti $u
> I see this:
>   LOG:  "2147484146" veritabanin transaction ID warp limiti postgres

Well, that's pretty broken too :-(.  The tr.po file entry is

msgid "transaction ID wrap limit is %u, limited by database \"%s\""
msgstr "\"%2$s\" veritabanın transaction ID warp limiti %1$u"

and if I'm not completely confused, correct translated output would be

"postgres" veritabanın transaction ID warp limiti 2147484146

Nicolai's report looks a bit like what you would expect from an sprintf
implementation that hadn't heard of %n$ specs at all.  Your report looks
suspiciously like what our broken version of sprintf was producing last
week --- see
http://archives.postgresql.org/pgsql-hackers/2005-12/msg00194.php

How certain are you that that config setting is inhibiting use of
port/snprintf.c?  It seems unlikely that any other implementation would
have duplicated our bug.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan



Tom Lane wrote:


Andrew Dunstan <[EMAIL PROTECTED]> writes:
 

However, a very simple test shows that the libintl printf does indeed do 
%m$ processing:

...
So the next question is why isn't it working in the build.
   



Is it possible that the build that was being complained of was using our
previous, very-broken snprintf.c?


 



There's currently a config setting that is supposed to inhibit its use 
on Windows. I am quite confused.


What is more, when I set the locale of my machine to Turkish and run the 
installer project's 8.1_RC1 which I happen to have installed there, and 
set lc_messages to tr_TR.UTF-8, I don't see lines like Nicolai reported:


 LOG:  "$s" veritaban?n transaction ID warp limiti $u

I see this:
 
 LOG:  "2147484146" veritabanin transaction ID warp limiti postgres


So I'm inclined to think there might be something odd about his setup and maybe 
we aren't quite so broken after all.

cheers

andrew

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> However, a very simple test shows that the libintl printf does indeed do 
> %m$ processing:
> ...
> So the next question is why isn't it working in the build.

Is it possible that the build that was being complained of was using our
previous, very-broken snprintf.c?

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-05 Thread Andrew Dunstan


I wrote:


Bruce Momjian said:
 


OK, a few things.  First, Tom has fixed snprintf.c so it should
properly process positional parameters now.  Would you first test the
libintl version of *printf to see if it can process %$ parameters
(probably by hacking up any language file and testing the printing),
and then try your patch below to see if it is properly reorders the
arguments.  If libintl does not reorder properly, but our snprintf.c
does, would you please generate an appliable patch we can put into
8.1.X?  Thanks.

I know I am asking a lot, but you are "the man" on this one.  :-)

   



Since the effect of the configure change I am proposing to reverse was to
force use of the *printf in libintl, don't we already know the answer to
your first question from Nicolai's report?


 



However, a very simple test shows that the libintl printf does indeed do 
%m$ processing:



$ cat testpf.c
#include 
main()
{
   printf("%2$s %1$s\n","arg1","arg2");
}
$ gcc -o testpf testpf.c -lintl
$ ./testpf.exe
arg2 arg1
$

So the next question is why isn't it working in the build.

cheers

andrew




---(end of broadcast)---
TIP 4: Have you searched our list archives?

  http://archives.postgresql.org


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Andrew Dunstan
Bruce Momjian said:
>
> OK, a few things.  First, Tom has fixed snprintf.c so it should
> properly process positional parameters now.  Would you first test the
> libintl version of *printf to see if it can process %$ parameters
> (probably by hacking up any language file and testing the printing),
> and then try your patch below to see if it is properly reorders the
> arguments.  If libintl does not reorder properly, but our snprintf.c
> does, would you please generate an appliable patch we can put into
> 8.1.X?  Thanks.
>
> I know I am asking a lot, but you are "the man" on this one.  :-)
>

Since the effect of the configure change I am proposing to reverse was to
force use of the *printf in libintl, don't we already know the answer to
your first question from Nicolai's report?

cheers

andrew



---(end of broadcast)---
TIP 1: 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] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Bruce Momjian

OK, a few things.  First, Tom has fixed snprintf.c so it should properly
process positional parameters now.  Would you first test the libintl
version of *printf to see if it can process %$ parameters (probably by
hacking up any language file and testing the printing), and then try
your patch below to see if it is properly reorders the arguments.  If
libintl does not reorder properly, but our snprintf.c does, would you
please generate an appliable patch we can put into 8.1.X?  Thanks.

I know I am asking a lot, but you are "the man" on this one.  :-)

---

Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
> 
> >Tom Lane wrote:
> >  
> >
> >>Andrew Dunstan <[EMAIL PROTECTED]> writes:
> >>
> >>
> >>>That got me through the backend compile and through libpq to ecpg, which 
> >>>fell over at the link stage complaining about missing references to 
> >>>pg_sprintf and pg_snprintf ... not sure how to fix that - windows 
> >>>experts, please advise.
> >>>  
> >>>
> >>Plan A would be to make libpq export pg_snprintf and friends, Plan B
> >>would be to give ecpg its own copy of snprintf.o.  Plan A is probably
> >>better since you're going to hit the same issue no doubt in all of the
> >>src/bin programs.
> >>
> >>
> >
> >I am confused why we would need either of these.  All apps use
> >libpgport, and that pg_*printf should be in that library.  The original
> >work Andrew did has problems particularly with ecpg, but why does ecpg
> >cause problems?  Doesn't it link in pgport?
> >
> >  
> >
> 
> 
> 
> libpgtypes doesn't link with either libpgport or libpq.
> 
> What I have done to get a working build in addition to the previous 
> report is to undef snprintf and sprintf in 
> interfaces/pgtypeslib/extern.h (instead of creating an additional link 
> burden), and to add entries for pg_snprintf, pg_sprintf and pg_fprintf 
> to interfaces/libpq/exports.txt. That let me get a clean compile and 
> regression run. The diff against 8.1 is attached for comment.
> 
> I suspect we should probably add all the pg_*printf functions to 
> exports.txt.
> 
> (Of course, first you need to install gettext and libintl from the 
> gnuwin32 project, or you can't even configure with --enable-nls)
> 
> cheers
> 
> andrew
> 
> 


> 
> ---(end of broadcast)---
> TIP 2: Don't 'kill -9' the postmaster

-- 
  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 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Andrew Dunstan



Bruce Momjian wrote:


Tom Lane wrote:
 


Andrew Dunstan <[EMAIL PROTECTED]> writes:
   

That got me through the backend compile and through libpq to ecpg, which 
fell over at the link stage complaining about missing references to 
pg_sprintf and pg_snprintf ... not sure how to fix that - windows 
experts, please advise.
 


Plan A would be to make libpq export pg_snprintf and friends, Plan B
would be to give ecpg its own copy of snprintf.o.  Plan A is probably
better since you're going to hit the same issue no doubt in all of the
src/bin programs.
   



I am confused why we would need either of these.  All apps use
libpgport, and that pg_*printf should be in that library.  The original
work Andrew did has problems particularly with ecpg, but why does ecpg
cause problems?  Doesn't it link in pgport?

 





libpgtypes doesn't link with either libpgport or libpq.

What I have done to get a working build in addition to the previous 
report is to undef snprintf and sprintf in 
interfaces/pgtypeslib/extern.h (instead of creating an additional link 
burden), and to add entries for pg_snprintf, pg_sprintf and pg_fprintf 
to interfaces/libpq/exports.txt. That let me get a clean compile and 
regression run. The diff against 8.1 is attached for comment.


I suspect we should probably add all the pg_*printf functions to 
exports.txt.


(Of course, first you need to install gettext and libintl from the 
gnuwin32 project, or you can't even configure with --enable-nls)


cheers

andrew


Index: configure
===
RCS file: /projects/cvsroot/pgsql/configure,v
retrieving revision 1.461
diff -c -r1.461 configure
*** configure	5 Nov 2005 04:01:38 -	1.461
--- configure	4 Dec 2005 22:56:58 -
***
*** 17111,17123 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that
! # understands arg control, so we don't need our own.  In fact, it
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
--- 17111,17117 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no ; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" >&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
Index: src/include/c.h
===
RCS file: /projects/cvsroot/pgsql/src/include/c.h,v
retrieving revision 1.190
diff -c -r1.190 c.h
*** src/include/c.h	15 Oct 2005 02:49:41 -	1.190
--- src/include/c.h	4 Dec 2005 22:57:02 -
***
*** 96,101 
--- 96,122 
  
  #ifdef ENABLE_NLS
  #include 
+ #ifdef WIN32
+ #ifdef USE_SNPRINTF
+ 
+ #ifdef printf
+ #undef printf
+ #endif
+ #ifdef fprintf
+ #undef fprintf
+ #endif
+ #ifdef sprintf
+ #undef sprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
+ #ifdef vsnprintf
+ #undef vsnprintf
+ #endif
+ 
+ #endif
+ #endif
  #else
  #define gettext(x) (x)
  #endif
Index: src/interfaces/ecpg/pgtypeslib/extern.h
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/pgtypeslib/extern.h,v
retrieving revision 1.7
diff -c -r1.7 extern.h
*** src/interfaces/ecpg/pgtypeslib/extern.h	15 Oct 2005 02:49:47 -	1.7
--- src/interfaces/ecpg/pgtypeslib/extern.h	4 Dec 2005 22:57:05 -
***
*** 1,6 
--- 1,14 
  #ifndef __PGTYPES_COMMON_H__
  #define __PGTYPES_COMMON_H__
  
+ 
+ #ifdef sprintf
+ #undef sprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
+ 
  #include "pgtypes_error.h"
  
  /* These are the constants that decide which printf() format we'll use in
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.5
diff -c -r1.5 exports.txt
*** src/interfaces/libpq/exports.txt	21 Oct 2005 15:21:21 -	1.5
--- src/interfaces/libpq/exports.txt	4 Dec 2005 22:57:06 -
***
*** 125,127 
--- 125,130 
  lo_create 123
  PQinitSSL 124
  PQregisterThreadLock  125
+ pg_sprintf		  126
+ pg_snprintf		  127
+ pg_fprintf		  128
Index: src/interfaces/libpq/win32.h
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/win32.h

Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Bruce Momjian
Tom Lane wrote:
> Andrew Dunstan <[EMAIL PROTECTED]> writes:
> > That got me through the backend compile and through libpq to ecpg, which 
> > fell over at the link stage complaining about missing references to 
> > pg_sprintf and pg_snprintf ... not sure how to fix that - windows 
> > experts, please advise.
> 
> Plan A would be to make libpq export pg_snprintf and friends, Plan B
> would be to give ecpg its own copy of snprintf.o.  Plan A is probably
> better since you're going to hit the same issue no doubt in all of the
> src/bin programs.

I am confused why we would need either of these.  All apps use
libpgport, and that pg_*printf should be in that library.  The original
work Andrew did has problems particularly with ecpg, but why does ecpg
cause problems?  Doesn't it link in pgport?

-- 
  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 6: explain analyze is your friend


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Tom Lane
Andrew Dunstan <[EMAIL PROTECTED]> writes:
> That got me through the backend compile and through libpq to ecpg, which 
> fell over at the link stage complaining about missing references to 
> pg_sprintf and pg_snprintf ... not sure how to fix that - windows 
> experts, please advise.

Plan A would be to make libpq export pg_snprintf and friends, Plan B
would be to give ecpg its own copy of snprintf.o.  Plan A is probably
better since you're going to hit the same issue no doubt in all of the
src/bin programs.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Andrew Dunstan



Andrew Dunstan wrote:



Tom said:

Would it work to modify c.h so that it #include's libintl.h, then 
#undefs

these macros, then #includes port.h to define 'em the way we want?
Some or all of this might need to be #ifdef WIN32, but that seems like
a reasonably noninvasive solution if it can work.



IIRC last time I tried this it didn't work too well ;-( I will have 
another go. I think it's the best way to go.





progress so far: I undid the config changes Bruce had made and undefined 
printf, fprintf, sprintf, snprintf and vsnprintf after the include of 
libintl.h in include/c.h. Then to clean up some warnings I undefined  
vsnprintf and snprintf in interfaces/libpq/win32.h before their 
redefinition.


That got me through the backend compile and through libpq to ecpg, which 
fell over at the link stage complaining about missing references to 
pg_sprintf and pg_snprintf ... not sure how to fix that - windows 
experts, please advise.


cheers

andrew


---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working

2005-12-04 Thread Andrew Dunstan



Bruce Momjian wrote:


OK, here is what happened.  In March 2005, we got reports of compile
problems on Win32 using NLS:

http://archives.postgresql.org/pgsql-hackers/2005-03/msg00710.php

(See the quoted text under the posted text as well.)  Basically,
libintl.h on Win32 replaces *printf calls with its own versions, and
does it using macros, _just_ like we do.  This of course causes
conflicts and the system fails to compile.  The _fix_ was to disable
port/*printf on Win32 when using NLS because NLS wants to use its own
*printf.  I _assumed_ that libintl.h did this so it could use its own
routines that understood %$, but never verified that.  It didn't seem we
had any choice to fix this, and got no problem reports about %$ not
working until yours.

After over a month with no solution I added the code as you see it now:

http://archives.postgresql.org/pgsql-hackers-win32/2005-05/msg00011.php

Oh, and it was Andrew Dunstan who worked on this, not Magnus.  (Sorry
Magnus, Hello Andrew.)

Anyway, I think the big question is, was the pginstaller built with a
libintl that replaces *printf, and is it an *printf that doesn't
understand positional parameters, and so, how can we fix it.

 



Well , I diagnosed the problem - you're on your own as far as the 
"solution" goes ;-)


On thing that seems clear to me is that we need a way of testing NLS 
support.


Tom said:


Would it work to modify c.h so that it #include's libintl.h, then #undefs
these macros, then #includes port.h to define 'em the way we want?
Some or all of this might need to be #ifdef WIN32, but that seems like
a reasonably noninvasive solution if it can work.



IIRC last time I tried this it didn't work too well ;-( I will have 
another go. I think it's the best way to go.


cheers

andrew

---(end of broadcast)---
TIP 1: 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] [HACKERS] snprintf() argument reordering not working under

2005-12-03 Thread Tom Lane
Bruce Momjian  writes:
> (See the quoted text under the posted text as well.)  Basically,
> libintl.h on Win32 replaces *printf calls with its own versions, and
> does it using macros, _just_ like we do.  This of course causes
> conflicts and the system fails to compile.  The _fix_ was to disable
> port/*printf on Win32 when using NLS because NLS wants to use its own
> *printf.  I _assumed_ that libintl.h did this so it could use its own
> routines that understood %$, but never verified that.

Oops ... [ insert standard cliche about assumptions ]

It might be interesting to find out why libintl is replacing these
functions if not to support arg reordering, but I suppose the bottom
line will just be that Microsoft is as brain dead as usual :-(

> Anyway, I think the big question is, was the pginstaller built with a
> libintl that replaces *printf, and is it an *printf that doesn't
> understand positional parameters, and so, how can we fix it.

Would it work to modify c.h so that it #include's libintl.h, then #undefs
these macros, then #includes port.h to define 'em the way we want?
Some or all of this might need to be #ifdef WIN32, but that seems like
a reasonably noninvasive solution if it can work.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] snprintf() argument reordering not working under

2005-12-03 Thread Bruce Momjian
Nicolai Tufar wrote:
> Greetings,
> 
> I thought it will be as simple as changing Makefile, the issue seem to be
> much more complicated. Unfortunately I have no PostgreSQL building
> environment handy and will not be able to look at it until the end of next
> week because I am moving my house :( But since this issue waited for so
> long it may wait a week more.

Agreed.  The problem is actually worse than I described --- see below.

> 2005/12/3, Bruce Momjian :
> > Sure, I remember.  So glad you returned at this time.  I found a design
> > limitation in that file yesterday.  It can not output more then 4096
> > characters, and there are some cases with NUMERIC that try to output
> > more than that.  For example:
> >
> > SELECT pow(10::numeric, 1) + 1;
> >
> > should show a '1' at the end of the number, but with the existing code
> > you will just see 4095 0's and no more.
> >
> > I am attaching the new snprintf.c and the patch itself for your review.
> > The change is to pass 'stream' down into the routines and output to the
> > FILE* right from inside the routine, rather than using a string.  This
> > fixes the problem.
> 
> Your patch increase buffers from  4095 to 8192. Sounds good to me.

Well, that fixed-size buffer is now used only for sprintf().  The others
use the specified length (for snprintf()) or output directly to the
FILE* stream.

> > I am also thinking of modifying the code so if we are using snprintf.c
> > only because we need positional parameter control, we check for '$' in
> > the string and only use snprintf.c in those cases.
> 
> I too, was thinking of it at the beginning but decided that the code would
> get even more complicated than it was at the moment and was afraid that
> core team would not accept my patch.   :)

Seems Tom didn't like that and no one else has commented.

> > > NLS messages of some languages, like Turkish, rely heavily on argument
> > > reordering because of the language structure. In 8.1 Turkish messages
> > > in Windows version are all broken because argument reordering is not 
> > > there.
> >
> > Really?  I have not heard any report of that but this is new code in 8.1.
> 
> Windows installer does not set lc_messages configuration variable
> correctly in postgresql.conf file. It is a known issue and will be solved
> in next version. Meanwhile user needs to open postgresql.conf file
> and change
> 
> lc_messages = 'Turkish_Turkey.28599'
>to
> lc_messages = 'tr_TR.UTF-8'
> 
> manually. Apparently nobody cared to do it and Devrim never ever touches
> Windows. The problem is there, I am definitely positive about it, here are
> two lines from log file:
> 
> 2005-11-20 12:36:37 HATA:  "$s"  yerinde $s 1 karakterinde
> 2005-12-03 19:14:27 LOG:  "$s" veritaban?n transaction ID warp limiti $u

OK.

> > Actually, that changes means that there was nothing backend-specific in
> > snprintf.c so we don't need a _special_ version for the backend.   The
> > real change not to use snprintf.c on Win32 is in configure.in with this
> > comment:
> >
> > # Force use of our snprintf if system's doesn't do arg control
> > # This feature is used by NLS
> > if test "$enable_nls" = yes &&
> >test $pgac_need_repl_snprintf = no &&
> > # On Win32, libintl replaces snprintf() with its own version that
> > # understands arg control, so we don't need our own.  In fact, it
> > # also uses macros that conflict with ours, so we _can't_ use
> > # our own.
> >test "$PORTNAME" != "win32"; then
> >   PGAC_FUNC_PRINTF_ARG_CONTROL
> >   if test $pgac_cv_printf_arg_control != yes ; then
> > pgac_need_repl_snprintf=yes
> >   fi
> > fi
> >
> > Here is the commit:
> >
> > revision 1.409
> > date: 2005/05/05 19:15:54;  author: momjian;  state: Exp;  lines: 
> > +8 -2
> > On Win32, libintl replaces snprintf() with its own version that
> > understands arg control, so we don't need our own.  In fact, it
> > also uses macros that conflict with ours, so we _can't_ use
> > our own.
> 
> I don't have MinGW build environment on my computer at the moment
> and will not be able to install it until the end of next week but log messages
> above were produced by PostgreSQL installed with 8.1.0-2 Windows installer
> downloaded from postgresql.org. Since Turkish messages are printed
> I suppose it was compiled with $enable_nls = yes

OK, here is what happened.  In March 2005, we got reports of compile
problems on Win32 using NLS:

http://archives.postgresql.org/pgsql-hackers/2005-03/msg00710.php

(See the quoted text under the posted text as well.)  Basically,
libintl.h on Win32 replaces *printf calls with its own versions, and
does it using macros, _just_ like we do.  This of course causes
conflicts and the system fails to compile.  The _fix_ was to disable
port/*printf on Win32 when using NLS because NLS wants to use its own

[HACKERS] snprintf() argument reordering not working under Windows in 8.1

2005-12-03 Thread Nicolai Tufar
Greetings,

Last April we have made some changes to src/ports/snprintf.c so that it
would support argument reordering like %2$s, %1$d and such on
platforms where original snprintf() does not support it, like Windows,
HP-UX or NetBSD.

NLS messages of some languages, like Turkish, rely heavily on argument
reordering because of the language structure. In 8.1 Turkish messages
in Windows version are all broken because argument reordering is not there.

I examined commit logs and came to conclusion that src/port/snprintf.c
is not included in server when compiling under Windows because of change
to src/port/Makefile made in revision 1.28 by Bruce Momjian.  See here:

http://developer.postgresql.org/cvsweb.cgi/pgsql/src/port/Makefile

Comment to the commit says:
`No server version of snprintf needed, so remove Makefile rule.'
In fact I think we need snprintf in server because NLS messages are
printed by the server.


Kindest regards,
Nicolai Tufar

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-05-05 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> After some further digging, I think we have 3 problems.
> 
> 1. On Windows gettext wants to hijack printf and friends, as below. This
> strikes me as rather unfriendly behaviour by a library header file.
> Anyway, mercifully libintl.h is included in our source in exactly one
> spot, so I think the thing to do for this problem is a) undo that
> hijacking and b) make sure any hijacking we want to do occurs after the
> point where that file in included (in c.h). This causes most of the
> noise, but is probably harmless, since our hijacking does in fact win
> out. We need to fix the arnings, though.
> 
> 2. We have multiple #defines for snprintf and vsnprintf (in port.h and
> win32.h).
> 
> 3. ecpg wants to use our pg*printf routines (because USE_SNPRINTF is
> defined) but doesn't know where to find them.
> 
> what a mess :-(

Based on the "mess" analysis, I decided it is better to allow libintl to
use its own snprintf() for Win32 when NLS is enabled, rather than try to
override that with our own snprintf.  I have added code to configure.in
so that we don't check for arg control in native snprintf on Win32
because when we are using NLS, we are going to get their snprintf anyway
and not the native one.

Patch attached and applied.

-- 
  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
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.435
diff -c -c -r1.435 configure
*** configure   5 May 2005 11:50:17 -   1.435
--- configure   5 May 2005 19:13:35 -
***
*** 14706,14712 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" 
>&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
--- 14706,14718 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that
! # understands arg control, so we don't need our own.  In fact, it
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
echo "$as_me:$LINENO: checking whether printf supports argument control" >&5
  echo $ECHO_N "checking whether printf supports argument control... $ECHO_C" 
>&6
  if test "${pgac_cv_printf_arg_control+set}" = set; then
Index: configure.in
===
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.408
diff -c -c -r1.408 configure.in
*** configure.in5 May 2005 11:50:18 -   1.408
--- configure.in5 May 2005 19:13:36 -
***
*** 1095,1101 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes && test $pgac_need_repl_snprintf = no; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
  pgac_need_repl_snprintf=yes
--- 1095,1107 
  
  # Force use of our snprintf if system's doesn't do arg control
  # This feature is used by NLS
! if test "$enable_nls" = yes &&
!test $pgac_need_repl_snprintf = no &&
! # On Win32, libintl replaces snprintf() with its own version that 
! # understands arg control, so we don't need our own.  In fact, it 
! # also uses macros that conflict with ours, so we _can't_ use
! # our own.
!test "$PORTNAME" != "win32"; then
PGAC_FUNC_PRINTF_ARG_CONTROL
if test $pgac_cv_printf_arg_control != yes ; then
  pgac_need_repl_snprintf=yes

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-20 Thread Andrew Dunstan

After some further digging, I think we have 3 problems.

1. On Windows gettext wants to hijack printf and friends, as below. This
strikes me as rather unfriendly behaviour by a library header file.
Anyway, mercifully libintl.h is included in our source in exactly one
spot, so I think the thing to do for this problem is a) undo that
hijacking and b) make sure any hijacking we want to do occurs after the
point where that file in included (in c.h). This causes most of the
noise, but is probably harmless, since our hijacking does in fact win
out. We need to fix the arnings, though.

2. We have multiple #defines for snprintf and vsnprintf (in port.h and
win32.h).

3. ecpg wants to use our pg*printf routines (because USE_SNPRINTF is
defined) but doesn't know where to find them.

what a mess :-(

cheers

andrew


Bruce Momjian wrote:

>Thanks to Andrew Dunstan, I found the cause of these link errors. 
>Andrew found this in libintl:
>   
>   #undef snprintf
>   #define snprintf libintl_snprintf
>   extern int snprintf (char *, size_t, const char *, ...);
>
>What is happening is that we do:
>
>   #define snprintfpg_snprintf
>
>and then libintl.h (?) does:
>
>   #define snprintf libintl_snprintf
>
>so the effect is:
>
>   #define pg_snprintf libintl_snprintf
>
>In fact, in this example, the system complains about a missing X3 symbol:
>
>   #define X1 X2
>   #define X2 X3
>   
>   int
>   main(int argc, char *argv[])
>   {
>   X1;
>   }
>
>so the effet of the defines is:
>
>   #define X1 X3
>
>Anyway, the reason ecpg is failing is that it is the only client-side
>program that doesn't use libintl for internationalization.  It is on our
>TODO list to do that, but it hasn't been done yet.
>
>However, only Win32 is seeing this failure, and only when configure
>--enable-nls.  I think this is because only Win32 does the redefine of
>snprint and friends.
>
>Comments?
>   
>---
>
>Nicolai Tufar wrote:
>  
>
>>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
>>
>>
>>
>
>  
>


---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-19 Thread Greg Stark
Bruce Momjian  writes:

> so the effect is:
> 
>   #define pg_snprintf libintl_snprintf

That's not how CPP works.

> In fact, in this example, the system complains about a missing X3 symbol:
> 
>   #define X1 X2
>   #define X2 X3

In this case any occurrence of X1 replaced by X2 but then the result is
rescanned for macros and X2 is turned into X3.

-- 
greg


---(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

2005-03-19 Thread Bruce Momjian

Thanks to Andrew Dunstan, I found the cause of these link errors. 
Andrew found this in libintl:

#undef snprintf
#define snprintf libintl_snprintf
extern int snprintf (char *, size_t, const char *, ...);

What is happening is that we do:

#define snprintfpg_snprintf

and then libintl.h (?) does:

#define snprintf libintl_snprintf

so the effect is:

#define pg_snprintf libintl_snprintf

In fact, in this example, the system complains about a missing X3 symbol:

#define X1 X2
#define X2 X3

int
main(int argc, char *argv[])
{
X1;
}

so the effet of the defines is:

#define X1 X3

Anyway, the reason ecpg is failing is that it is the only client-side
program that doesn't use libintl for internationalization.  It is on our
TODO list to do that, but it hasn't been done yet.

However, only Win32 is seeing this failure, and only when configure
--enable-nls.  I think this is because only Win32 does the redefine of
snprint and friends.

Comments?

---

Nicolai Tufar wrote:
> 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
> 

-- 
  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: [PATCHES] [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-16 Thread Bruce Momjian
Nicolai Tufar wrote:
> 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. 
> 

Here is another patch that adds sprintf() support, and support for '+',
'h', and fixes '%*s' support.

Applied.

-- 
  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
Index: src/bin/psql/command.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.141
diff -c -c -r1.141 command.c
*** src/bin/psql/command.c  11 Mar 2005 17:20:34 -  1.141
--- src/bin/psql/command.c  16 Mar 2005 21:17:50 -
***
*** 1574,1584 
shellName = DEFAULT_SHELL;
  
sys = pg_malloc(strlen(shellName) + 16);
sprintf(sys,
/* See EDITOR handling comment for an explaination */
- #ifndef WIN32
"exec %s", shellName);
  #else
"%s\"%s\"%s", SYSTEMQUOTE, shellName, 
SYSTEMQUOTE);
  #endif
result = system(sys);
--- 1574,1586 
shellName = DEFAULT_SHELL;
  
sys = pg_malloc(strlen(shellName) + 16);
+ #ifndef WIN32
sprintf(sys,
/* See EDITOR handling comment for an explaination */
"exec %s", shellName);
  #else
+   sprintf(sys,
+   /* See EDITOR handling comment for an explaination */
"%s\"%s\"%s", SYSTEMQUOTE, shellName, 
SYSTEMQUOTE);
  #endif
result = system(sys);
Index: src/include/port.h
===
RCS file: /cvsroot/pgsql/src/include/port.h,v
retrieving revision 1.72
diff -c -c -r1.72 port.h
*** src/include/port.h  11 Mar 2005 19:13:42 -  1.72
--- src/include/port.h  16 Mar 2005 21:17:50 -
***
*** 112,117 
--- 112,120 
  extern int pg_snprintf(char *str, size_t count, const char *fmt,...)
  /* This extension allows gcc to check the format string */
  __attribute__((format(printf, 3, 4)));
+ extern int pg_sprintf(char *str, const char *fmt,...)
+ /* This extension allows gcc to check the format string */
+ __attribute__((format(printf, 2, 3)));
  extern int pg_fprintf(FILE *stream, const char *fmt,...)
  /* This extension allows gcc to check the format string */
  __attribute__((format(printf, 2, 3)));
***
*** 127,137 
--- 130,142 
  #ifdef __GNUC__
  #define   vsnprintf(...)  pg_vsnprintf(__VA_ARGS__)
  #define snprintf(...) pg_snprintf(__VA_ARGS__)
+ #define sprintf(...)  pg_sprintf(__VA_ARGS__)
  #define fprintf(...)  pg_fprintf(__VA_ARGS__)
  #define printf(...)   pg_printf(__VA_ARGS__)
  #else
  #define vsnprintf pg_vsnprintf
  #define snprintf  pg_snprintf
+ #define sprintf   pg_sprintf
  #define fprintf   pg_fprintf
  #define printfpg_printf
  #endif
Index: src/port/snprintf.c
===
RCS file: /cvsroot/pgsql/src/port/snprintf.c,v
retrieving revision 1.22
diff -c -c -r1.22 snprintf.c
*** src/port/snprintf.c 16 Mar 2005 15:12:18 -  1.22
--- src/port/snprintf.c 16 Mar 2005 21:17:51 -
***
*** 67,80 
  
  /*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.22 
2005/03/16 15:12:18 momjian Exp $";*/
  
- int   pg_snprintf(char *str, size_t count, const char 
*fmt,...);
- int   pg_vsnprintf(char *str, size_t count, const char *fmt, 
va_list args);
- int   pg_printf(const char *format,...);
  static void dopr(char *buffer, const char *format, va_list args, char *end);
  
  /* Prevent recursion */
  #undefvsnprintf
  #undefsnprintf
  #undeffprintf
  #undefprintf
  
--- 67,78 
  
  /*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.22 
2005/03/16 15:12:18 momjian Exp $";*/
  
  static void dopr(char *buffer, const char *format, va_list args, char *end);
  
  /* Prevent recursion */
  #undefvsnprintf
  #undefsnprintf
+ #undefsprintf
  #undeffprintf
  #undefprintf
  
***
*** 104,121 
  }
  
  int
  pg_fprintf(FILE *stream, const char *fmt,...)
  {
int len;
va_list args;
!   char   *buffer[4096];
char   *p;
  
va_start(args, fmt);
!   len = pg_vsnprintf((ch

Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-16 Thread Bruce Momjian
Nicolai Tufar wrote:
> On Wed, 16 Mar 2005 01:00:21 -0500 (EST), Bruce Momjian
>  wrote:
> > 
> > I have applied a modified version of your patch, attached.
> 

Here is a patch that fixes the %*$ case.

FYI, I am going to pgindent snprintf.c to make it consistent so please
us CVS for your next patch.

I will work on your Win32 compile problem next.

-- 
  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
Index: src/port/snprintf.c
===
RCS file: /cvsroot/pgsql/src/port/snprintf.c,v
retrieving revision 1.20
diff -c -c -r1.20 snprintf.c
*** src/port/snprintf.c 16 Mar 2005 06:00:58 -  1.20
--- src/port/snprintf.c 16 Mar 2005 14:59:00 -
***
*** 467,481 
fmtparptr[i]->charvalue = va_arg(args, int);
break;
case FMTLEN:
!   if (i + 1 < fmtpos && fmtpar[i + 1].func != FMTWIDTH)
!   fmtpar[i + 1].len = va_arg(args, int);
/* For "%*.*f", use the second arg */
!   if (i + 2 < fmtpos && fmtpar[i + 1].func == FMTWIDTH)
!   fmtpar[i + 2].len = va_arg(args, int);
break;
case FMTWIDTH:
if (i + 1 < fmtpos)
!   fmtpar[i + 1].maxwidth = fmtpar[i + 
1].precision =

va_arg(args, int);
break;
}
--- 467,481 
fmtparptr[i]->charvalue = va_arg(args, int);
break;
case FMTLEN:
!   if (i + 1 < fmtpos && fmtparptr[i + 1]->func != 
FMTWIDTH)
!   fmtparptr[i + 1]->len = va_arg(args, int);
/* For "%*.*f", use the second arg */
!   if (i + 2 < fmtpos && fmtparptr[i + 1]->func == 
FMTWIDTH)
!   fmtparptr[i + 2]->len = va_arg(args, int);
break;
case FMTWIDTH:
if (i + 1 < fmtpos)
!   fmtparptr[i + 1]->maxwidth = fmtparptr[i + 
1]->precision =

va_arg(args, int);
break;
}

---(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

2005-03-16 Thread Nicolai Tufar
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

2005-03-15 Thread Bruce Momjian

I have applied a modified version of your patch, attached.

initdb will not work without %*s support, so I had to add that.  Please
look over my work.  I don't think i handle %*1$ but I am not even sure
what that means or if our translators would ever use such a thing.  You
can probably test that better than I can.

Your patch didn't handle signed vs. unsigned va_arg values properly..

There were also maxwidth tests around 's' that I had to remove to
support %*.  I think those tests are down in fmtstr too but please
check.

initdb and regression pass.

---

Nicolai Tufar wrote:
> 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
> > 
> > 
> >

[ Attachment, skipping... ]

-- 
  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
Index: src/port/snprintf.c
===
RCS file: /cvsroot/pgsql/src/port/snprintf.c,v
retrieving revision 1.19
diff -c -c -r1.19 snprintf.c
*** src/port/snprintf.c 12 Mar 2005 04:00:56 -  1.19
--- src/port/snprintf.c 16 Mar 2005 05:46:33 -
***
*** 151,170 
  
  #define   FMTSTR  1
  #define   FMTNUM  2
! #define   FMTFLOAT3
! #define   FMTCHAR 4
  
  static void
  dopr(char *buffer, const char *format, va_list args, char *end)
  {
int ch;
-   int64   value;
-   double  fvalue;
int longlongflag = 0;
int longflag = 0;
int pointflag = 0;
int maxwidth = 0;
-   char   *strvalue;
int ljust;
int len;
int zpad;
--- 151,170 
  
  #define   FMTSTR  1
  #define   FMTNUM  2
! #define   FMTNUM_U3
! #define   FMTFLOAT4
! #define   FMTCHAR 5
! #define   FMTWIDTH6
! #define   FMTLEN  7
  
  static void
  dopr(char *buffer, const char *format, va_list args, char *end)
  {
int ch;
int longlongflag = 0;
int longflag = 0;
int pointflag = 0;
int maxwidth = 0;
int ljust;
int len;
int zpad;
***
*** 173,178 
--- 173,179 
const char* fmtbegin;
int fmtpos = 1;
int realpos = 0;
+   int precision;
int position;
char*output;
int percents = 1;
***
*** 195,200 
--- 196,203 
int pointflag;
charfunc;
int realpos;
+   int longflag;
+   int longlongflag;
} *fmtpar, **fmtparptr;
  
/* Create enough structures to hold all arguments */
***
*** 229,240 
longflag = longlongflag = pointflag = 0;
fmtbegin = format - 1;
realpos = 0;
!   position = 0;
nextch:
ch = *format++;
switch (ch)
{
!   case 0:
goto performpr;
   

Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-14 Thread Tom Lane
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

2005-03-14 Thread Bruce Momjian
Tom Lane wrote:
> 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.

Couldn't you spin through the varargs and reconstruct a new one?
Is there no way to create a va_arg va_list structure in C?

-- 
  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 6: Have you searched our list archives?

   http://archives.postgresql.org


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-14 Thread Tom Lane
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

2005-03-14 Thread Bruce Momjian
Nicolai Tufar 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.

(Your new patch is in the queue.)

I have been thinking about our current snprintf() implementation.  As I
remember, we use snprintf mostly for an snprintf that doesn't support
long long, and now those that don't support %$.  I am wondering if we
should just process long long args and %$ args, and pass everything else
to the native snprintf.

In fact, one trick would be to substitute long long and %$ in the printf
format string, and then pass that to the native libc printf, with
adjustments for the printf format arguments.  That might be simpler than
emulating all of snprintf.

FYI, now that we are using pg_snprintf macros the native snprintf is
available to us.

Anyway, I am sure there are some platforms that don't have vsnprint or
snprintf, but could we just say we don't support them, or emulate one of
we only have the other?

-- 
  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 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

2005-03-12 Thread Nicolai Tufar
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

2005-03-11 Thread Nicolai Tufar
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

2005-03-11 Thread Bruce Momjian
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian  writes:
> > >> I'm not sure that macros can have variable number of arguments on all
> > >> supported platforms. I've been burnt by this before.
> > 
> > > The actual patch is:
> > 
> > >   + #ifdef __GNUC__
> > >   + #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
> > >   + #define snprintf(...) pg_snprintf(__VA_ARGS__)
> > >   + #define printf(...)   pg_printf(__VA_ARGS__)
> > >   + #else
> > >   + #define vsnprintf pg_vsnprintf
> > >   + #define snprintf  pg_snprintf
> > >   + #define printfpg_printf
> > >   + #endif
> > 
> > Uh, why bother with the different approach for gcc?
> 
> Because if we don't do that then the code above fails:
>   
>   extern int pg_snprintf(char *str, size_t count, const char *fmt,...)
>   /* This extension allows gcc to check the format string */
>   __attribute__((format(printf, 3, 4)));
> 
> The issue is that the "printf" here is interpreted specially by the
> compiler to mean "check arguments as printf".  If the preprocessor
> changes that, we get a failure.  The good news is that only gcc supports
> arg checking using __attribute__ and it also supports the __VA_ARGS__
> macros.  What I think we do lose is argument checking for non-gcc, but
> this seems as close as we can get.

I am adding a comment explaining why those macros are used.

-- 
  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


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-11 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  writes:
> >> I'm not sure that macros can have variable number of arguments on all
> >> supported platforms. I've been burnt by this before.
> 
> > The actual patch is:
>   
> > + #ifdef __GNUC__
> > + #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
> > + #define snprintf(...) pg_snprintf(__VA_ARGS__)
> > + #define printf(...)   pg_printf(__VA_ARGS__)
> > + #else
> > + #define vsnprintf pg_vsnprintf
> > + #define snprintf  pg_snprintf
> > + #define printfpg_printf
> > + #endif
> 
> Uh, why bother with the different approach for gcc?

Because if we don't do that then the code above fails:

extern int pg_snprintf(char *str, size_t count, const char *fmt,...)
/* This extension allows gcc to check the format string */
__attribute__((format(printf, 3, 4)));

The issue is that the "printf" here is interpreted specially by the
compiler to mean "check arguments as printf".  If the preprocessor
changes that, we get a failure.  The good news is that only gcc supports
arg checking using __attribute__ and it also supports the __VA_ARGS__
macros.  What I think we do lose is argument checking for non-gcc, but
this seems as close as we can get.

> Also, what happened to fprintf?  We're going to need that too for
> localization of the client programs.

It was never there.  I will add it 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 7: don't forget to increase your free space map settings


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-11 Thread Tom Lane
Bruce Momjian  writes:
>> I'm not sure that macros can have variable number of arguments on all
>> supported platforms. I've been burnt by this before.

> The actual patch is:

>   + #ifdef __GNUC__
>   + #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
>   + #define snprintf(...) pg_snprintf(__VA_ARGS__)
>   + #define printf(...)   pg_printf(__VA_ARGS__)
>   + #else
>   + #define vsnprintf pg_vsnprintf
>   + #define snprintf  pg_snprintf
>   + #define printfpg_printf
>   + #endif

Uh, why bother with the different approach for gcc?

Also, what happened to fprintf?  We're going to need that too for
localization of the client programs.

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

2005-03-11 Thread Bruce Momjian
[EMAIL PROTECTED] wrote:
> > Tom Lane wrote:
> >> [EMAIL PROTECTED] writes:
> >> >>> Please see my posting about using a macro for snprintf.
> >>
> >> > Wasn't the issue about odd behavior of the Win32 linker choosing the
> >> wrong
> >> > vnsprintf?
> >>
> >> You're right, the point about the macro was to avoid linker weirdness on
> >> Windows.  We need to do that part in any case.  I think Bruce confused
> >> that issue with the one about whether our version supported %n$
> >> adequately ... which it doesn't just yet ...
> >
> > Perhaps I am reading old email in this reply but I thought I should
> > clarify:
> >
> > Once we do:
> >
> > #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
> > #define snprintf(...) pg_snprintf(__VA_ARGS__)
> > #define printf(...)   pg_printf(__VA_ARGS__)
> 
> 
> I'm not sure that macros can have variable number of arguments on all
> supported platforms. I've been burnt by this before.

The actual patch is:

+ #ifdef __GNUC__
+ #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
+ #define snprintf(...) pg_snprintf(__VA_ARGS__)
+ #define printf(...)   pg_printf(__VA_ARGS__)
+ #else
+ #define vsnprintf pg_vsnprintf
+ #define snprintf  pg_snprintf
+ #define printfpg_printf
+ #endif

-- 
  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 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

2005-03-11 Thread pgsql
> Tom Lane wrote:
>> [EMAIL PROTECTED] writes:
>> >>> Please see my posting about using a macro for snprintf.
>>
>> > Wasn't the issue about odd behavior of the Win32 linker choosing the
>> wrong
>> > vnsprintf?
>>
>> You're right, the point about the macro was to avoid linker weirdness on
>> Windows.  We need to do that part in any case.  I think Bruce confused
>> that issue with the one about whether our version supported %n$
>> adequately ... which it doesn't just yet ...
>
> Perhaps I am reading old email in this reply but I thought I should
> clarify:
>
> Once we do:
>
>   #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
>   #define snprintf(...) pg_snprintf(__VA_ARGS__)
>   #define printf(...)   pg_printf(__VA_ARGS__)


I'm not sure that macros can have variable number of arguments on all
supported platforms. I've been burnt by this before.


---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-11 Thread Bruce Momjian
Nicolai Tufar wrote:
> On Thu, 10 Mar 2005 16:26:47 -0500 (EST), Bruce Momjian
>  wrote:
> > 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.
> 
> Oh, Bruce. It will be the best solution. I was worried about
> the problems with my modifications to snprintf.c Tom Lane
> pointed out. But if we really separate snprintf() used by
> messages and snprintf() used by the like of 
> src/backend/utils/adt/int8.c then we are safe. We can claim
> current release safe and I will modify src/port/snprintf.c at my
> leisure later. I will try out your modifications tomorrow. It
> is late here and I have a PostgreSQL class to to teach 
> tomorrow ;)
> 
> I still think that it is more convenient to rip off current
> implementation of snprintf.c and replace it with a very much
> stripped down of Trio's one. I will work on it and try to get
> a patch in one week's time. Thank you all for your patience.

I am not heading in the direction of using a different snprintf for
messages and for int8.c.  I am just renaming the calls via macros so we
don't leak snprintf from libpq.

One new idea I had was to have pg_snprintf() look over the format string
and adjust the arguments to match what the format string is requesting,
remove %$ from the format string, and then pass it to the native libc
snprintf().  That might be the easiest solution for the many platforms
with a good snprintf but not %$ support.

Is that possible/easier?

-- 
  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: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-11 Thread Bruce Momjian
Tom Lane wrote:
> [EMAIL PROTECTED] writes:
> >>> Please see my posting about using a macro for snprintf.
> 
> > Wasn't the issue about odd behavior of the Win32 linker choosing the wrong
> > vnsprintf?
> 
> You're right, the point about the macro was to avoid linker weirdness on
> Windows.  We need to do that part in any case.  I think Bruce confused
> that issue with the one about whether our version supported %n$
> adequately ... which it doesn't just yet ...

Perhaps I am reading old email in this reply but I thought I should
clarify:

Once we do:

#define vsnprintf(...)pg_vsnprintf(__VA_ARGS__)
#define snprintf(...) pg_snprintf(__VA_ARGS__)
#define printf(...)   pg_printf(__VA_ARGS__)

we also rename the functions in snprintf.c to pg_* names so there is no
longer a conflict with the system libc versions.

The macro is to prevent our snprintf from leaking out of libraries like
libpq, not to fix the win32 linker problem, which we already had fixed
by reordering the entries in the C file.

Perhaps the macro idea originally came as a fix for Win32 but it is much
larger that that 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 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-10 Thread Nicolai Tufar
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

2005-03-10 Thread Tom Lane
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

2005-03-10 Thread Nicolai Tufar
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

2005-03-10 Thread Bruce Momjian
[EMAIL PROTECTED] wrote:
> > Tom Lane wrote:
> >> 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.
> >
> > Oh, so our existing implementation doesn't even meet our needs. OK.
> 
> Wasn't the issue about odd behavior of the Win32 linker choosing the wrong
> vnsprintf?

Ah, but with my new patch to be applied tomorrow to use macros and
rename to pg_snprintf there no longer is any conflict with the system
versions of these functions.

-- 
  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: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-10 Thread Tom Lane
[EMAIL PROTECTED] writes:
>>> Please see my posting about using a macro for snprintf.

> Wasn't the issue about odd behavior of the Win32 linker choosing the wrong
> vnsprintf?

You're right, the point about the macro was to avoid linker weirdness on
Windows.  We need to do that part in any case.  I think Bruce confused
that issue with the one about whether our version supported %n$
adequately ... which it doesn't just yet ...

regards, tom lane

---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-10 Thread pgsql
> Tom Lane wrote:
>> 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.
>
> Oh, so our existing implementation doesn't even meet our needs. OK.

Wasn't the issue about odd behavior of the Win32 linker choosing the wrong
vnsprintf?

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-10 Thread Bruce Momjian
Tom Lane wrote:
> 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.

Oh, so our existing implementation doesn't even meet our needs. OK.

-- 
  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


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-10 Thread Tom Lane
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

2005-03-10 Thread Nicolai Tufar
On Thu, 10 Mar 2005 16:26:47 -0500 (EST), Bruce Momjian
 wrote:
> 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.

Oh, Bruce. It will be the best solution. I was worried about
the problems with my modifications to snprintf.c Tom Lane
pointed out. But if we really separate snprintf() used by
messages and snprintf() used by the like of 
src/backend/utils/adt/int8.c then we are safe. We can claim
current release safe and I will modify src/port/snprintf.c at my
leisure later. I will try out your modifications tomorrow. It
is late here and I have a PostgreSQL class to to teach 
tomorrow ;)

I still think that it is more convenient to rip off current
implementation of snprintf.c and replace it with a very much
stripped down of Trio's one. I will work on it and try to get
a patch in one week's time. Thank you all for your patience.


Best regards,
Nicolai Tufar


> 
> --
>   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])
>

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-10 Thread Bruce Momjian
Nicolai Tufar wrote:
> 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?

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.

-- 
  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: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-10 Thread Nicolai Tufar
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

2005-03-09 Thread Bruce Momjian
Nicolai Tufar wrote:
> 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?

Yes, maybe just add the proper %$ handling from Trio to what we have
now.

> 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?

I think the proper direction is not to export snprintf() from libpq so
that user applications will use the native snprintf, but our code can
use our custom version.

I will work on a patch 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 7: don't forget to increase your free space map settings


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-09 Thread pgsql
>From what I recall from the conversation, I would say rename the vsprintf
and the sprintf functions in postgres to pq_vsnprintf and pq_snprintf.
Define a couple macros: (in some common header, pqprintf.h?)

#define snprintf pq_snprintf
#define vsnprintf pq_snprintf

Then just maintain the postgres forms of printf which have seemed to be OK
except that on Win32 vnsprintf, although in the same object file was not
being used.



> 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
>


---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-09 Thread Nicolai Tufar
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

2005-03-02 Thread Bruce Momjian
Tom Lane wrote:
> 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.

Done.

> > 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 snprintf  pq_snprintf
> 
> and not have to uglify the source code.

Yes, this is what I was thinking of too.  I think it would need a macro
in libpq to map the libc names to the pq_* names, and a separate /port C
file that maps the normal libc names to the pg_* names.  For client
applications and the backend, this new C file would catch all the
snprintf calls, while for libpq the pg_* calls would be used directly
and the new C file with the libc symbols would not be pulled in.

Does that sound like a plan?

The reason we can't just use the macro everwhere is that we don't want
applications using libpq to all be using pg_* functions, only psql and
our own.  The only other solution I can think of is to make sure all
client apps use FRONTEND as a define and trigger the macros from libc
names to pg_* names on that.

-- 
  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 7: don't forget to increase your free space map settings


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-02 Thread pgsql
> 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 snprintf  pq_snprintf

Didn't I suggest that earlier? :) Also, since it is vsnprintf that seems
to be a bigger issue:

#define vsnprintf pq_vsnprintf

---(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

2005-03-02 Thread Tom Lane
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: [pgsql-hackers-win32] [HACKERS] snprintf causes regression

2005-03-02 Thread pgsql
>
> Yes, strangly the Window's linker is fine because libpqdll.def defines
> what symbols are exported.  I don't think Unix has that capability.

A non-static "public" function in a Windows DLL is not available for
dynamic linking unless explicitly declared as dll export. This behavior is
completely different than UNIX shared libraries.

Windows static libraries operate completely differently than Windows DLLs,
they work like their UNIX equivilents.

So, if you create an snprintf function in code that will be in both a
static and dynamic library, the static library may have conflicts where as
the dynamic one will not.

Don't you love Windows?

---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests

2005-03-02 Thread Bruce Momjian
Tom Lane wrote:
> 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.

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.

> 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?

I just tested on BSD/OS and a program with a single printf() call does
call our printf if libpq is used on the link line.  The program makes no
libpq calls at all.

> Third thought: Windows' linker seems to be broken enough that it may
> create problems of this ilk that exist on no other platform.

Yes, strangly the Window's linker is fine because libpqdll.def defines
what symbols are exported.  I don't think Unix has that capability.

Is there any way we can have just gettext() call our snprintf under a
special name?

-- 
  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

2005-03-02 Thread pgsql
>
> 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

2005-03-02 Thread Nicolai Tufar
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

2005-03-01 Thread Bruce Momjian
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

2005-03-01 Thread Bruce Momjian
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

2005-03-01 Thread Tom Lane
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

2005-03-01 Thread Bruce Momjian
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

2005-03-01 Thread Bruce Momjian
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

2005-03-01 Thread Tom Lane
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

2005-03-01 Thread Bruce Momjian
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

2005-03-01 Thread Nicolai Tufar
On Tue, 1 Mar 2005 16:20:50 -0500 (EST), [EMAIL PROTECTED]
<[EMAIL PROTECTED]> wrote:
> 
> 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?

Defenitely not. I checked it by placing debugging print 
statements in code.

---(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

2005-03-01 Thread Tom Lane
[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

2005-03-01 Thread pgsql
> 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.
>

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?


---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail

2005-03-01 Thread Nicolai Tufar
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

2005-03-01 Thread Magnus Hagander
>> 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]


  1   2   >