From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: printf format selection vs. reality |
Date: | 2018-05-23 20:28:55 |
Message-ID: | 8383.1527107335@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(dot)1526862605(at)sss(dot)pgh(dot)pa(dot)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(dot)1526862605(at)sss(dot)pgh(dot)pa(dot)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 ...
From | Date | Subject | |
---|---|---|---|
Next Message | Christoph Berg | 2018-05-23 20:45:26 | Re: PG11 jit failing on ppc64el |
Previous Message | Alvaro Herrera | 2018-05-23 20:10:46 | Re: printf format selection vs. reality |