Thread: -Wformat-signedness

Hi hackers,

There're probably mostly harmless, being mostly error and debug
messages and the like, and considering that eg OID parsing tolerates
negative numbers when reading them back in, but for what it's worth:
GCC complains about many %d vs %u type mixups if you build with
$SUBJECT.

On 2020-10-29 22:37, Thomas Munro wrote:
> There're probably mostly harmless, being mostly error and debug
> messages and the like, and considering that eg OID parsing tolerates
> negative numbers when reading them back in, but for what it's worth:
> GCC complains about many %d vs %u type mixups if you build with
> $SUBJECT.

I had looked into this some time ago.  I have dusted off my patch again. 
The attached version fixes all warnings for me.

The following are the main categories of issues:

1. enums are unsigned by default in gcc, so all those internal error 
messages "unrecognized blah kind: %d" need to be changed to %u.

I have split that into its own patch since it's easily separable.  All 
the remaining issues are in one patch.

2. Various trickery at the boundary of internal counters that are 
unsigned and external functions or views using signed types.  These need 
another look.

3. Various messages print signed values using %x formats, which need to 
be unsigned.  These might also need another look.

4. Issues with constants being signed by default.  For example, things 
like elog(ERROR, "foo is %u but should be %u", somevar, 55) warns 
because of the constant.  Should be changed to something like 55U for 
symmetry, or change the %u to %d.  This also reaches into genbki 
territory with all the OID constants being generated.

5. Some "surprising" but correct C behavior.  For example, unsigned 
short is promoted to int (not unsigned int) in variable arguments, so 
needs a %d format.

6. Finally, a bunch of uses were just plain wrong and should be corrected.

I haven't found anything that is a really serious bug, but I imagine you 
could run into trouble in various ways when you exceed the INT_MAX 
value.  But then again, if you use up INT_MAX WAL timelines, you 
probably have other problems. ;-)

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> 1. enums are unsigned by default in gcc, so all those internal error 
> messages "unrecognized blah kind: %d" need to be changed to %u.

Do we have reason to think that that is true in every C compiler?
My own preference for this would be to leave the messages as-is
and add explicit "(int)" casts to the arguments.  There are some
fraction of these that are like that already.

            regards, tom lane

On Tue, Nov 10, 2020 at 4:25 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> > 1. enums are unsigned by default in gcc, so all those internal error
> > messages "unrecognized blah kind: %d" need to be changed to %u.
>
> Do we have reason to think that that is true in every C compiler?
> My own preference for this would be to leave the messages as-is
> and add explicit "(int)" casts to the arguments.  There are some
> fraction of these that are like that already.

From experimentation, it seems that GCC enumerator constants are int,
but enum variables are int or signed int depending on whether any
negative values were defined.  Valid values have to be representable
as int anyway regardless of what size and signedness a compiler
chooses to use, so yeah, +1 for casting to int.