Re: -Wformat-signedness

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: -Wformat-signedness
Date: 2020-11-09 15:02:01
Message-ID: a9006a98-46aa-01b7-bf7c-15b6493c05f6@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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. ;-)

Attachment Content-Type Size
v1-0001-Add-Wformat-signedness.patch text/plain 2.4 KB
v1-0002-Fix-Wformat-signedness-warnings-for-enums.patch text/plain 49.6 KB
v1-0003-WIP-Fix-remaining-Wformat-signedness-warnings.patch text/plain 135.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-11-09 15:11:43 Re: Prevent printing "next step instructions" in initdb and pg_upgrade
Previous Message Jehan-Guillaume de Rorthais 2020-11-09 14:50:49 Re: vacuum -vs reltuples on insert only index