snprintf() argument reordering not working under Windows in 8.1

Started by Nicolai Tufarabout 20 years ago45 messages
#1Nicolai Tufar
ntufar@gmail.com

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

#2Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Nicolai Tufar (#1)
2 attachment(s)
Re: [HACKERS] snprintf() argument reordering not working under Windows

Nicolai Tufar wrote:

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.

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, 10000) + 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.

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.

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.

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.

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.

So, I think it was Magnus who said that Win32 didn' need and couldn't
use our snprintf. Magnus, any ideas?

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

Attachments:

/pg/port/snprintf.ctext/plainDownload
/*
 * Copyright (c) 1983, 1995, 1996 Eric P. Allman
 * Copyright (c) 1988, 1993
 *	The Regents of the University of California.  All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *	  notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *	  notice, this list of conditions and the following disclaimer in the
 *	  documentation and/or other materials provided with the distribution.
 * 3. All advertising materials mentioning features or use of this software
 *	  must display the following acknowledgement:
 *	This product includes software developed by the University of
 *	California, Berkeley and its contributors.
 * 4. Neither the name of the University nor the names of its contributors
 *	  may be used to endorse or promote products derived from this software
 *	  without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.	IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

#include "c.h"

#ifndef WIN32
#include <sys/ioctl.h>
#endif
#include <sys/param.h>


/*
**	SNPRINTF, VSNPRINT -- counted versions of printf
**
**	These versions have been grabbed off the net.  They have been
**	cleaned up to compile properly and support for .precision and
**	%lx has been added.
*/

/**************************************************************
 * Original:
 * Patrick Powell Tue Apr 11 09:48:21 PDT 1995
 * A bombproof version of doprnt (dopr) included.
 * Sigh.  This sort of thing is always nasty do deal with.	Note that
 * the version here does not include floating point. (now it does ... tgl)
 *
 * snprintf() is used instead of sprintf() as it does limit checks
 * for string length.  This covers a nasty loophole.
 *
 * The other functions are there to prevent NULL pointers from
 * causing nasty effects.
 **************************************************************/

/*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.29 2005/10/15 02:49:51 momjian Exp $";*/

static int dopr(FILE *stream, char *buffer, const char *format, va_list args, char *end);

/* Prevent recursion */
#undef	vsnprintf
#undef	snprintf
#undef	sprintf
#undef	fprintf
#undef	printf

	
static int
pg_fvsnprintf(FILE *stream, char *str, size_t count, const char *fmt, va_list args)
{
	char	   *end = NULL;
	int			len;

	if (str)
	{
		str[0] = '\0';
		end = str + count - 1;
	}
	len = dopr(stream, str, fmt, args, end);
	if (str && count > 0)
		end[0] = '\0';
	return len;
}

int
pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args)
{
	return pg_fvsnprintf(NULL, str, count, fmt, args);
}

int
pg_snprintf(char *str, size_t count, const char *fmt,...)
{
	int			len;
	va_list		args;

	va_start(args, fmt);
	len = pg_fvsnprintf(NULL, str, count, fmt, args);
	va_end(args);
	return len;
}

int
pg_sprintf(char *str, const char *fmt,...)
{
	int			len;
	va_list		args;
	char		buffer[8192];	/* arbitrary limit */

	va_start(args, fmt);
	len = pg_fvsnprintf(NULL, buffer, (size_t) 4096, fmt, args);
	va_end(args);
	/* limit output to string */
	StrNCpy(str, buffer, (len + 1 < 8192) ? len + 1 : 8192);
	return len;
}

int
pg_fprintf(FILE *stream, const char *fmt,...)
{
	int			len;
	va_list		args;

	va_start(args, fmt);
	len = pg_fvsnprintf(stream, NULL, 0, fmt, args);
	va_end(args);
	return len;
}

int
pg_printf(const char *fmt,...)
{
	int			len;
	va_list		args;

	va_start(args, fmt);
	len = pg_fvsnprintf(stdout, NULL, 0, fmt, args);
	va_end(args);

	return len;
}

static int	adjust_sign(int is_negative, int forcesign, int *signvalue);
static void adjust_padlen(int minlen, int vallen, int leftjust, int *padlen);
static void leading_pad(int zpad, int *signvalue, int *padlen, char *end,
			char **output, FILE *stream, int *outlen);
static void trailing_pad(int *padlen, char *end, char **output,
						 FILE *stream, int *outlen);

static void fmtstr(char *value, int leftjust, int minlen, int maxwidth,
	   char *end, char **output, FILE *stream, int *outlen);
static void fmtint(int64 value, int base, int dosign, int forcesign,
	   int leftjust, int minlen, int zpad, char *end, char **output,
	   FILE *stream, int *outlen);
static void fmtfloat(double value, char type, int forcesign,
 int leftjust, int minlen, int zpad, int precision, int pointflag, char *end,
		 char **output, FILE *stream, int *outlen);
static void dostr(char *str, int cut, char *end, char **output, FILE *stream,
				  int *outlen);
static void dopr_outch(int c, char *end, char **output, FILE *stream, int *outlen);

#define FMTSTR		1
#define FMTNUM		2
#define FMTNUM_U	3
#define FMTFLOAT	4
#define FMTCHAR		5
#define FMTWIDTH	6
#define FMTLEN		7

/*
 * dopr(): poor man's version of doprintf
 */

static int
dopr(FILE *stream, char *buffer, const char *format, va_list args, char *end)
{
	int			ch;
	int			longlongflag;
	int			longflag;
	int			pointflag;
	int			maxwidth;
	int			leftjust;
	int			minlen;
	int			zpad;
	int			forcesign;
	int			i;
	const char *format_save;
	const char *fmtbegin;
	int			fmtpos = 1;
	int			realpos = 0;
	int			precision;
	int			position;
	char	   *output;
	int			nargs = 1;
	int			outlen = 0;
	const char *p;
	struct fmtpar
	{
		const char *fmtbegin;
		const char *fmtend;
		void	   *value;
		int64		numvalue;
		double		fvalue;
		int			charvalue;
		int			leftjust;
		int			minlen;
		int			zpad;
		int			maxwidth;
		int			base;
		int			dosign;
		int			forcesign;
		char		type;
		int			precision;
		int			pointflag;
		char		func;
		int			realpos;
		int			longflag;
		int			longlongflag;
	}		   *fmtpar, **fmtparptr;

	/*
	 * Create enough structures to hold all arguments.	This overcounts, eg
	 * not all '*' characters are necessarily arguments, but it's not worth
	 * being exact.
	 */
	for (p = format; *p != '\0'; p++)
		if (*p == '%' || *p == '*')
			nargs++;

	/* Need to use malloc() because memory system might not be started yet. */
	if ((fmtpar = malloc(sizeof(struct fmtpar) * nargs)) == NULL)
	{
		fprintf(stderr, _("out of memory\n"));
		exit(1);
	}
	if ((fmtparptr = malloc(sizeof(struct fmtpar *) * nargs)) == NULL)
	{
		fprintf(stderr, _("out of memory\n"));
		exit(1);
	}

	format_save = format;

	output = buffer;
	while ((ch = *format++))
	{
		if (ch == '%')
		{
			leftjust = minlen = zpad = forcesign = maxwidth = 0;
			longflag = longlongflag = pointflag = 0;
			fmtbegin = format - 1;
			realpos = 0;
			position = precision = 0;
	nextch:
			ch = *format++;
			switch (ch)
			{
				case '\0':
					goto performpr;
				case '-':
					leftjust = 1;
					goto nextch;
				case '+':
					forcesign = 1;
					goto nextch;
				case '0':	/* set zero padding if minlen not set */
					if (minlen == 0 && !pointflag)
						zpad = '0';
				case '1':
				case '2':
				case '3':
				case '4':
				case '5':
				case '6':
				case '7':
				case '8':
				case '9':
					if (!pointflag)
					{
						minlen = minlen * 10 + ch - '0';
						position = position * 10 + ch - '0';
					}
					else
					{
						maxwidth = maxwidth * 10 + ch - '0';
						precision = precision * 10 + ch - '0';
					}
					goto nextch;
				case '$':
					realpos = position;
					minlen = 0;
					goto nextch;
				case '*':
					MemSet(&fmtpar[fmtpos], 0, sizeof(fmtpar[fmtpos]));
					if (!pointflag)
						fmtpar[fmtpos].func = FMTLEN;
					else
						fmtpar[fmtpos].func = FMTWIDTH;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					goto nextch;
				case '.':
					pointflag = 1;
					goto nextch;
				case 'l':
					if (longflag)
						longlongflag = 1;
					else
						longflag = 1;
					goto nextch;
				case 'h':
					/* ignore */
					goto nextch;
#ifdef NOT_USED

					/*
					 * We might export this to client apps so we should
					 * support 'qd' and 'I64d'(MinGW) also in case the
					 * native version does.
					 */
				case 'q':
					longlongflag = 1;
					longflag = 1;
					goto nextch;
				case 'I':
					if (*format == '6' && *(format + 1) == '4')
					{
						format += 2;
						longlongflag = 1;
						longflag = 1;
						goto nextch;
					}
					break;
#endif
				case 'u':
				case 'U':
					fmtpar[fmtpos].longflag = longflag;
					fmtpar[fmtpos].longlongflag = longlongflag;
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].base = 10;
					fmtpar[fmtpos].dosign = 0;
					fmtpar[fmtpos].forcesign = forcesign;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].func = FMTNUM_U;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 'o':
				case 'O':
					fmtpar[fmtpos].longflag = longflag;
					fmtpar[fmtpos].longlongflag = longlongflag;
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].base = 8;
					fmtpar[fmtpos].dosign = 0;
					fmtpar[fmtpos].forcesign = forcesign;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].func = FMTNUM_U;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 'd':
				case 'D':
					fmtpar[fmtpos].longflag = longflag;
					fmtpar[fmtpos].longlongflag = longlongflag;
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].base = 10;
					fmtpar[fmtpos].dosign = 1;
					fmtpar[fmtpos].forcesign = forcesign;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].func = FMTNUM;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 'x':
					fmtpar[fmtpos].longflag = longflag;
					fmtpar[fmtpos].longlongflag = longlongflag;
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].base = 16;
					fmtpar[fmtpos].dosign = 0;
					fmtpar[fmtpos].forcesign = forcesign;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].func = FMTNUM_U;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 'X':
					fmtpar[fmtpos].longflag = longflag;
					fmtpar[fmtpos].longlongflag = longlongflag;
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].base = -16;
					fmtpar[fmtpos].dosign = 1;
					fmtpar[fmtpos].forcesign = forcesign;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].func = FMTNUM_U;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 's':
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].maxwidth = maxwidth;
					fmtpar[fmtpos].func = FMTSTR;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 'c':
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].func = FMTCHAR;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case 'e':
				case 'E':
				case 'f':
				case 'g':
				case 'G':
					fmtpar[fmtpos].fmtbegin = fmtbegin;
					fmtpar[fmtpos].fmtend = format;
					fmtpar[fmtpos].type = ch;
					fmtpar[fmtpos].forcesign = forcesign;
					fmtpar[fmtpos].leftjust = leftjust;
					fmtpar[fmtpos].minlen = minlen;
					fmtpar[fmtpos].zpad = zpad;
					fmtpar[fmtpos].precision = precision;
					fmtpar[fmtpos].pointflag = pointflag;
					fmtpar[fmtpos].func = FMTFLOAT;
					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
					fmtpos++;
					break;
				case '%':
					break;
			}
		}
	}

performpr:
	/* reorder pointers */
	for (i = 1; i < fmtpos; i++)
		fmtparptr[i] = &fmtpar[fmtpar[i].realpos];

	/* assign values */
	for (i = 1; i < fmtpos; i++)
	{
		switch (fmtparptr[i]->func)
		{
			case FMTSTR:
				fmtparptr[i]->value = va_arg(args, char *);
				break;
			case FMTNUM:
				if (fmtparptr[i]->longflag)
				{
					if (fmtparptr[i]->longlongflag)
						fmtparptr[i]->numvalue = va_arg(args, int64);
					else
						fmtparptr[i]->numvalue = va_arg(args, long);
				}
				else
					fmtparptr[i]->numvalue = va_arg(args, int);
				break;
			case FMTNUM_U:
				if (fmtparptr[i]->longflag)
				{
					if (fmtparptr[i]->longlongflag)
						fmtparptr[i]->numvalue = va_arg(args, uint64);
					else
						fmtparptr[i]->numvalue = va_arg(args, unsigned long);
				}
				else
					fmtparptr[i]->numvalue = va_arg(args, unsigned int);
				break;
			case FMTFLOAT:
				fmtparptr[i]->fvalue = va_arg(args, double);
				break;
			case FMTCHAR:
				fmtparptr[i]->charvalue = va_arg(args, int);
				break;
			case FMTLEN:
				{
					int			minlen = va_arg(args, int);
					int			leftjust = 0;

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

	/* do the output */
	output = buffer;
	format = format_save;
	while ((ch = *format++))
	{
		bool skip_output = false;
		
		for (i = 1; i < fmtpos; i++)
		{
			if (ch == '%' && *format == '%')
			{
				format++;
				continue;
			}
			if (fmtpar[i].fmtbegin == format - 1)
			{
				switch (fmtparptr[i]->func)
				{
					case FMTSTR:
						fmtstr(fmtparptr[i]->value, fmtparptr[i]->leftjust,
							   fmtparptr[i]->minlen, fmtparptr[i]->maxwidth,
							   end, (output) ? &output : NULL, stream, &outlen);
						break;
					case FMTNUM:
					case FMTNUM_U:
						fmtint(fmtparptr[i]->numvalue, fmtparptr[i]->base,
							   fmtparptr[i]->dosign, fmtparptr[i]->forcesign,
							   fmtparptr[i]->leftjust, fmtparptr[i]->minlen,
							   fmtparptr[i]->zpad, end,
							   (output) ? &output : NULL, stream, &outlen);
						break;
					case FMTFLOAT:
						fmtfloat(fmtparptr[i]->fvalue, fmtparptr[i]->type,
							 fmtparptr[i]->forcesign, fmtparptr[i]->leftjust,
								 fmtparptr[i]->minlen, fmtparptr[i]->zpad,
							fmtparptr[i]->precision, fmtparptr[i]->pointflag,
								 end, (output) ? &output : NULL, stream, &outlen);
						break;
					case FMTCHAR:
						dopr_outch(fmtparptr[i]->charvalue, end,
								   (output) ? &output : NULL, stream, &outlen);
						break;
				}
				format = fmtpar[i].fmtend;
				skip_output = true;
				break;
			}
		}
		if (!skip_output)
			dopr_outch(ch, end, (output) ? &output : NULL, stream, &outlen);
	}
	if (output)
		*output = '\0';

	free(fmtpar);
	free(fmtparptr);

	return outlen;
}

static void
fmtstr(char *value, int leftjust, int minlen, int maxwidth, char *end,
	   char **output, FILE *stream, int *outlen)
{
	int			padlen,
				vallen;			/* amount to pad */

	if (value == NULL)
		value = "<NULL>";

	vallen = strlen(value);
	if (maxwidth && vallen > maxwidth)
		vallen = maxwidth;

	adjust_padlen(minlen, vallen, leftjust, &padlen);

	while (padlen > 0)
	{
		dopr_outch(' ', end, output, stream, outlen);
		--padlen;
	}
	dostr(value, maxwidth, end, output, stream, outlen);

	trailing_pad(&padlen, end, output, stream, outlen);
}

static void
fmtint(int64 value, int base, int dosign, int forcesign, int leftjust,
	   int minlen, int zpad, char *end, char **output, FILE *stream,
	   int *outlen)
{
	int			signvalue = 0;
	char		convert[64];
	int			vallen = 0;
	int			padlen = 0;		/* amount to pad */
	int			caps = 0;

	/* Handle +/- and %X (uppercase hex) */
	if (dosign && adjust_sign((value < 0), forcesign, &signvalue))
		value = -value;
	if (base < 0)
	{
		caps = 1;
		base = -base;
	}

	/* make integer string */
	do
	{
		convert[vallen++] = (caps ? "0123456789ABCDEF" : "0123456789abcdef")
			[value % (unsigned) base];
		value = (value / (unsigned) base);
	} while (value);
	convert[vallen] = 0;

	adjust_padlen(minlen, vallen, leftjust, &padlen);

	leading_pad(zpad, &signvalue, &padlen, end, output, stream, outlen);

	while (vallen > 0)
		dopr_outch(convert[--vallen], end, output, stream, outlen);

	trailing_pad(&padlen, end, output, stream, outlen);
}

static void
fmtfloat(double value, char type, int forcesign, int leftjust,
		 int minlen, int zpad, int precision, int pointflag, char *end,
		 char **output, FILE *stream, int *outlen)
{
	int			signvalue = 0;
	int			vallen;
	char		fmt[32];
	char		convert[512];
	int			padlen = 0;		/* amount to pad */

	/* we rely on regular C library's sprintf to do the basic conversion */
	if (pointflag)
		sprintf(fmt, "%%.%d%c", precision, type);
	else
		sprintf(fmt, "%%%c", type);

	if (adjust_sign((value < 0), forcesign, &signvalue))
		value = -value;

	vallen = sprintf(convert, fmt, value);

	adjust_padlen(minlen, vallen, leftjust, &padlen);

	leading_pad(zpad, &signvalue, &padlen, end, output, stream, outlen);

	dostr(convert, 0, end, output, stream, outlen);

	trailing_pad(&padlen, end, output, stream, outlen);
}

static void
dostr(char *str, int cut, char *end, char **output, FILE *stream, int *outlen)
{
	if (cut)
		while (*str && cut-- > 0)
			dopr_outch(*str++, end, output, stream, outlen);
	else
		while (*str)
			dopr_outch(*str++, end, output, stream, outlen);
}

static void
dopr_outch(int c, char *end, char **output, FILE *stream, int *outlen)
{
#ifdef NOT_USED
	if (iscntrl((unsigned char) c) && c != '\n' && c != '\t')
	{
		c = '@' + (c & 0x1F);
		if (output)
		{
			if (end == 0 || *output < end)
			{
				*(*output)++ = '^';
				(*outlen)++;
			}
		}
		else
		{
			putc(c, stream);
			(*outlen)++;
		}
	}
#endif
	if (output)
	{
		if (end == 0 || *output < end)
		{
			*(*output)++ = c;
			(*outlen)++;
		}
	}
	else
	{
		putc(c, stream);
		(*outlen)++;
	}
}


static int
adjust_sign(int is_negative, int forcesign, int *signvalue)
{
	if (is_negative)
	{
		*signvalue = '-';
		return true;
	}
	else if (forcesign)
		*signvalue = '+';
	return false;
}


static void
adjust_padlen(int minlen, int vallen, int leftjust, int *padlen)
{
	*padlen = minlen - vallen;
	if (*padlen < 0)
		*padlen = 0;
	if (leftjust)
		*padlen = -*padlen;
}


static void
leading_pad(int zpad, int *signvalue, int *padlen, char *end, char **output,
			FILE *stream, int *outlen)
{
	if (*padlen > 0 && zpad)
	{
		if (*signvalue)
		{
			dopr_outch(*signvalue, end, output, stream, outlen);
			--*padlen;
			*signvalue = 0;
		}
		while (*padlen > 0)
		{
			dopr_outch(zpad, end, output, stream, outlen);
			--*padlen;
		}
	}
	while (*padlen > 0 + (*signvalue != 0))
	{
		dopr_outch(' ', end, output, stream, outlen);
		--*padlen;
	}
	if (*signvalue)
	{
		dopr_outch(*signvalue, end, output, stream, outlen);
		if (*padlen > 0)
			--* padlen;
		if (padlen < 0)
			++padlen;
	}
}


static void
trailing_pad(int *padlen, char *end, char **output, FILE *stream, int *outlen)
{
	while (*padlen < 0)
	{
		dopr_outch(' ', end, output, stream, outlen);
		++*padlen;
	}
}
/pgpatches/snprintftext/plainDownload
Index: src/port/snprintf.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/snprintf.c,v
retrieving revision 1.29
diff -c -c -r1.29 snprintf.c
*** src/port/snprintf.c	15 Oct 2005 02:49:51 -0000	1.29
--- src/port/snprintf.c	3 Dec 2005 05:01:16 -0000
***************
*** 64,70 ****
  
  /*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.29 2005/10/15 02:49:51 momjian Exp $";*/
  
! static void dopr(char *buffer, const char *format, va_list args, char *end);
  
  /* Prevent recursion */
  #undef	vsnprintf
--- 64,70 ----
  
  /*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.29 2005/10/15 02:49:51 momjian Exp $";*/
  
! static int dopr(FILE *stream, char *buffer, const char *format, va_list args, char *end);
  
  /* Prevent recursion */
  #undef	vsnprintf
***************
*** 73,89 ****
  #undef	fprintf
  #undef	printf
  
! int
! pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args)
  {
! 	char	   *end;
  
! 	str[0] = '\0';
! 	end = str + count - 1;
! 	dopr(str, fmt, args, end);
! 	if (count > 0)
  		end[0] = '\0';
! 	return strlen(str);
  }
  
  int
--- 73,100 ----
  #undef	fprintf
  #undef	printf
  
! 	
! static int
! pg_fvsnprintf(FILE *stream, char *str, size_t count, const char *fmt, va_list args)
  {
! 	char	   *end = NULL;
! 	int			len;
  
! 	if (str)
! 	{
! 		str[0] = '\0';
! 		end = str + count - 1;
! 	}
! 	len = dopr(stream, str, fmt, args, end);
! 	if (str && count > 0)
  		end[0] = '\0';
! 	return len;
! }
! 
! int
! pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args)
! {
! 	return pg_fvsnprintf(NULL, str, count, fmt, args);
  }
  
  int
***************
*** 93,99 ****
  	va_list		args;
  
  	va_start(args, fmt);
! 	len = pg_vsnprintf(str, count, fmt, args);
  	va_end(args);
  	return len;
  }
--- 104,110 ----
  	va_list		args;
  
  	va_start(args, fmt);
! 	len = pg_fvsnprintf(NULL, str, count, fmt, args);
  	va_end(args);
  	return len;
  }
***************
*** 103,115 ****
  {
  	int			len;
  	va_list		args;
! 	char		buffer[4096];
  
  	va_start(args, fmt);
! 	len = pg_vsnprintf(buffer, (size_t) 4096, fmt, args);
  	va_end(args);
  	/* limit output to string */
! 	StrNCpy(str, buffer, (len + 1 < 4096) ? len + 1 : 4096);
  	return len;
  }
  
--- 114,126 ----
  {
  	int			len;
  	va_list		args;
! 	char		buffer[8192];	/* arbitrary limit */
  
  	va_start(args, fmt);
! 	len = pg_fvsnprintf(NULL, buffer, (size_t) 4096, fmt, args);
  	va_end(args);
  	/* limit output to string */
! 	StrNCpy(str, buffer, (len + 1 < 8192) ? len + 1 : 8192);
  	return len;
  }
  
***************
*** 118,131 ****
  {
  	int			len;
  	va_list		args;
- 	char		buffer[4096];
- 	char	   *p;
  
  	va_start(args, fmt);
! 	len = pg_vsnprintf(buffer, (size_t) 4096, fmt, args);
  	va_end(args);
- 	for (p = buffer; *p; p++)
- 		putc(*p, stream);
  	return len;
  }
  
--- 129,138 ----
  {
  	int			len;
  	va_list		args;
  
  	va_start(args, fmt);
! 	len = pg_fvsnprintf(stream, NULL, 0, fmt, args);
  	va_end(args);
  	return len;
  }
  
***************
*** 134,166 ****
  {
  	int			len;
  	va_list		args;
- 	char		buffer[4096];
- 	char	   *p;
  
  	va_start(args, fmt);
! 	len = pg_vsnprintf(buffer, (size_t) 4096, fmt, args);
  	va_end(args);
  
- 	for (p = buffer; *p; p++)
- 		putchar(*p);
  	return len;
  }
  
  static int	adjust_sign(int is_negative, int forcesign, int *signvalue);
  static void adjust_padlen(int minlen, int vallen, int leftjust, int *padlen);
  static void leading_pad(int zpad, int *signvalue, int *padlen, char *end,
! 			char **output);
! static void trailing_pad(int *padlen, char *end, char **output);
  
  static void fmtstr(char *value, int leftjust, int minlen, int maxwidth,
! 	   char *end, char **output);
  static void fmtint(int64 value, int base, int dosign, int forcesign,
! 	   int leftjust, int minlen, int zpad, char *end, char **output);
  static void fmtfloat(double value, char type, int forcesign,
   int leftjust, int minlen, int zpad, int precision, int pointflag, char *end,
! 		 char **output);
! static void dostr(char *str, int cut, char *end, char **output);
! static void dopr_outch(int c, char *end, char **output);
  
  #define FMTSTR		1
  #define FMTNUM		2
--- 141,172 ----
  {
  	int			len;
  	va_list		args;
  
  	va_start(args, fmt);
! 	len = pg_fvsnprintf(stdout, NULL, 0, fmt, args);
  	va_end(args);
  
  	return len;
  }
  
  static int	adjust_sign(int is_negative, int forcesign, int *signvalue);
  static void adjust_padlen(int minlen, int vallen, int leftjust, int *padlen);
  static void leading_pad(int zpad, int *signvalue, int *padlen, char *end,
! 			char **output, FILE *stream, int *outlen);
! static void trailing_pad(int *padlen, char *end, char **output,
! 						 FILE *stream, int *outlen);
  
  static void fmtstr(char *value, int leftjust, int minlen, int maxwidth,
! 	   char *end, char **output, FILE *stream, int *outlen);
  static void fmtint(int64 value, int base, int dosign, int forcesign,
! 	   int leftjust, int minlen, int zpad, char *end, char **output,
! 	   FILE *stream, int *outlen);
  static void fmtfloat(double value, char type, int forcesign,
   int leftjust, int minlen, int zpad, int precision, int pointflag, char *end,
! 		 char **output, FILE *stream, int *outlen);
! static void dostr(char *str, int cut, char *end, char **output, FILE *stream,
! 				  int *outlen);
! static void dopr_outch(int c, char *end, char **output, FILE *stream, int *outlen);
  
  #define FMTSTR		1
  #define FMTNUM		2
***************
*** 174,181 ****
   * dopr(): poor man's version of doprintf
   */
  
! static void
! dopr(char *buffer, const char *format, va_list args, char *end)
  {
  	int			ch;
  	int			longlongflag;
--- 180,187 ----
   * dopr(): poor man's version of doprintf
   */
  
! static int
! dopr(FILE *stream, char *buffer, const char *format, va_list args, char *end)
  {
  	int			ch;
  	int			longlongflag;
***************
*** 195,200 ****
--- 201,207 ----
  	int			position;
  	char	   *output;
  	int			nargs = 1;
+ 	int			outlen = 0;
  	const char *p;
  	struct fmtpar
  	{
***************
*** 246,463 ****
  	output = buffer;
  	while ((ch = *format++))
  	{
! 		switch (ch)
  		{
! 			case '%':
! 				leftjust = minlen = zpad = forcesign = maxwidth = 0;
! 				longflag = longlongflag = pointflag = 0;
! 				fmtbegin = format - 1;
! 				realpos = 0;
! 				position = precision = 0;
! 		nextch:
! 				ch = *format++;
! 				switch (ch)
! 				{
! 					case '\0':
! 						goto performpr;
! 					case '-':
! 						leftjust = 1;
! 						goto nextch;
! 					case '+':
! 						forcesign = 1;
! 						goto nextch;
! 					case '0':	/* set zero padding if minlen not set */
! 						if (minlen == 0 && !pointflag)
! 							zpad = '0';
! 					case '1':
! 					case '2':
! 					case '3':
! 					case '4':
! 					case '5':
! 					case '6':
! 					case '7':
! 					case '8':
! 					case '9':
! 						if (!pointflag)
! 						{
! 							minlen = minlen * 10 + ch - '0';
! 							position = position * 10 + ch - '0';
! 						}
! 						else
! 						{
! 							maxwidth = maxwidth * 10 + ch - '0';
! 							precision = precision * 10 + ch - '0';
! 						}
! 						goto nextch;
! 					case '$':
! 						realpos = position;
! 						minlen = 0;
! 						goto nextch;
! 					case '*':
! 						MemSet(&fmtpar[fmtpos], 0, sizeof(fmtpar[fmtpos]));
! 						if (!pointflag)
! 							fmtpar[fmtpos].func = FMTLEN;
! 						else
! 							fmtpar[fmtpos].func = FMTWIDTH;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						goto nextch;
! 					case '.':
! 						pointflag = 1;
! 						goto nextch;
! 					case 'l':
! 						if (longflag)
! 							longlongflag = 1;
! 						else
! 							longflag = 1;
! 						goto nextch;
! 					case 'h':
! 						/* ignore */
! 						goto nextch;
  #ifdef NOT_USED
  
! 						/*
! 						 * We might export this to client apps so we should
! 						 * support 'qd' and 'I64d'(MinGW) also in case the
! 						 * native version does.
! 						 */
! 					case 'q':
  						longlongflag = 1;
  						longflag = 1;
  						goto nextch;
! 					case 'I':
! 						if (*format == '6' && *(format + 1) == '4')
! 						{
! 							format += 2;
! 							longlongflag = 1;
! 							longflag = 1;
! 							goto nextch;
! 						}
! 						break;
  #endif
! 					case 'u':
! 					case 'U':
! 						fmtpar[fmtpos].longflag = longflag;
! 						fmtpar[fmtpos].longlongflag = longlongflag;
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].base = 10;
! 						fmtpar[fmtpos].dosign = 0;
! 						fmtpar[fmtpos].forcesign = forcesign;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].func = FMTNUM_U;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 'o':
! 					case 'O':
! 						fmtpar[fmtpos].longflag = longflag;
! 						fmtpar[fmtpos].longlongflag = longlongflag;
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].base = 8;
! 						fmtpar[fmtpos].dosign = 0;
! 						fmtpar[fmtpos].forcesign = forcesign;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].func = FMTNUM_U;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 'd':
! 					case 'D':
! 						fmtpar[fmtpos].longflag = longflag;
! 						fmtpar[fmtpos].longlongflag = longlongflag;
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].base = 10;
! 						fmtpar[fmtpos].dosign = 1;
! 						fmtpar[fmtpos].forcesign = forcesign;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].func = FMTNUM;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 'x':
! 						fmtpar[fmtpos].longflag = longflag;
! 						fmtpar[fmtpos].longlongflag = longlongflag;
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].base = 16;
! 						fmtpar[fmtpos].dosign = 0;
! 						fmtpar[fmtpos].forcesign = forcesign;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].func = FMTNUM_U;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 'X':
! 						fmtpar[fmtpos].longflag = longflag;
! 						fmtpar[fmtpos].longlongflag = longlongflag;
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].base = -16;
! 						fmtpar[fmtpos].dosign = 1;
! 						fmtpar[fmtpos].forcesign = forcesign;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].func = FMTNUM_U;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 's':
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].maxwidth = maxwidth;
! 						fmtpar[fmtpos].func = FMTSTR;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 'c':
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].func = FMTCHAR;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case 'e':
! 					case 'E':
! 					case 'f':
! 					case 'g':
! 					case 'G':
! 						fmtpar[fmtpos].fmtbegin = fmtbegin;
! 						fmtpar[fmtpos].fmtend = format;
! 						fmtpar[fmtpos].type = ch;
! 						fmtpar[fmtpos].forcesign = forcesign;
! 						fmtpar[fmtpos].leftjust = leftjust;
! 						fmtpar[fmtpos].minlen = minlen;
! 						fmtpar[fmtpos].zpad = zpad;
! 						fmtpar[fmtpos].precision = precision;
! 						fmtpar[fmtpos].pointflag = pointflag;
! 						fmtpar[fmtpos].func = FMTFLOAT;
! 						fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 						fmtpos++;
! 						break;
! 					case '%':
! 						break;
! 					default:
! 						dostr("???????", 0, end, &output);
! 				}
! 				break;
! 			default:
! 				dopr_outch(ch, end, &output);
! 				break;
  		}
  	}
  
--- 253,463 ----
  	output = buffer;
  	while ((ch = *format++))
  	{
! 		if (ch == '%')
  		{
! 			leftjust = minlen = zpad = forcesign = maxwidth = 0;
! 			longflag = longlongflag = pointflag = 0;
! 			fmtbegin = format - 1;
! 			realpos = 0;
! 			position = precision = 0;
! 	nextch:
! 			ch = *format++;
! 			switch (ch)
! 			{
! 				case '\0':
! 					goto performpr;
! 				case '-':
! 					leftjust = 1;
! 					goto nextch;
! 				case '+':
! 					forcesign = 1;
! 					goto nextch;
! 				case '0':	/* set zero padding if minlen not set */
! 					if (minlen == 0 && !pointflag)
! 						zpad = '0';
! 				case '1':
! 				case '2':
! 				case '3':
! 				case '4':
! 				case '5':
! 				case '6':
! 				case '7':
! 				case '8':
! 				case '9':
! 					if (!pointflag)
! 					{
! 						minlen = minlen * 10 + ch - '0';
! 						position = position * 10 + ch - '0';
! 					}
! 					else
! 					{
! 						maxwidth = maxwidth * 10 + ch - '0';
! 						precision = precision * 10 + ch - '0';
! 					}
! 					goto nextch;
! 				case '$':
! 					realpos = position;
! 					minlen = 0;
! 					goto nextch;
! 				case '*':
! 					MemSet(&fmtpar[fmtpos], 0, sizeof(fmtpar[fmtpos]));
! 					if (!pointflag)
! 						fmtpar[fmtpos].func = FMTLEN;
! 					else
! 						fmtpar[fmtpos].func = FMTWIDTH;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					goto nextch;
! 				case '.':
! 					pointflag = 1;
! 					goto nextch;
! 				case 'l':
! 					if (longflag)
! 						longlongflag = 1;
! 					else
! 						longflag = 1;
! 					goto nextch;
! 				case 'h':
! 					/* ignore */
! 					goto nextch;
  #ifdef NOT_USED
  
! 					/*
! 					 * We might export this to client apps so we should
! 					 * support 'qd' and 'I64d'(MinGW) also in case the
! 					 * native version does.
! 					 */
! 				case 'q':
! 					longlongflag = 1;
! 					longflag = 1;
! 					goto nextch;
! 				case 'I':
! 					if (*format == '6' && *(format + 1) == '4')
! 					{
! 						format += 2;
  						longlongflag = 1;
  						longflag = 1;
  						goto nextch;
! 					}
! 					break;
  #endif
! 				case 'u':
! 				case 'U':
! 					fmtpar[fmtpos].longflag = longflag;
! 					fmtpar[fmtpos].longlongflag = longlongflag;
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].base = 10;
! 					fmtpar[fmtpos].dosign = 0;
! 					fmtpar[fmtpos].forcesign = forcesign;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].func = FMTNUM_U;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 'o':
! 				case 'O':
! 					fmtpar[fmtpos].longflag = longflag;
! 					fmtpar[fmtpos].longlongflag = longlongflag;
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].base = 8;
! 					fmtpar[fmtpos].dosign = 0;
! 					fmtpar[fmtpos].forcesign = forcesign;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].func = FMTNUM_U;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 'd':
! 				case 'D':
! 					fmtpar[fmtpos].longflag = longflag;
! 					fmtpar[fmtpos].longlongflag = longlongflag;
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].base = 10;
! 					fmtpar[fmtpos].dosign = 1;
! 					fmtpar[fmtpos].forcesign = forcesign;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].func = FMTNUM;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 'x':
! 					fmtpar[fmtpos].longflag = longflag;
! 					fmtpar[fmtpos].longlongflag = longlongflag;
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].base = 16;
! 					fmtpar[fmtpos].dosign = 0;
! 					fmtpar[fmtpos].forcesign = forcesign;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].func = FMTNUM_U;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 'X':
! 					fmtpar[fmtpos].longflag = longflag;
! 					fmtpar[fmtpos].longlongflag = longlongflag;
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].base = -16;
! 					fmtpar[fmtpos].dosign = 1;
! 					fmtpar[fmtpos].forcesign = forcesign;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].func = FMTNUM_U;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 's':
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].maxwidth = maxwidth;
! 					fmtpar[fmtpos].func = FMTSTR;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 'c':
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].func = FMTCHAR;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case 'e':
! 				case 'E':
! 				case 'f':
! 				case 'g':
! 				case 'G':
! 					fmtpar[fmtpos].fmtbegin = fmtbegin;
! 					fmtpar[fmtpos].fmtend = format;
! 					fmtpar[fmtpos].type = ch;
! 					fmtpar[fmtpos].forcesign = forcesign;
! 					fmtpar[fmtpos].leftjust = leftjust;
! 					fmtpar[fmtpos].minlen = minlen;
! 					fmtpar[fmtpos].zpad = zpad;
! 					fmtpar[fmtpos].precision = precision;
! 					fmtpar[fmtpos].pointflag = pointflag;
! 					fmtpar[fmtpos].func = FMTFLOAT;
! 					fmtpar[fmtpos].realpos = realpos ? realpos : fmtpos;
! 					fmtpos++;
! 					break;
! 				case '%':
! 					break;
! 			}
  		}
  	}
  
***************
*** 538,543 ****
--- 538,545 ----
  	format = format_save;
  	while ((ch = *format++))
  	{
+ 		bool skip_output = false;
+ 		
  		for (i = 1; i < fmtpos; i++)
  		{
  			if (ch == '%' && *format == '%')
***************
*** 552,596 ****
  					case FMTSTR:
  						fmtstr(fmtparptr[i]->value, fmtparptr[i]->leftjust,
  							   fmtparptr[i]->minlen, fmtparptr[i]->maxwidth,
! 							   end, &output);
  						break;
  					case FMTNUM:
  					case FMTNUM_U:
  						fmtint(fmtparptr[i]->numvalue, fmtparptr[i]->base,
  							   fmtparptr[i]->dosign, fmtparptr[i]->forcesign,
  							   fmtparptr[i]->leftjust, fmtparptr[i]->minlen,
! 							   fmtparptr[i]->zpad, end, &output);
  						break;
  					case FMTFLOAT:
  						fmtfloat(fmtparptr[i]->fvalue, fmtparptr[i]->type,
  							 fmtparptr[i]->forcesign, fmtparptr[i]->leftjust,
  								 fmtparptr[i]->minlen, fmtparptr[i]->zpad,
  							fmtparptr[i]->precision, fmtparptr[i]->pointflag,
! 								 end, &output);
  						break;
  					case FMTCHAR:
! 						dopr_outch(fmtparptr[i]->charvalue, end, &output);
  						break;
  				}
  				format = fmtpar[i].fmtend;
! 				goto nochar;
  			}
  		}
! 		dopr_outch(ch, end, &output);
! nochar:
! 		/* nothing */
! 		;						/* semicolon required because a goto has to be
! 								 * attached to a statement */
  	}
! 	*output = '\0';
  
  	free(fmtpar);
  	free(fmtparptr);
  }
  
  static void
  fmtstr(char *value, int leftjust, int minlen, int maxwidth, char *end,
! 	   char **output)
  {
  	int			padlen,
  				vallen;			/* amount to pad */
--- 554,601 ----
  					case FMTSTR:
  						fmtstr(fmtparptr[i]->value, fmtparptr[i]->leftjust,
  							   fmtparptr[i]->minlen, fmtparptr[i]->maxwidth,
! 							   end, (output) ? &output : NULL, stream, &outlen);
  						break;
  					case FMTNUM:
  					case FMTNUM_U:
  						fmtint(fmtparptr[i]->numvalue, fmtparptr[i]->base,
  							   fmtparptr[i]->dosign, fmtparptr[i]->forcesign,
  							   fmtparptr[i]->leftjust, fmtparptr[i]->minlen,
! 							   fmtparptr[i]->zpad, end,
! 							   (output) ? &output : NULL, stream, &outlen);
  						break;
  					case FMTFLOAT:
  						fmtfloat(fmtparptr[i]->fvalue, fmtparptr[i]->type,
  							 fmtparptr[i]->forcesign, fmtparptr[i]->leftjust,
  								 fmtparptr[i]->minlen, fmtparptr[i]->zpad,
  							fmtparptr[i]->precision, fmtparptr[i]->pointflag,
! 								 end, (output) ? &output : NULL, stream, &outlen);
  						break;
  					case FMTCHAR:
! 						dopr_outch(fmtparptr[i]->charvalue, end,
! 								   (output) ? &output : NULL, stream, &outlen);
  						break;
  				}
  				format = fmtpar[i].fmtend;
! 				skip_output = true;
! 				break;
  			}
  		}
! 		if (!skip_output)
! 			dopr_outch(ch, end, (output) ? &output : NULL, stream, &outlen);
  	}
! 	if (output)
! 		*output = '\0';
  
  	free(fmtpar);
  	free(fmtparptr);
+ 
+ 	return outlen;
  }
  
  static void
  fmtstr(char *value, int leftjust, int minlen, int maxwidth, char *end,
! 	   char **output, FILE *stream, int *outlen)
  {
  	int			padlen,
  				vallen;			/* amount to pad */
***************
*** 606,622 ****
  
  	while (padlen > 0)
  	{
! 		dopr_outch(' ', end, output);
  		--padlen;
  	}
! 	dostr(value, maxwidth, end, output);
  
! 	trailing_pad(&padlen, end, output);
  }
  
  static void
  fmtint(int64 value, int base, int dosign, int forcesign, int leftjust,
! 	   int minlen, int zpad, char *end, char **output)
  {
  	int			signvalue = 0;
  	char		convert[64];
--- 611,628 ----
  
  	while (padlen > 0)
  	{
! 		dopr_outch(' ', end, output, stream, outlen);
  		--padlen;
  	}
! 	dostr(value, maxwidth, end, output, stream, outlen);
  
! 	trailing_pad(&padlen, end, output, stream, outlen);
  }
  
  static void
  fmtint(int64 value, int base, int dosign, int forcesign, int leftjust,
! 	   int minlen, int zpad, char *end, char **output, FILE *stream,
! 	   int *outlen)
  {
  	int			signvalue = 0;
  	char		convert[64];
***************
*** 644,661 ****
  
  	adjust_padlen(minlen, vallen, leftjust, &padlen);
  
! 	leading_pad(zpad, &signvalue, &padlen, end, output);
  
  	while (vallen > 0)
! 		dopr_outch(convert[--vallen], end, output);
  
! 	trailing_pad(&padlen, end, output);
  }
  
  static void
  fmtfloat(double value, char type, int forcesign, int leftjust,
  		 int minlen, int zpad, int precision, int pointflag, char *end,
! 		 char **output)
  {
  	int			signvalue = 0;
  	int			vallen;
--- 650,667 ----
  
  	adjust_padlen(minlen, vallen, leftjust, &padlen);
  
! 	leading_pad(zpad, &signvalue, &padlen, end, output, stream, outlen);
  
  	while (vallen > 0)
! 		dopr_outch(convert[--vallen], end, output, stream, outlen);
  
! 	trailing_pad(&padlen, end, output, stream, outlen);
  }
  
  static void
  fmtfloat(double value, char type, int forcesign, int leftjust,
  		 int minlen, int zpad, int precision, int pointflag, char *end,
! 		 char **output, FILE *stream, int *outlen)
  {
  	int			signvalue = 0;
  	int			vallen;
***************
*** 676,712 ****
  
  	adjust_padlen(minlen, vallen, leftjust, &padlen);
  
! 	leading_pad(zpad, &signvalue, &padlen, end, output);
  
! 	dostr(convert, 0, end, output);
  
! 	trailing_pad(&padlen, end, output);
  }
  
  static void
! dostr(char *str, int cut, char *end, char **output)
  {
  	if (cut)
  		while (*str && cut-- > 0)
! 			dopr_outch(*str++, end, output);
  	else
  		while (*str)
! 			dopr_outch(*str++, end, output);
  }
  
  static void
! dopr_outch(int c, char *end, char **output)
  {
  #ifdef NOT_USED
  	if (iscntrl((unsigned char) c) && c != '\n' && c != '\t')
  	{
  		c = '@' + (c & 0x1F);
! 		if (end == 0 || *output < end)
! 			*(*output)++ = '^';
  	}
  #endif
! 	if (end == 0 || *output < end)
! 		*(*output)++ = c;
  }
  
  
--- 682,740 ----
  
  	adjust_padlen(minlen, vallen, leftjust, &padlen);
  
! 	leading_pad(zpad, &signvalue, &padlen, end, output, stream, outlen);
  
! 	dostr(convert, 0, end, output, stream, outlen);
  
! 	trailing_pad(&padlen, end, output, stream, outlen);
  }
  
  static void
! dostr(char *str, int cut, char *end, char **output, FILE *stream, int *outlen)
  {
  	if (cut)
  		while (*str && cut-- > 0)
! 			dopr_outch(*str++, end, output, stream, outlen);
  	else
  		while (*str)
! 			dopr_outch(*str++, end, output, stream, outlen);
  }
  
  static void
! dopr_outch(int c, char *end, char **output, FILE *stream, int *outlen)
  {
  #ifdef NOT_USED
  	if (iscntrl((unsigned char) c) && c != '\n' && c != '\t')
  	{
  		c = '@' + (c & 0x1F);
! 		if (output)
! 		{
! 			if (end == 0 || *output < end)
! 			{
! 				*(*output)++ = '^';
! 				(*outlen)++;
! 			}
! 		}
! 		else
! 		{
! 			putc(c, stream);
! 			(*outlen)++;
! 		}
  	}
  #endif
! 	if (output)
! 	{
! 		if (end == 0 || *output < end)
! 		{
! 			*(*output)++ = c;
! 			(*outlen)++;
! 		}
! 	}
! 	else
! 	{
! 		putc(c, stream);
! 		(*outlen)++;
! 	}
  }
  
  
***************
*** 736,765 ****
  
  
  static void
! leading_pad(int zpad, int *signvalue, int *padlen, char *end, char **output)
  {
  	if (*padlen > 0 && zpad)
  	{
  		if (*signvalue)
  		{
! 			dopr_outch(*signvalue, end, output);
  			--*padlen;
  			*signvalue = 0;
  		}
  		while (*padlen > 0)
  		{
! 			dopr_outch(zpad, end, output);
  			--*padlen;
  		}
  	}
  	while (*padlen > 0 + (*signvalue != 0))
  	{
! 		dopr_outch(' ', end, output);
  		--*padlen;
  	}
  	if (*signvalue)
  	{
! 		dopr_outch(*signvalue, end, output);
  		if (*padlen > 0)
  			--* padlen;
  		if (padlen < 0)
--- 764,794 ----
  
  
  static void
! leading_pad(int zpad, int *signvalue, int *padlen, char *end, char **output,
! 			FILE *stream, int *outlen)
  {
  	if (*padlen > 0 && zpad)
  	{
  		if (*signvalue)
  		{
! 			dopr_outch(*signvalue, end, output, stream, outlen);
  			--*padlen;
  			*signvalue = 0;
  		}
  		while (*padlen > 0)
  		{
! 			dopr_outch(zpad, end, output, stream, outlen);
  			--*padlen;
  		}
  	}
  	while (*padlen > 0 + (*signvalue != 0))
  	{
! 		dopr_outch(' ', end, output, stream, outlen);
  		--*padlen;
  	}
  	if (*signvalue)
  	{
! 		dopr_outch(*signvalue, end, output, stream, outlen);
  		if (*padlen > 0)
  			--* padlen;
  		if (padlen < 0)
***************
*** 769,779 ****
  
  
  static void
! trailing_pad(int *padlen, char *end, char **output)
  {
  	while (*padlen < 0)
  	{
! 		dopr_outch(' ', end, output);
  		++*padlen;
  	}
  }
--- 798,808 ----
  
  
  static void
! trailing_pad(int *padlen, char *end, char **output, FILE *stream, int *outlen)
  {
  	while (*padlen < 0)
  	{
! 		dopr_outch(' ', end, output, stream, outlen);
  		++*padlen;
  	}
  }
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: [HACKERS] snprintf() argument reordering not working under Windows

Bruce Momjian <pgman@candle.pha.pa.us> writes:

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.

What's the point? If the code is in there we may as well use it.

regards, tom lane

#4Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#3)
Re: [HACKERS] snprintf() argument reordering not working under

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

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.

What's the point? If the code is in there we may as well use it.

I was thinking it might avoid us hitting other bugs in there, but that
is pessimistic, of course. I will hold off on that idea.

-- 
  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
#5Nicolai Tufar
ntufar@gmail.com
In reply to: Bruce Momjian (#2)
Re: [HACKERS] snprintf() argument reordering not working under Windows in 8.1

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.

2005/12/3, Bruce Momjian <pgman@candle.pha.pa.us>:

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, 10000) + 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.

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. :)

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

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

Thank you for your attention,
Regards,
Nicolai Tufar

#6Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Nicolai Tufar (#5)
Re: [PATCHES] snprintf() argument reordering not working under

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 <pgman@candle.pha.pa.us>:

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, 10000) + 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
*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.

-- 
  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
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: [PATCHES] snprintf() argument reordering not working under

Bruce Momjian <pgman@candle.pha.pa.us> 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

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#6)
Re: [PATCHES] snprintf() argument reordering not working

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

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#8)
Re: [PATCHES] snprintf() argument reordering not working

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

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#9)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#11Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#10)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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
#12Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#11)
1 attachment(s)
Re: [PATCHES] snprintf() argument reordering not working

Bruce Momjian wrote:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

Attachments:

nls.difftext/x-patch; name=nls.diffDownload
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 -0000	1.461
--- configure	4 Dec 2005 22:56:58 -0000
***************
*** 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 -0000	1.190
--- src/include/c.h	4 Dec 2005 22:57:02 -0000
***************
*** 96,101 ****
--- 96,122 ----
  
  #ifdef ENABLE_NLS
  #include <libintl.h>
+ #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 -0000	1.7
--- src/interfaces/ecpg/pgtypeslib/extern.h	4 Dec 2005 22:57:05 -0000
***************
*** 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 -0000	1.5
--- src/interfaces/libpq/exports.txt	4 Dec 2005 22:57:06 -0000
***************
*** 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,v
retrieving revision 1.27
diff -c -r1.27 win32.h
*** src/interfaces/libpq/win32.h	31 Jul 2004 06:19:23 -0000	1.27
--- src/interfaces/libpq/win32.h	4 Dec 2005 22:57:06 -0000
***************
*** 16,21 ****
--- 16,27 ----
  #define write(a,b,c) _write(a,b,c)
  #endif
  
+ #ifdef vsnprintf
+ #undef vsnprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
  #define vsnprintf(a,b,c,d) _vsnprintf(a,b,c,d)
  #define snprintf _snprintf
  
#13Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Andrew Dunstan (#12)
Re: [PATCHES] snprintf() argument reordering not working

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 <andrew@dunslane.net> 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
#14Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#13)
Re: [PATCHES] snprintf() argument reordering not working

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

#15Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#14)
Re: [PATCHES] snprintf() argument reordering not working

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 <libintl.h>
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

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#15)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#17Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#16)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#17)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#19Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#18)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#19)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#21Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#20)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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
#22Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#21)
Re: [PATCHES] snprintf() argument reordering not working

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

#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#22)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#22)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#25Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#23)
1 attachment(s)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

Attachments:

nls.difftext/x-patch; name=nls.diffDownload
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 -0000	1.461
--- configure	5 Dec 2005 18:56:04 -0000
***************
*** 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 -0000	1.190
--- src/include/c.h	5 Dec 2005 18:56:23 -0000
***************
*** 96,101 ****
--- 96,122 ----
  
  #ifdef ENABLE_NLS
  #include <libintl.h>
+ #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 -0000	1.7
--- src/interfaces/ecpg/pgtypeslib/extern.h	5 Dec 2005 18:56:24 -0000
***************
*** 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 -0000	1.138
--- src/interfaces/libpq/Makefile	5 Dec 2005 18:56:24 -0000
***************
*** 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' -e 's/^\(.* \)\([0-9][0-9]*\)/    \1@ \2/' < $< >> $@
  
  $(srcdir)/blibpqdll.def: exports.txt
  	echo '; DEF file for Borland C++ Builder' > $@
  	echo 'LIBRARY BLIBPQ' >> $@
  	echo 'DESCRIPTION "PostgreSQL Client Library"' >> $@
  	echo 'EXPORTS' >> $@
! 	sed -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/    _\1@ \2/' < $< >> $@
  	echo '' >> $@
  	echo '; Aliases for MS compatible names' >> $@
! 	sed -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/    \1= _\1/' < $< | sed 's/ *$$//' >> $@
  
  # depend on Makefile.global to force rebuild on re-run of configure
  $(srcdir)/libpq.rc: libpq.rc.in $(top_builddir)/src/Makefile.global
--- 107,130 ----
  	echo 'LIBRARY LIBPQ' >> $@
  	echo 'DESCRIPTION "PostgreSQL Client Library"' >> $@
  	echo 'EXPORTS' >> $@
! 	sed $(NONLS) -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 $(NONLS) -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/    \1@ \2/' < $< >> $@
  
  $(srcdir)/blibpqdll.def: exports.txt
  	echo '; DEF file for Borland C++ Builder' > $@
  	echo 'LIBRARY BLIBPQ' >> $@
  	echo 'DESCRIPTION "PostgreSQL Client Library"' >> $@
  	echo 'EXPORTS' >> $@
! 	sed $(NONLS) -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/    _\1@ \2/' < $< >> $@
  	echo '' >> $@
  	echo '; Aliases for MS compatible names' >> $@
! 	sed $(NONLS) -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/    \1= _\1/' < $< | sed 's/ *$$//' >> $@
  
  # depend on Makefile.global to force rebuild on re-run of configure
  $(srcdir)/libpq.rc: libpq.rc.in $(top_builddir)/src/Makefile.global
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 -0000	1.5
--- src/interfaces/libpq/exports.txt	5 Dec 2005 18:56:24 -0000
***************
*** 125,127 ****
--- 125,132 ----
  lo_create                 123
  PQinitSSL                 124
  PQregisterThreadLock      125
+ pg_sprintf                126
+ pg_snprintf               127
+ pg_fprintf                128
+ pg_printf                 129
+ pg_vsnprintf              130
Index: src/interfaces/libpq/win32.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/win32.h,v
retrieving revision 1.27
diff -c -r1.27 win32.h
*** src/interfaces/libpq/win32.h	31 Jul 2004 06:19:23 -0000	1.27
--- src/interfaces/libpq/win32.h	5 Dec 2005 18:56:24 -0000
***************
*** 16,21 ****
--- 16,27 ----
  #define write(a,b,c) _write(a,b,c)
  #endif
  
+ #ifdef vsnprintf
+ #undef vsnprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
  #define vsnprintf(a,b,c,d) _vsnprintf(a,b,c,d)
  #define snprintf _snprintf
  
#26Dave Page
dpage@vale-housing.co.uk
In reply to: Andrew Dunstan (#25)
Re: [PATCHES] snprintf() argument reordering not working

-----Original Message-----
From: "Andrew Dunstan"<andrew@dunslane.net>
Sent: 05/12/05 19:03:17
To: "Tom Lane"<tgl@sss.pgh.pa.us>
Cc: "Bruce Momjian"<pgman@candle.pha.pa.us>, "ntufar@gmail.com"<ntufar@gmail.com>, "devrim@kivi.com.tr"<devrim@kivi.com.tr>, "mha@sollentuna.net"<mha@sollentuna.net>, "pgsql-hackers@postgresql.org"<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 <andrew@dunslane.net> 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

#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#25)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#21)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

#29Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#28)
1 attachment(s)
Re: [PATCHES] snprintf() argument reordering not working

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

Attachments:

nls.difftext/x-patch; name=nls.diffDownload
? 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 -0000	1.461
--- configure	5 Dec 2005 22:22:11 -0000
***************
*** 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 <stdio.h> 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 -0000	1.431
--- configure.in	5 Dec 2005 22:22:12 -0000
***************
*** 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 <stdio.h> 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 <stdio.h> 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 -0000	1.190
--- src/include/c.h	5 Dec 2005 22:22:16 -0000
***************
*** 96,101 ****
--- 96,122 ----
  
  #ifdef ENABLE_NLS
  #include <libintl.h>
+ #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/ecpglib/Makefile
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/ecpg/ecpglib/Makefile,v
retrieving revision 1.33
diff -c -r1.33 Makefile
*** src/interfaces/ecpg/ecpglib/Makefile	14 Mar 2005 17:27:50 -0000	1.33
--- src/interfaces/ecpg/ecpglib/Makefile	5 Dec 2005 22:22:17 -0000
***************
*** 33,38 ****
--- 33,40 ----
  ifeq ($(PORTNAME), win32)
  # Link to shfolder.dll instead of shell32.dll
  SHLIB_LINK += -lshfolder
+ # and use our snprintf
+ OBJS += snprintf.o
  endif
  
  all: all-lib
***************
*** 51,56 ****
--- 53,61 ----
  exec.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
+ snprintf.c: % : $(top_srcdir)/src/port/%
+ 	rm -f $@ && $(LN_S) $< .
+ 
  path.o: path.c $(top_builddir)/src/port/pg_config_paths.h
  
  $(top_builddir)/src/port/pg_config_paths.h:
Index: src/interfaces/ecpg/pgtypeslib/extern.h
===================================================================
RCS file: /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 -0000	1.7
--- src/interfaces/ecpg/pgtypeslib/extern.h	5 Dec 2005 22:22:17 -0000
***************
*** 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/win32.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/win32.h,v
retrieving revision 1.27
diff -c -r1.27 win32.h
*** src/interfaces/libpq/win32.h	31 Jul 2004 06:19:23 -0000	1.27
--- src/interfaces/libpq/win32.h	5 Dec 2005 22:22:18 -0000
***************
*** 16,21 ****
--- 16,27 ----
  #define write(a,b,c) _write(a,b,c)
  #endif
  
+ #ifdef vsnprintf
+ #undef vsnprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
  #define vsnprintf(a,b,c,d) _vsnprintf(a,b,c,d)
  #define snprintf _snprintf
  
#30Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#29)
1 attachment(s)
Re: [PATCHES] snprintf() argument reordering not working

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

Attachments:

nls.difftext/x-patch; name=nls.diffDownload
? 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 -0000	1.461
--- configure	5 Dec 2005 22:39:43 -0000
***************
*** 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 <stdio.h> 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 -0000	1.431
--- configure.in	5 Dec 2005 22:39:44 -0000
***************
*** 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 <stdio.h> 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 <stdio.h> 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
===================================================================
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 -0000	1.190
--- src/include/c.h	5 Dec 2005 22:39:47 -0000
***************
*** 96,101 ****
--- 96,122 ----
  
  #ifdef ENABLE_NLS
  #include <libintl.h>
+ #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/ecpglib/Makefile
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/ecpg/ecpglib/Makefile,v
retrieving revision 1.33
diff -c -r1.33 Makefile
*** src/interfaces/ecpg/ecpglib/Makefile	14 Mar 2005 17:27:50 -0000	1.33
--- src/interfaces/ecpg/ecpglib/Makefile	5 Dec 2005 22:39:48 -0000
***************
*** 33,38 ****
--- 33,40 ----
  ifeq ($(PORTNAME), win32)
  # Link to shfolder.dll instead of shell32.dll
  SHLIB_LINK += -lshfolder
+ # and use our snprintf
+ OBJS += snprintf.o
  endif
  
  all: all-lib
***************
*** 51,56 ****
--- 53,61 ----
  exec.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
+ snprintf.c: % : $(top_srcdir)/src/port/%
+ 	rm -f $@ && $(LN_S) $< .
+ 
  path.o: path.c $(top_builddir)/src/port/pg_config_paths.h
  
  $(top_builddir)/src/port/pg_config_paths.h:
Index: src/interfaces/ecpg/pgtypeslib/extern.h
===================================================================
RCS file: /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 -0000	1.7
--- src/interfaces/ecpg/pgtypeslib/extern.h	5 Dec 2005 22:39:49 -0000
***************
*** 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/win32.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/win32.h,v
retrieving revision 1.27
diff -c -r1.27 win32.h
*** src/interfaces/libpq/win32.h	31 Jul 2004 06:19:23 -0000	1.27
--- src/interfaces/libpq/win32.h	5 Dec 2005 22:39:49 -0000
***************
*** 16,21 ****
--- 16,27 ----
  #define write(a,b,c) _write(a,b,c)
  #endif
  
+ #ifdef vsnprintf
+ #undef vsnprintf
+ #endif
+ #ifdef snprintf
+ #undef snprintf
+ #endif
  #define vsnprintf(a,b,c,d) _vsnprintf(a,b,c,d)
  #define snprintf _snprintf
  
#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#29)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#32Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#30)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#33Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#32)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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&amp;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

#34Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Andrew Dunstan (#17)
Re: [PATCHES] snprintf() argument reordering not working

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 <libintl.h>.

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 <andrew@dunslane.net> 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
#35Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#21)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#36Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#35)
Re: [PATCHES] snprintf() argument reordering not working

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

#37Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#33)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#38Nicolai Tufar
ntufar@gmail.com
In reply to: Andrew Dunstan (#8)
Re: [PATCHES] snprintf() argument reordering not working under Windows in 8.1

2005/12/4, Andrew Dunstan <andrew@dunslane.net>:

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.

Show quoted text

cheers

andrew

#39Nicolai Tufar
ntufar@gmail.com
In reply to: Nicolai Tufar (#38)
Re: [PATCHES] snprintf() argument reordering not working under Windows in 8.1

2005/12/6, Nicolai Tufar <ntufar@gmail.com>:

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

#40Dave Page
dpage@vale-housing.co.uk
In reply to: Tom Lane (#37)
Re: [PATCHES] snprintf() argument reordering not working

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Bruce Momjian
Sent: 06 December 2005 04:40
To: Andrew Dunstan
Cc: Tom Lane; ntufar@gmail.com; devrim@kivi.com.tr;
mha@sollentuna.net; 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.

#41Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#37)
Re: [PATCHES] snprintf() argument reordering not working

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&amp;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

#42Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#41)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan <andrew@dunslane.net> 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

#43Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#42)
Re: [PATCHES] snprintf() argument reordering not working

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

#44Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#43)
Re: [PATCHES] snprintf() argument reordering not working

Andrew Dunstan wrote:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> 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

#45Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#44)
Re: [PATCHES] snprintf() argument reordering not working

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