Peter Eisentraut <peter.eisentr...@enterprisedb.com> writes: Hi,
> 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. When Dean pointed me this thread[1], I was thinking we need to add the "-Wformat-signedness" and fix all the existing warnning. Then after some research, it is not such easy and seems we need some agreement first if we want to fix them. > 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. IIUC, we have agreed that we should cast enum to int and continue to use "%d". At least Tom suggested this and Thomas agreed this [1] and Peter didn't raise any opposition. > 2. Various trickery at the boundary of internal counters that are > unsigned and external functions or views using signed types. These need > another look. I also noticed we lack of UNSIGNED INT32/64 SQL type. Changing the counter to signed looks not good to me as well. This one looks doesn't have an agreement yet. > 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. 7. __FILE__ in gcc is 'int', but we elog() it with "%u". Should we change it to "%d"? > 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. ;-) Me too, just that want some clean code:) But FWIW, "-Wformat-signedness" is not supported by clang so far, so if people is using clang, they still can't benefit from this changes. My soluation (I use clang everyday) is adding a "gcc-checker" for my c file, if I make such mistake, it can remind me directly. [0] https://www.postgresql.org/message-id/874j4yl4cj.fsf%40163.com [1] https://www.postgresql.org/message-id/CA%2BhUKGJNUk434tcsVbs5YUGsujZbveo43QcZeWbv0xPzg9us-A%40mail.gmail.com -- Best Regards Andy Fan
