Re: Add %z support to elog/ereport?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add %z support to elog/ereport?
Date: 2014-01-17 18:50:08
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> [ 0001-Add-support-for-printing-Size-arguments-to-elog-erep.patch ]

I think this approach is fundamentally broken, because it can't reasonably
cope with any case more complicated than "%zu" or "%zd". While it's
arguable that we can get along without the ability to specify field width,
since the [U]INT64_FORMAT macros never allowed that, it is surely going
to be the case that translators will need to insert "n$" flags in the
translated versions of messages.

Another objection is that this only fixes the problem in elog/ereport
format strings, not anyplace else that it might be useful to print a
size_t value. You suggest below that we could invent some additional
macros to support that; but since the "z" flag is in C99, there really
ought to be only a small minority of platforms where it doesn't work.
So I don't think we should be contorting our code with some nonstandard
notation for the case, if there's any way to avoid that.

I think a better solution approach is to teach our src/port/snprintf.c
about the z flag, then extend configure's checking to force use of our
snprintf if the platform's version doesn't handle z. While it might be
objected that this creates a need for a run-time check in configure,
we already have one to check if snprintf handles "n$", so this approach
doesn't really make life any worse for cross-compiles.

> In patch 01, I've modified configure to not define [U]INT64_FORMAT
> directly, but rather just define INT64_LENGTH_MODIFIER as
> appropriate. The formats are now defined in c.h.
> INT64_LENGTH_MODIFIER is defined without quotes - I am not sure whether
> that was the right choice, it requires using CppAsString2() in some
> places. Maybe I should just have defined it with quotes instead. Happy
> to rejigger it if somebody thinks the other way would be better.

Meh. This isn't needed if we do what I suggest above, but in any case
I don't approve of removing the existing [U]INT64_FORMAT macros.
That breaks code that doesn't need to get broken, probably including
third-party modules. It'd be more sensible to just add an additional
macro for the flag character(s). (And yeah, I'd put quotes in it.)

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2014-01-17 18:58:29 Re: currawong is not a happy animal
Previous Message Alexander Korotkov 2014-01-17 18:49:37 Re: GIN improvements part 1: additional information