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 |
Message-ID: | 8607.1389984608@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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
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 |