SEO

What is the official stance on handling compiler warnings?
I got a bit curious today on how many warnings our builds are generating
on the buildfarm.
I have hacked up a small script that (in a very primitive way) parses
the "make" stage logfiles of all unix boxes reporting on HEAD and prints
the number of lines that appear to be either diagnostics or warnings
emited during a build with the following results:

animal: mongoose warnings: 2379
animal: warthog warnings: 1368
animal: kudu warnings: 748
animal: turnip_moth warnings: 736
animal: gypsy_moth warnings: 736
animal: winter_moth warnings: 700
animal: ghost_moth warnings: 700
animal: dot_moth warnings: 700
animal: codlin_moth warnings: 700
animal: dugong warnings: 371
animal: emu warnings: 265
animal: orangutan warnings: 198
animal: spoonbill warnings: 126
animal: zebra warnings: 124
animal: tucuxi warnings: 124
animal: lionfish warnings: 87
animal: wildebeest warnings: 77
animal: rook warnings: 76
animal: rosella warnings: 74
animal: dragonfly warnings: 72
animal: wombat warnings: 71
animal: Shad warnings: 71
animal: quagga warnings: 71
animal: caracara warnings: 71
animal: bobcat warnings: 71
animal: barasingha warnings: 71
animal: grebe warnings: 50
animal: eel warnings: 43
animal: thrush warnings: 32
animal: salamander warnings: 27
animal: clownfish warnings: 26
animal: osprey warnings: 24
animal: luna_moth warnings: 15
animal: emperor_moth warnings: 15
animal: canary warnings: 14
animal: impala warnings: 9
animal: jackal warnings: 7
animal: bear warnings: 6
animal: viper warnings: 5
animal: cuckoo warnings: 5
animal: cardinal warnings: 5
animal: boodie warnings: 5
animal: aubrac warnings: 5
animal: sponge warnings: 3
animal: dungbeetle warnings: 3
animal: tapir warnings: 0
animal: platypus warnings: 0
animal: ermine warnings: 0

a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
icc boxes or the "statement not reached" spam from the sun compilers)
but others might indicate real issues.
To find warnings that might be a real problem we might want to look into
suppressing those - if possible - using compiler switches.

comments ?


Stefan

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> What is the official stance on handling compiler warnings?

The compilers I use give me 1 or 2 warnings on HEAD, coming from flex's
sloppiness about not generating unused code. I wouldn't care to work
with a compiler that generated more than a few. At the same time,
I'm not prepared to contort the source code unduly to suppress what
you referred to as "spam" warnings from over-chatty compilers.

What would probably be useful if you want to pursue this is to filter
out the obvious spam like statement-not-reached, and see what's left.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:
> a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
> icc boxes or the "statement not reached" spam from the sun compilers)
> but others might indicate real issues.
> To find warnings that might be a real problem we might want to look into
> *suppressing those - if possible - *using compiler switches.

It would be good to determine an appropriate set of compiler switches to
reduce the warnings to a reasonable level.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>> What is the official stance on handling compiler warnings?
>
> The compilers I use give me 1 or 2 warnings on HEAD, coming from flex's
> sloppiness about not generating unused code. I wouldn't care to work
> with a compiler that generated more than a few. At the same time,
> I'm not prepared to contort the source code unduly to suppress what
> you referred to as "spam" warnings from over-chatty compilers.
>
> What would probably be useful if you want to pursue this is to filter
> out the obvious spam like statement-not-reached, and see what's left.

ok I did that for a few members (removing all the statement not reached
ones as well as some purely informal notices and all the flex related
warnings) and came up with something similiar to:


animal: eel warnings: 4
dirmod.c:206: warning: no previous prototype for 'pgsymlink'
dirmod.c:206: warning: no previous prototype for 'pgsymlink'
pg_ctl.c: In function `pgwin32_doRunAsService':
pg_ctl.c:1240: warning: initialization discards qualifiers from pointer
target type

animal: clownfish warnings: 12
"dynloader.c", line 4: warning: empty translation unit
"postgres.c", line 3758: warning: loop not entered at top
"xml.c", line 2810: warning: integer overflow detected: op "<<"
"xml.c", line 2810: warning: integer overflow detected: op "UNARY -"
"xml.c", line 2810: warning: integer overflow detected: op "<<"
"dfmgr.c", line 117: warning: assignment type mismatch:
pointer to function(pointer to struct FunctionCallInfoData
{pointer to struct FmgrInfo {..} flinfo, pointer to struct Node {..}
context, pointer to struc
t Node {..} resultinfo, char isnull, short nargs, array[100] of unsigned
long arg, array[100] of char argnull}) returning unsigned long "="
pointer to void
"dfmgr.c", line 165: warning: return value type mismatch
"wchar.c", line 283: warning: integer overflow detected: op "<<"
"wchar.c", line 283: warning: integer overflow detected: op "<<"
"pg_resetxlog.c", line 76: warning: initializer does not fit or is out
of range: -1
"pg_resetxlog.c", line 80: warning: initializer does not fit or is out
of range: -1


animal: jackal warnings: 2
postmaster.c: In function 'PostmasterMain':
postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
(declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)


animal: impala warnings: 6
auth.c: In function 'pg_GSS_recvauth':
auth.c:435: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'
auth.c:456: warning: format '%u' expects type 'unsigned int', but
argument 5 has type 'size_t'
auth.c:464: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'
auth.c: In function 'sendAuthRequest':
auth.c:787: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'

animal: grebe warnings: 45
xlog.c: In function 'XLogInsert':
xlog.c:651: warning: implicit declaration of function '_check_lock'
xlog.c:654: warning: implicit declaration of function '_clear_lock'
hba.c: In function 'ident_unix':
hba.c:1449: warning: implicit declaration of function 'getpeereid'
ip.c: In function 'getaddrinfo_unix':
ip.c:254: warning: large integer implicitly truncated to unsigned type
bgwriter.c: In function 'BackgroundWriterMain':
bgwriter.c:305: warning: implicit declaration of function '_check_lock'
bgwriter.c:308: warning: implicit declaration of function '_clear_lock'
buf_init.c: In function 'InitBufferPool':
buf_init.c:118: warning: implicit declaration of function '_clear_lock'
bufmgr.c: In function 'ReadBuffer_common':
bufmgr.c:249: warning: implicit declaration of function '_check_lock'
bufmgr.c:252: warning: implicit declaration of function '_clear_lock'
freelist.c: In function 'StrategyGetBuffer':
freelist.c:143: warning: implicit declaration of function '_check_lock'
freelist.c:150: warning: implicit declaration of function '_clear_lock'
shmem.c: In function 'InitShmemAllocation':
shmem.c:127: warning: implicit declaration of function '_clear_lock'
shmem.c: In function 'ShmemAlloc':
shmem.c:168: warning: implicit declaration of function '_check_lock'
proc.c: In function 'InitProcGlobal':
proc.c:216: warning: implicit declaration of function '_clear_lock'
proc.c: In function 'InitProcess':
proc.c:247: warning: implicit declaration of function '_check_lock'
lwlock.c: In function 'CreateLWLocks':
lwlock.c:256: warning: implicit declaration of function '_clear_lock'
lwlock.c: In function 'LWLockAssign':
lwlock.c:291: warning: implicit declaration of function '_check_lock'
s_lock.c: In function 's_lock':
s_lock.c:99: warning: implicit declaration of function '_check_lock'
dynahash.c: In function 'init_htab':
dynahash.c:526: warning: implicit declaration of function '_clear_lock'
dynahash.c: In function 'hash_search_with_hash_value':
dynahash.c:876: warning: implicit declaration of function '_check_lock'
guc.c:2866: warning: 'guc_get_index' defined but not used
Extra instructions are being generated for each reference to a TOC
symbol if the symbol is in the TOC overflow area.
ip.c: In function 'getaddrinfo_unix':
ip.c:254: warning: large integer implicitly truncated to unsigned type
connect.c:23: warning: missing braces around initializer
connect.c:23: warning: (near initialization for
'actual_connection_key_once.__on_word')
misc.c:67: warning: missing braces around initializer
misc.c:67: warning: (near initialization for 'sqlca_key_once.__on_word')


is that enough reduction in noise to make it useful ?


Stefan

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Peter Eisentraut wrote:
> Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:
>> a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
>> icc boxes or the "statement not reached" spam from the sun compilers)
>> but others might indicate real issues.
>> To find warnings that might be a real problem we might want to look into
>> suppressing those - if possible - using compiler switches.
>
> It would be good to determine an appropriate set of compiler switches to
> reduce the warnings to a reasonable level.

yeah once we have determined that this whole experiment is useful it
should be pretty easy to tweak the compiler switches for the non-gcc
compilers (mostly icc and sun studio seem to be the ones that generate
excessive output).


Stefan

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> ok I did that for a few members (removing all the statement not reached
> ones as well as some purely informal notices and all the flex related
> warnings) and came up with something similiar to:
> [snip]

Yeah, this looks like a good list. I can't readily check the ones from
"eel" as they appear to be in Windows-specific code; anyone else want to
fix those?

> animal: jackal warnings: 2
> postmaster.c: In function 'PostmasterMain':
> postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
> (declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)

This one we knew about; there's been previous discussion of rewriting
the Bonjour support to use a more portable API, but I don't think anyone
feels like doing it right now.

I'll take a look at the rest.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>> ok I did that for a few members (removing all the statement not reached
>> ones as well as some purely informal notices and all the flex related
>> warnings) and came up with something similiar to:
>> [snip]
>
> Yeah, this looks like a good list. I can't readily check the ones from
> "eel" as they appear to be in Windows-specific code; anyone else want to
> fix those?

The pg_ctl one is a windows one, I'll deal with that one.

The dirmod one doesn't appear on win32, only cygwin. I don't have a
cygwin to check that against, so I'll have to pass on that one.

//Magnus


---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Magnus Hagander <magnus@hagander.net> writes:
> Tom Lane wrote:
>> Yeah, this looks like a good list. I can't readily check the ones from
>> "eel" as they appear to be in Windows-specific code; anyone else want to
>> fix those?

> The pg_ctl one is a windows one, I'll deal with that one.

> The dirmod one doesn't appear on win32, only cygwin. I don't have a
> cygwin to check that against, so I'll have to pass on that one.

Eyeing the code, it looks like the issue is that port.h declares
pgsymlink if
#if defined(WIN32) && !defined(__CYGWIN__)
while dirmod.c defines it if
#ifdef WIN32

So this seems like an actual bug, at least to the extent that pgsymlink
is being compiled into code but not used on Cygwin. But more to the
point, maybe port.h is wrong and we should be using pgsymlink on Cygwin?
In any case these two files need to be put into sync.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Tom Lane wrote:
>>> Yeah, this looks like a good list. I can't readily check the ones from
>>> "eel" as they appear to be in Windows-specific code; anyone else want to
>>> fix those?
>
>> The pg_ctl one is a windows one, I'll deal with that one.
>
>> The dirmod one doesn't appear on win32, only cygwin. I don't have a
>> cygwin to check that against, so I'll have to pass on that one.
>
> Eyeing the code, it looks like the issue is that port.h declares
> pgsymlink if
> #if defined(WIN32) && !defined(__CYGWIN__)
> while dirmod.c defines it if
> #ifdef WIN32
>
> So this seems like an actual bug, at least to the extent that pgsymlink
> is being compiled into code but not used on Cygwin. But more to the
> point, maybe port.h is wrong and we should be using pgsymlink on Cygwin?
> In any case these two files need to be put into sync.

Agreed, but I don't know which should be the master

Well, actually. Since it's not #defined in port.h, that should mean it's
not being used anyway? Since the symbol in dirmod.c is pgsymlink, which
shouldn't be referenced directly anywhere. If that's so, then changing
dirmod.c to just exclude the whole thing on CYGWIN should fix the issue.

I don't have a way to test it, but perhaps we should just throw it in
and see if eel breaks on the buildfarm? Unless some poor soul actually
has a cygwin dev box to test on?

//Magnus


---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>> ok I did that for a few members (removing all the statement not reached
>> ones as well as some purely informal notices and all the flex related
>> warnings) and came up with something similiar to:
>> [snip]
>
> Yeah, this looks like a good list. I can't readily check the ones from
> "eel" as they appear to be in Windows-specific code; anyone else want to
> fix those?
>
>> animal: jackal warnings: 2
>> postmaster.c: In function 'PostmasterMain':
>> postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
>> (declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)
>
> This one we knew about; there's been previous discussion of rewriting
> the Bonjour support to use a more portable API, but I don't think anyone
> feels like doing it right now.
>
> I'll take a look at the rest.

some more(I have removed duplicates and ones that should be fixed by
your latest commits though):

animal: salamander warnings: 27
cash.c: In function `cash_in':
cash.c:244: warning: subscript has type `char'
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here


animal: canary warnings: 14
plpython.c: In function `PLyMapping_ToTuple':
plpython.c:1717: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1732: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1733: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLySequence_ToTuple':
plpython.c:1797: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1821: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1822: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLyObject_ToTuple':
plpython.c:1879: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1892: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1893: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLy_spi_execute_plan':
plpython.c:2434: warning: variable `i' might be clobbered by `longjmp'
or `vfork'

animal: dragonfly warnings: 67
auth.c:61: warning: initialization from incompatible pointer type
cash.c: In function `cash_in':
cash.c:244: warning: subscript has type `char'
connect.c:23: warning: missing braces around initializer
connect.c:23: warning: (near initialization for
`actual_connection_key_once.__pthread_once_pad')
misc.c:67: warning: missing braces around initializer
misc.c:67: warning: (near initialization for
`sqlca_key_once.__pthread_once_pad')


animal: emperor_moth warnings: 10
auth.c:61: warning: initialization from incompatible pointer type


animal: osprey warnings: 22
s_lock.c:222: warning: `tas_dummy' defined but not used
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here
fmgr.c: In function `fmgr_oldstyle':
fmgr.c:629: warning: assignment makes pointer from integer without a cast
fmgr.c:638: warning: assignment makes pointer from integer without a cast
fmgr.c:641: warning: assignment makes pointer from integer without a cast
fmgr.c:645: warning: assignment makes pointer from integer without a cast
fmgr.c:649: warning: assignment makes pointer from integer without a cast
fmgr.c:654: warning: assignment makes pointer from integer without a cast
fmgr.c:659: warning: assignment makes pointer from integer without a cast
fmgr.c:665: warning: assignment makes pointer from integer without a cast
fmgr.c:671: warning: assignment makes pointer from integer without a cast
fmgr.c:678: warning: assignment makes pointer from integer without a cast
fmgr.c:685: warning: assignment makes pointer from integer without a cast
fmgr.c:693: warning: assignment makes pointer from integer without a cast
fmgr.c:701: warning: assignment makes pointer from integer without a cast
fmgr.c:710: warning: assignment makes pointer from integer without a cast
fmgr.c:719: warning: assignment makes pointer from integer without a cast
fmgr.c:729: warning: assignment makes pointer from integer without a cast
fmgr.c:739: warning: assignment makes pointer from integer without a cast

animal: lionfish warnings: 16
/tmp/cclwN8N9.s: Assembler messages:
/tmp/cclwN8N9.s:109082: Warning: Macro instruction expanded into
multiple instructions
/tmp/cclwN8N9.s:109246: Warning: Macro instruction expanded into
multiple instructions
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here
/tmp/ccnsL6Et.s: Assembler messages:
/tmp/ccnsL6Et.s:160502: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:160661: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:160702: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:160805: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:190739: Warning: Macro instruction expanded into
multiple instructions
/tmp/ccnsL6Et.s:192442: Warning: Macro instruction expanded into
multiple instructions
scan.l:180: warning, the character range [<80>-<FF>] is ambiguous in a
case-insensitive scanner
scan.l:180: warning, the character range [<80>-<FF>] is ambiguous in a
case-insensitive scanner
scan.l:302: warning, the character range [<80>-<FF>] is ambiguous in a
case-insensitive scanner


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend