spurious use of %m format in pg_upgrade

Peter Eisentraut <peter_e@gmx.net> writes:

pg_upgrade's pg_scandir_internal() makes use of the non-standard %m
format:

pg_log(PG_FATAL, "could not open directory \"%s\": %m\n", dirname);

Is this an oversight, or is there an undocumented assumption that this
code will only be used on platforms where %m works?

Surely an oversight; everywhere else in frontend code, we take care to
use strerror instead. Is there a way to persuade gcc to complain about
such extensions when used in contexts where we don't know they work?

(Which platforms don't have scandir() anyway?)

Hmmm ... my neolithic HPUX box has it, but OTOH the Open Group specs
seem to have added it only in Issue 7 (2008), so I'd not want to bet
money that any random Unix has got it.

regards, tom lane

On tor, 2011-07-07 at 00:22 -0400, Tom Lane wrote:

Is there a way to persuade gcc to complain about such extensions when
used in contexts where we don't know they work?

I don't think so.

First of all, the comment in pg_config_manual.h says that we *want* the
compiler to recognize %m as valid, and we apply the same attribute
globally. And secondly, the difference between the gnu_printf attribute
and the plain printf attribute is that the latter checks for what the
target's C library accepts, which is equivalent to gnu_printf if you're
on glibc, at least. According to the gcc source code, the target is
supposed to override the conversion specifier list accordingly, but it
looks like, for example, FreeBSD doesn't do that, which could be
considered a bug there. The only override in the gcc source code itself
is MinGW, which is where the whole trouble in pg_config_manual.h stems
from in the first place. (Look for TARGET_OVERRIDES_FORMAT_ATTRIBUTES
in the gcc source if you want to investigate.)

There does not, unfortunately, appear to be an attribute that says to
check conversion specifiers according to some standard (even though the
internal structures of gcc appear to support that, but they'd currently
have %m in the wrong category there anyway).

Peter Eisentraut wrote:

pg_upgrade's pg_scandir_internal() makes use of the non-standard %m
format:

pg_log(PG_FATAL, "could not open directory \"%s\": %m\n", dirname);

Is this an oversight, or is there an undocumented assumption that this
code will only be used on platforms where %m works?

(Which platforms don't have scandir() anyway?)

Yes, surely an oversight, and I see it has been removed --- good.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +