From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: printf format selection vs. reality
Date: Wed, 23 May 2018 16:28:55 -0400
Message-ID: <8383.1527107335@sss.pgh.pa.us> (raw)
In-Reply-To: <20180523201046.4znqfw52bmr6o3gr@alvherre.pgsql>
References: <20180523201046.4znqfw52bmr6o3gr@alvherre.pgsql>
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-May-23, Tom Lane wrote:
>> The practical alternatives seem to be to avoid %z in frontend code,
>> or to invent a macro SIZE_T_MODIFIER and use it like INT64_MODIFIER.
>> Either one will be extremely error-prone, I'm afraid, unless we can
>> find a way to get compiler warnings for violations.
> Usage of %z outside safe-known seems really limited. It would be sad to
> force SIZE_T_MODIFIER for elog calls (where it is prevalent) just for
> the benefit of usage outside of elog on fringe platforms -- you're right
> that we do have a few cases of %z under fprintf() already. The good
> news is that AFAICS those strings are not translatable today, so
> changing only those to SIZE_T_MODIFIER (and leaving alone those using
> elog) is maybe not such a big deal. I think those are dshash.c, dsa.c,
> slab.c and aset.c only.
Yeah, I just went through things myself, and concluded that right now
the only hazards are in debug code such as dsa_dump(). So I think that
(a) we don't have a problem we have to fix right now, and (b) going
over to SIZE_T_MODIFIER seems like more trouble than it'd be worth.
Still, this seems like something that will certainly bite us eventually
if we don't install some kind of check.
> (I assume without checking that with the stringinfo API it's OK to use %z).
It is, that goes to snprintf.
> Can't we raise warnings on such usages with an archetype change? (Hm,
> is it possible to change archetype for fprintf?)
The problem is to get a compiler that thinks that %z is a violation
of *any* archetype. gaur's compiler does think that, but it has no
archetype that does accept %z, so that's little help (I've had it
building with -Wno-format since we added %z).
It might be possible for me to install a fractionally-newer compiler
on gaur's host and get usable warnings that way. I think however
that a more practical approach is likely to be to depend on the
Windows/gcc buildfarm members, where (if gcc is correctly installed
and doing what it's supposed to) we should find that %z is accepted
by the gnu_printf archetype but not the plain printf one. So I wish
somebody would try out the patch in <2975.1526862605@sss.pgh.pa.us>
on MinGW. It would also be good to find out whether MSVC can be
persuaded to check printf strings.
regards, tom lane
PS: per the above, the patch in <2975.1526862605@sss.pgh.pa.us>
would need to be adjusted to use gnu_printf on the stringinfo
functions, if we don't want complaints about %z. This opens
the question of whether we want to allow %m there ...
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: printf format selection vs. reality
In-Reply-To: <8383.1527107335@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox