Re: compiler warnings on the buildfarm

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

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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

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/

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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

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

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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

Magnus Hagander <magnus(at)hagander(dot)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

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)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

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
/tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc

He says that this comes from trgm_op.c file. I don't get the warning
myself obviously, so I started guessing.

3FFFFFFFC = 1111111111111111111111111111111100
FFFFFFFC = 11111111111111111111111111111100

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE(). It
looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
bits left. I can see no such operation though.

--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
Dios hizo a Adán, pero fue Eva quien lo hizo hombre.

Alvaro Herrera wrote:
> Stefan showed me via Jabber this warning:
>
> /tmp/ccM7MfqX.s: Assembler messages:
> /tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
> /tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc
>
> He says that this comes from trgm_op.c file. I don't get the warning
> myself obviously, so I started guessing.
>
> 3FFFFFFFC = 1111111111111111111111111111111100
> FFFFFFFC = 11111111111111111111111111111100
>
> So the upper 2 bits are being lost (the second number is 32 bits wide).
>
> The only thing that I think is related is the usage of VARSIZE(). It
> looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
> bits left. I can see no such operation though.

this one is from
http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=caracara&dt=2007-07-12%20200001&stg=make-contrib

as I'm going through the warnings in contrib now.

Stefan

Alvaro Herrera wrote:
> Stefan showed me via Jabber this warning:
>
> /tmp/ccM7MfqX.s: Assembler messages:
> /tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
> /tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc
>
> He says that this comes from trgm_op.c file. I don't get the warning
> myself obviously, so I started guessing.
>
> 3FFFFFFFC = 1111111111111111111111111111111100
> FFFFFFFC = 11111111111111111111111111111100
>
> So the upper 2 bits are being lost (the second number is 32 bits wide).
>
> The only thing that I think is related is the usage of VARSIZE(). It
> looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
> bits left. I can see no such operation though.

The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that
warning is coming from, though. What's the real source behind "ccM7MfqX.s"?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> Alvaro Herrera wrote:
>> Stefan showed me via Jabber this warning:
>> /tmp/ccM7MfqX.s: Assembler messages:
>> /tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to
>> 00000000fffffffc
>> /tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to
>> 00000000fffffffc
>> He says that this comes from trgm_op.c file. I don't get the warning
>> myself obviously, so I started guessing.
>> 3FFFFFFFC = 1111111111111111111111111111111100
>> FFFFFFFC = 11111111111111111111111111111100
>> So the upper 2 bits are being lost (the second number is 32 bits wide).
>> The only thing that I think is related is the usage of VARSIZE(). It
>> looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
>> bits left. I can see no such operation though.
>
> The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that
> warning is coming from, though. What's the real source behind "ccM7MfqX.s"?

trgm_op.c

I'm not sure that the shift in SET_VARSIZE_4B is applicable here,
because it would have to be passed a len of FFFFFFFF.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

"Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> writes:

> Alvaro Herrera wrote:
>> Stefan showed me via Jabber this warning:
>>
>> /tmp/ccM7MfqX.s: Assembler messages:
>> /tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to 00000000fffffffc
>> /tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to 00000000fffffffc
>>
>> He says that this comes from trgm_op.c file. I don't get the warning
>> myself obviously, so I started guessing.

I've occasionally seen this warning too. It seems to be pretty random though.
Nor have I been able to find any actual problems caused by it.

But istm this has to be a compiler bug since any overflow in the C code should
be handled (and potentially warned) by the compiler long before the assembler
gets to see it.

>> So the upper 2 bits are being lost (the second number is 32 bits wide).
>>
>> The only thing that I think is related is the usage of VARSIZE(). It
>> looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
>> bits left. I can see no such operation though.
>
> The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that warning is
> coming from, though. What's the real source behind "ccM7MfqX.s"?

Huh. I wonder.

Could the compiler be incorrectly optimizing cases of

SET_VARSIZE(new_datum VARSIZE(olddatum))

which amounts to:

((olddatum >> 2) & 0x3FFFFFFF) << 2

If it does constant propagation without handling overflow it could end up
with:

(olddatum >> 2 << 2) & 0x3FFFFFFFC

note that in fact truncating the high two bits as the assembler did would in
fact be the correct thing to do here which would explain why it doesn't cause
any actual problems.

Also note that it doesn't require an actual single statement of the form
above. The compiler could be doing constant propagation from an earlier
statement. Code which calls VARSIZE(olddatum) and then passes the result to
SET_VARSIZE(newdatum,len) is quite common.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Alvaro Herrera wrote:
> Heikki Linnakangas wrote:
>> Alvaro Herrera wrote:
>>> Stefan showed me via Jabber this warning:
>>> /tmp/ccM7MfqX.s: Assembler messages:
>>> /tmp/ccM7MfqX.s:703: Warning: 00000003fffffffc shortened to
>>> 00000000fffffffc
>>> /tmp/ccM7MfqX.s:738: Warning: 00000003fffffffc shortened to
>>> 00000000fffffffc
>>> He says that this comes from trgm_op.c file. I don't get the warning
>>> myself obviously, so I started guessing.
>>> 3FFFFFFFC = 1111111111111111111111111111111100
>>> FFFFFFFC = 11111111111111111111111111111100
>>> So the upper 2 bits are being lost (the second number is 32 bits wide).
>>> The only thing that I think is related is the usage of VARSIZE(). It
>>> looks like 0x3FFFFFFF (actual constant in the toast code) is shift two
>>> bits left. I can see no such operation though.
>> The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that
>> warning is coming from, though. What's the real source behind "ccM7MfqX.s"?
>
> trgm_op.c
>
> I'm not sure that the shift in SET_VARSIZE_4B is applicable here,
> because it would have to be passed a len of FFFFFFFF.

Hmm. It looks like I get that warning on my laptop as well. I tracked it
down to these two places:

Line 209:
> while (ptr - GETARR(trg) < ARRNELEM(trg))
> {
> text *item = (text *) palloc(VARHDRSZ + 3);
>
> SET_VARSIZE(item, VARHDRSZ + 3);
> CPTRGM(VARDATA(item), ptr);
>>>>> d[ptr - GETARR(trg)] = PointerGetDatum(item);
> ptr++;
> }

Line 224:
> ptr = GETARR(trg);
> while (ptr - GETARR(trg) < ARRNELEM(trg))
> {
>>>>> pfree(DatumGetPointer(d[ptr - GETARR(trg)]));
> ptr++;
> }

The warning seems to be in related array indexing. If you replace ptr -
GETARR(trg) with a constant, the warning goes away. But having "i = ptr
- GETARR(trg)" in there doesn't give a warning.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
> Hmm. It looks like I get that warning on my laptop as well. I tracked it
> down to these two places:

> Line 209:
>> while (ptr - GETARR(trg) < ARRNELEM(trg))
>> {
>> text *item = (text *) palloc(VARHDRSZ + 3);
>>
>> SET_VARSIZE(item, VARHDRSZ + 3);
>> CPTRGM(VARDATA(item), ptr);
>>>> d[ptr - GETARR(trg)] = PointerGetDatum(item);
>> ptr++;
>> }

I'll betcha the compiler is trying to optimize the repeated calculations
of "ptr - GETARR(trg)" into a separate variable that it increments along
with ptr. Maybe it is getting it wrong, or maybe the assembler is just
confused. Does the warning go away if you dial down the -O level?

regards, tom lane

"Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> writes:

> The warning seems to be in related array indexing. If you replace ptr -
> GETARR(trg) with a constant, the warning goes away. But having "i = ptr -
> GETARR(trg)" in there doesn't give a warning.

Can you compile with -save-temps and send the corresponding assembly file?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> If it does constant propagation without handling overflow it could end up
> with:

> (olddatum >> 2 << 2) & 0x3FFFFFFFC

> note that in fact truncating the high two bits as the assembler did would in
> fact be the correct thing to do here which would explain why it doesn't cause
> any actual problems.

Good point, but I also note that the places Heikki saw were inside
loops. I think it might be some combination of the above and a loop
strength reduction optimization.

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
>> Hmm. It looks like I get that warning on my laptop as well. I tracked it
>> down to these two places:
>
>> Line 209:
>>> while (ptr - GETARR(trg) < ARRNELEM(trg))
>>> {
>>> text *item = (text *) palloc(VARHDRSZ + 3);
>>>
>>> SET_VARSIZE(item, VARHDRSZ + 3);
>>> CPTRGM(VARDATA(item), ptr);
>>>>> d[ptr - GETARR(trg)] = PointerGetDatum(item);
>>> ptr++;
>>> }
>
> I'll betcha the compiler is trying to optimize the repeated calculations
> of "ptr - GETARR(trg)" into a separate variable that it increments along
> with ptr. Maybe it is getting it wrong, or maybe the assembler is just
> confused. Does the warning go away if you dial down the -O level?

Yes, I don't get it with -O1 or -O0.

$ gcc --version
gcc (GCC) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)

Hmm. Prerelease? This version came from debian/testing.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Gregory Stark wrote:
> "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> writes:
>
>> The warning seems to be in related array indexing. If you replace ptr -
>> GETARR(trg) with a constant, the warning goes away. But having "i = ptr -
>> GETARR(trg)" in there doesn't give a warning.
>
> Can you compile with -save-temps and send the corresponding assembly file?

Sure.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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:

I've cleaned up most of this first batch. Open issues are:

> animal: eel warnings: 4
> dirmod.c:206: warning: no previous prototype for 'pgsymlink'

Somebody needs to figure out whether we are supposed to be using
pgsymlink on Cygwin.

> animal: clownfish warnings: 12
> "dynloader.c", line 4: warning: empty translation unit
> "postgres.c", line 3758: warning: loop not entered at top

The first of these is not a bug, the second seems to be some weird
aberration in their statement-not-reached detection.

> animal: grebe warnings: 45
> xlog.c:651: warning: implicit declaration of function '_check_lock'
> xlog.c:654: warning: implicit declaration of function '_clear_lock'
> hba.c:1449: warning: implicit declaration of function 'getpeereid'

Someone needs to find out which system headers declare these functions
on AIX.

> ip.c: In function 'getaddrinfo_unix':
> ip.c:254: warning: large integer implicitly truncated to unsigned type

This is complaining about

#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
unp->sun_len = sizeof(struct sockaddr_un);
#endif

I don't know how wide sun_len is on this platform. It's probably uint8,
but if we explicitly cast the sizeof to 8 bits, we could conceivably
break things on other platforms. Are there any where sockaddr_un is
longer than 255 bytes? Anyway I'm inclined to leave this alone.

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

This is fairly bizarre, since 'guc_get_index' *is* used in guc-file.c,
which is included into this same file. However I don't much like the
coding method used here (it is certainly not better than using a
temporary flag bit), so when I get a chance I'll rewrite it out of
existence.

> 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')

I think these are a platform bug. The spec clearly says that

static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT;

is exactly how you are supposed to do it. If pthread_once_t is a struct
on a given platform, that platform ought to be defining
PTHREAD_ONCE_INIT with the appropriate braces included. If we added
braces ourselves we'd break it for platforms where the macro is correct
already. Hence, not our problem.

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
>> Hmm. It looks like I get that warning on my laptop as well. I tracked it
>> down to these two places:
>
>> Line 209:
>>> while (ptr - GETARR(trg) < ARRNELEM(trg))
>>> {
>>> text *item = (text *) palloc(VARHDRSZ + 3);
>>>
>>> SET_VARSIZE(item, VARHDRSZ + 3);
>>> CPTRGM(VARDATA(item), ptr);
>>>>> d[ptr - GETARR(trg)] = PointerGetDatum(item);
>>> ptr++;
>>> }
>
> I'll betcha the compiler is trying to optimize the repeated calculations
> of "ptr - GETARR(trg)" into a separate variable that it increments along
> with ptr. Maybe it is getting it wrong, or maybe the assembler is just
> confused. Does the warning go away if you dial down the -O level?

FWIW, this patch makes the warnings go away, and makes the code a little
bit more readable as well. It would be nice to understand why exactly
it's complaining, though.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Tom Lane wrote:
>> animal: eel warnings: 4
>> dirmod.c:206: warning: no previous prototype for 'pgsymlink'
>>
>
> Somebody needs to figure out whether we are supposed to be using
> pgsymlink on Cygwin.
>

According to port.h:

* Cygwin has its own symlinks which work on Win95/98/ME where
* junction points don't, so use it instead. We have no way of
* knowing what type of system Cygwin binaries will be run on.

So it looks like we should not be using pgsymlink() on Cygwin, unless we
declare that these platforms under Cygwin are no longer supported.

cheers

andrew

Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
> FWIW, this patch makes the warnings go away, and makes the code a little
> bit more readable as well. It would be nice to understand why exactly
> it's complaining, though.

Let's apply the patch. We are clearly tickling a bug or near-bug in
gcc, and it may have worse consequences than this on other platforms
or other gcc versions.

At the same time, if anyone wants to trim the existing code down to a
small test case, I'm sure the gcc boys would appreciate a bug report.

regards, tom lane

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> Somebody needs to figure out whether we are supposed to be using
>> pgsymlink on Cygwin.

> According to port.h:
> * Cygwin has its own symlinks which work on Win95/98/ME where
> * junction points don't, so use it instead. We have no way of
> * knowing what type of system Cygwin binaries will be run on.

> So it looks like we should not be using pgsymlink() on Cygwin, unless we
> declare that these platforms under Cygwin are no longer supported.

Fair enough; Cygwin is kind of a legacy port at this point anyway, so
we shouldn't be doing anything to reduce its backwards compatibility.

I'll fix dirmod.c to match port.h.

regards, tom lane

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
> animal: dragonfly warnings: 67
> auth.c:61: warning: initialization from incompatible pointer type

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

Apparently, Solaris 9 and 10 have funny ideas about the signature of the
PAM callback function. We have

static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
struct pam_response ** resp, void *appdata_ptr);

which exactly matches what my Fedora 6 pam header file says it should
be. What is it on those Solaris machines? (Look for struct pam_conv
in the headers)

regards, tom lane

On Thu, 12 Jul 2007, Tom Lane wrote:

> static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
> struct pam_response ** resp, void *appdata_ptr);
>
> which exactly matches what my Fedora 6 pam header file says it should
> be. What is it on those Solaris machines?

struct pam_conv {
int (*conv)(int, struct pam_message **,
struct pam_response **, void *);
void *appdata_ptr; /* Application data ptr */
};

So pam_message ** isn't const.

Kris Jurka

Tom Lane wrote:
> At the same time, if anyone wants to trim the existing code down to a
> small test case, I'm sure the gcc boys would appreciate a bug report.

I reduced it to a self-contained test case, and filed bug in GCC
bugzilla: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32750

Surprisingly, this doesn't seem to be related to varvarlen at all. I'm
getting the same warnings on 8.2 and 8.1 as well.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Kris Jurka <books(at)ejurka(dot)com> writes:
> So pam_message ** isn't const.

Ah, thanks. I see luna_moth is giving the same warning, so it's still
not const in Solaris 11 either.

Is it worth working around this? It's strictly cosmetic AFAICS.

The main issue in my mind would be how to determine whether to use
const or not. If all Solaris releases are like this, and can be
expected to stay that way, I'd be inclined to just put a "#define
PAM_CONV_PROC_NOT_CONST" in include/port/solaris.h and drive the
function declaration off that. If there's a version dependency
involved then it gets a lot more complicated, and might not be worth
the trouble.

regards, tom lane

On Thu, 12 Jul 2007, Stefan Kaltenbrunner wrote:

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

I had gone through and looked at the warnings on mongoose before, but I am
running it against the current code now. Let me know if you want line
numbers on any of these...

count | msgtype | msgno | msg
-------+---------+-------+--------------------------------------------------------------------------------------------------------------------
552 | warning | 1292 | attribute "warn_unused_result" ignored

This is due to perl headers, so don't worry about this one

77 | warning | 188 | enumerated type mixed with another type
16 | warning | 186 | pointless comparison of unsigned integer with zero
9 | warning | 167 | argument of type "int *" is incompatible with parameter of type "socklen_t={__socklen_t={unsigned int}} *restrict"
2 | warning | 300 | const variable "all_zeroes" requires an initializer
1 | warning | 556 | a value of type "void *" cannot be assigned to an entity of type "rl_completion_func_t *"
(6 rows)

--
Give thought to your reputation. Consider changing name and moving to
a new town.

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
> animal: lionfish warnings: 16
> 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

This is evidently complaining about plpgsql's scan.l, which specifies
%option case-insensitive
and then defines
ident_start [A-Za-z\200-\377_]
which is the way we do it in the main grammar too. But I've never
seen this message in any of the flex versions I've used with PG.
(Which flex version is installed on lionfish anyway?)

I find some relevant points in the flex manual:
http://flex.sourceforge.net/manual/Patterns.html

Character classes are expanded immediately when seen in the flex
input. This means the character classes are sensitive to the locale in
which flex is executed, and the resulting scanner will not be sensitive
to the runtime locale. This may or may not be desirable.

Character classes with ranges, such as `[a-Z]', should be used with
caution in a case-insensitive scanner if the range spans upper or
lowercase characters. Flex does not know if you want to fold all upper
and lowercase characters together, or if you want the literal numeric
range specified (with no case folding). When in doubt, flex will assume
that you meant the literal numeric range, and will issue a warning. The
exception to this rule is a character range such as `[a-z]' or `[S-W]'
where it is obvious that you want case-folding to occur.

What I suspect is happening is that lionfish is running the buildfarm
script in a non-C locale, in which flex finds that some high-bit-set
characters are case-folded by tolower() and accordingly issues this
complaint. Now the statements that "it assumes you meant the literal
numeric range" and that the behavior is fully determined at compile time
(ie, no run-time invocations of tolower(), as indeed are not to be seen
in pl_scan.c) seem to mean that we'll get the behavior we want anyway.
But the warning is a bit nervous-making.

I wonder if it'd be a good idea to invoke flex with a command like
LANG=C flex ...
to try to improve the odds that it sees C locale when it's figuring
out what "case insensitive" means.

Anyone want to look into it more closely?

regards, tom lane

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
> some more(I have removed duplicates and ones that should be fixed by
> your latest commits though):

I did what I could with this batch. Some comments:

> animal: salamander warnings: 27
> cash.c: In function `cash_in':
> cash.c:244: warning: subscript has type `char'

I wish we could promote this one to be a hard error :-(. It typically
indicates (and did in this case) that someone has unportably forgotten
to cast the argument of a <ctype.h> macro to unsigned char.

> pg_lzcompress.c: In function `pglz_compress':
> pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'

This is not an error condition, it just means that gcc decided not to do
inlining because the called function was too big. IIRC we had some
discussion whether to specify -Winline or not, and decided to do so in
order to gather some info about whether we were overstressing "inline".
We could live with it as-is, or document somewhere (where?) that "it's
fine as long as you don't see very many of 'em", or decide that the
experiment is finished and we should take out -Winline. Comments?

> animal: osprey warnings: 22
> s_lock.c:222: warning: `tas_dummy' defined but not used

Fixing this one seems to require making tas_dummy a global name, which
does not seem like a net improvement.

> animal: lionfish warnings: 16
> /tmp/cclwN8N9.s: Assembler messages:
> /tmp/cclwN8N9.s:109082: Warning: Macro instruction expanded into
> multiple instructions
> [multiple occurrences]

This is pretty strange. It seems to occur only in files generated
from bison and/or flex. Anybody have a clue?

> scan.l:180: warning, the character range [<80>-<FF>] is ambiguous in a
> case-insensitive scanner

I commented on this already.

regards, tom lane

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
>> animal: lionfish warnings: 16
>> 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
>
> This is evidently complaining about plpgsql's scan.l, which specifies
> %option case-insensitive
> and then defines
> ident_start [A-Za-z\200-\377_]
> which is the way we do it in the main grammar too. But I've never
> seen this message in any of the flex versions I've used with PG.
> (Which flex version is installed on lionfish anyway?)

$ flex -V
flex 2.5.31

>
> I find some relevant points in the flex manual:
> http://flex.sourceforge.net/manual/Patterns.html
>
> Character classes are expanded immediately when seen in the flex
> input. This means the character classes are sensitive to the locale in
> which flex is executed, and the resulting scanner will not be sensitive
> to the runtime locale. This may or may not be desirable.
>
> Character classes with ranges, such as `[a-Z]', should be used with
> caution in a case-insensitive scanner if the range spans upper or
> lowercase characters. Flex does not know if you want to fold all upper
> and lowercase characters together, or if you want the literal numeric
> range specified (with no case folding). When in doubt, flex will assume
> that you meant the literal numeric range, and will issue a warning. The
> exception to this rule is a character range such as `[a-z]' or `[S-W]'
> where it is obvious that you want case-folding to occur.
>
> What I suspect is happening is that lionfish is running the buildfarm
> script in a non-C locale, in which flex finds that some high-bit-set
> characters are case-folded by tolower() and accordingly issues this
> complaint. Now the statements that "it assumes you meant the literal
> numeric range" and that the behavior is fully determined at compile time
> (ie, no run-time invocations of tolower(), as indeed are not to be seen
> in pl_scan.c) seem to mean that we'll get the behavior we want anyway.
> But the warning is a bit nervous-making.

hmmm - note that lionfish is not the only box reporting that kind of
warning - also affected are:

rosella (which is definitly running in a non-C locale as all the errors
are in german there)
wildebeest

Stefan

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
>> some more(I have removed duplicates and ones that should be fixed by
>> your latest commits though):
>
> I did what I could with this batch. Some comments:
>
>> animal: salamander warnings: 27
>> cash.c: In function `cash_in':
>> cash.c:244: warning: subscript has type `char'
>
> I wish we could promote this one to be a hard error :-(. It typically
> indicates (and did in this case) that someone has unportably forgotten
> to cast the argument of a <ctype.h> macro to unsigned char.

:-( we can promote certain warnings to an error on sun studio vor
example but salamander is running gcc ...

>
>> pg_lzcompress.c: In function `pglz_compress':
>> pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
>
> This is not an error condition, it just means that gcc decided not to do
> inlining because the called function was too big. IIRC we had some
> discussion whether to specify -Winline or not, and decided to do so in
> order to gather some info about whether we were overstressing "inline".
> We could live with it as-is, or document somewhere (where?) that "it's
> fine as long as you don't see very many of 'em", or decide that the
> experiment is finished and we should take out -Winline. Comments?

we have about 5 boxes on the farm with that exact warning (inlining
failed in pglz_find_match) there are no other inline related warnings at
all afaiks.

>> animal: lionfish warnings: 16
>> /tmp/cclwN8N9.s: Assembler messages:
>> /tmp/cclwN8N9.s:109082: Warning: Macro instruction expanded into
>> multiple instructions
>> [multiple occurrences]
>
> This is pretty strange. It seems to occur only in files generated
> from bison and/or flex. Anybody have a clue?

other than that lionfish is a weird mips box - no :-)

Stefan

Tom Lane wrote:
[...]
>> animal: clownfish warnings: 12
>> "dynloader.c", line 4: warning: empty translation unit
>> "postgres.c", line 3758: warning: loop not entered at top
>
> The first of these is not a bug, the second seems to be some weird
> aberration in their statement-not-reached detection.

will see about filtering out those

>
>> animal: grebe warnings: 45
>> xlog.c:651: warning: implicit declaration of function '_check_lock'
>> xlog.c:654: warning: implicit declaration of function '_clear_lock'
>> hba.c:1449: warning: implicit declaration of function 'getpeereid'
>
> Someone needs to find out which system headers declare these functions
> on AIX.
>
>> ip.c: In function 'getaddrinfo_unix':
>> ip.c:254: warning: large integer implicitly truncated to unsigned type
>
> This is complaining about
>
> #ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
> unp->sun_len = sizeof(struct sockaddr_un);
> #endif
>
> I don't know how wide sun_len is on this platform. It's probably uint8,
> but if we explicitly cast the sizeof to 8 bits, we could conceivably
> break things on other platforms. Are there any where sockaddr_un is
> longer than 255 bytes? Anyway I'm inclined to leave this alone.

no idea on AIX but I have added christopher to the CC list - maybe he
can shed some light on those things.

>
>> 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.
>
> This is fairly bizarre, since 'guc_get_index' *is* used in guc-file.c,
> which is included into this same file. However I don't much like the
> coding method used here (it is certainly not better than using a
> temporary flag bit), so when I get a chance I'll rewrite it out of
> existence.
>
>> 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')
>
> I think these are a platform bug. The spec clearly says that
>
> static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT;
>
> is exactly how you are supposed to do it. If pthread_once_t is a struct
> on a given platform, that platform ought to be defining
> PTHREAD_ONCE_INIT with the appropriate braces included. If we added
> braces ourselves we'd break it for platforms where the macro is correct
> already. Hence, not our problem.

I see

Stefan

Tom Lane wrote:
> Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)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?

and this is the initial list for contrib(excluding a lot of duplicate
warnings and stuff that is a result of invalid compiler flags which I
will mention seperatly):

animal: salamander warnings: 6
stopword.c: In function `readstoplist':
stopword.c:51: warning: subscript has type `char'

animal: dragonfly warnings: 4
pgbench.c: In function `main':
pgbench.c:1445: warning: int format, pid_t arg (arg 4)
stopword.c: In function `readstoplist':
stopword.c:51: warning: subscript has type `char'

animal: clownfish warnings: 12
"crc32.c", line 93: warning: initializer does not fit or is out of range: -1
"crc32.c", line 102: warning: initializer does not fit or is out of
range: -1
"imath.c", line 3202: warning: integer overflow detected: op "<<"
"imath.c", line 3206: warning: integer overflow detected: op "<<"
"query_cleanup.c", line 179: warning: macro redefined: V_FALSE
"crc32.c", line 95: warning: initializer does not fit or is out of range: -1
"query_support.c", line 199: warning: syntax error: empty declaration
"query_support.c", line 200: warning: syntax error: empty declaration
"query_support.c", line 201: warning: syntax error: empty declaration
"query_support.c", line 202: warning: syntax error: empty declaration
"query_support.c", line 203: warning: syntax error: empty declaration
"query_support.c", line 204: warning: syntax error: empty declaration

animal: kudu warnings: 13
"crc32.c", line 93: warning: initializer does not fit or is out of range: -1
"crc32.c", line 102: warning: initializer does not fit or is out of
range: -1
"oid2name.c", line 579: warning: Function has no return statement : main
"pg_standby.c", line 622: warning: Function has no return statement : main
"imath.c", line 3202: warning: integer overflow detected: op "<<"
"dict_thesaurus.c", line 699: warning: non-constant initializer: op "NAME"
"crc32.c", line 95: warning: initializer does not fit or is out of range: -1
"query_support.c", line 199: warning: syntax error: empty declaration
"query_support.c", line 200: warning: syntax error: empty declaration
"query_support.c", line 201: warning: syntax error: empty declaration
"query_support.c", line 202: warning: syntax error: empty declaration
"query_support.c", line 203: warning: syntax error: empty declaration
"query_support.c", line 204: warning: syntax error: empty declaration

animal: warthog warnings: 396
UX:acomp: WARNING: "btreefuncs.c", line 59: no macro replacement within
a string literal
UX:acomp: WARNING: "pgstatindex.c", line 50: no macro replacement within
a string literal
UX:acomp: WARNING: "xpath.c", line 212: argument #1 incompatible with
prototype: strlen()
UX:acomp: WARNING: "xpath.c", line 268: argument #2 incompatible with
prototype: xmlBufferWriteChar()
UX:acomp: WARNING: "xpath.c", line 607: argument #1 incompatible with
prototype: xmlStrdup()
UX:acomp: WARNING: "xpath.c", line 612: argument #1 incompatible with
prototype: strlen()
UX:acomp: WARNING: "xpath.c", line 663: assignment type mismatch
UX:acomp: WARNING: "xpath.c", line 738: assignment type mismatch
UX:acomp: WARNING: "xpath.c", line 742: argument #1 incompatible with
prototype: strstr()
UX:acomp: WARNING: "xpath.c", line 742: argument #2 incompatible with
prototype: strstr()
UX:acomp: WARNING: "xpath.c", line 742: assignment type mismatch
UX:acomp: WARNING: "xpath.c", line 896: argument #1 incompatible with
prototype: xmlStrdup()
UX:acomp: WARNING: "xpath.c", line 904: assignment type mismatch
UX:acomp: WARNING: "xslt_proc.c", line 105: argument #1 incompatible
with prototype: xsltParseStylesheetFile()

animal: emperor_moth warnings: 11
pgbench.c: In function `main':
pgbench.c:1445: warning: int format, pid_t arg (arg 4)
query_cleanup.c:179:1: warning: "V_FALSE" redefined
In file included from /usr/include/sys/stream.h:22,
from /usr/include/netinet/in.h:66,
from /usr/include/netdb.h:98,
from ../../src/include/port.h:17,
from ../../src/include/c.h:839,
from ../../src/include/postgres.h:48,
from query_cleanup.c:6:
/usr/include/sys/vnode.h:505:1: warning: this is the location of the
previous definition

animal: cuckoo warnings: 9
y.tab.c: In function 'yy_reduce_print':
y.tab.c:764: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c: In function 'yydestruct':
y.tab.c:1036: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c: In function 'cube_yyparse':
y.tab.c:1277: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c:1303: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c:1470: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c:1624: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type

animal: dugong warnings: 21
btreefuncs.c(218): warning #186: pointless comparison of unsigned
integer with zero
{ if ( (blkno)<0 && RelationGetNumberOfBlocks((rel))<=(blkno) )
elog_start("btreefuncs.c", 218, __func__), elog_finish(20
, "Block number out of range."); };
^
btreefuncs.c(341): warning #186: pointless comparison of unsigned
integer with zero
{ if ( (blkno)<0 &&
RelationGetNumberOfBlocks((uargs->rel))<=(blkno) )
elog_start("btreefuncs.c", 341, __func__),
elog_finish(20, "Block number out of range."); };
^
../../src/include/storage/s_lock.h(246): warning #167: argument of type
"volatile slock_t={unsigned int} *" is incompatible with
parameter of type "void *"
ret = _InterlockedExchange(lock,1);
^
pg_buffercache_pages.c(116): warning #188: enumerated type mixed with
another type
LWLockAcquire(FirstBufMappingLock + i, LW_SHARED);
^
pg_buffercache_pages.c(150): warning #188: enumerated type mixed with
another type
LWLockRelease(FirstBufMappingLock + i);
^
mbuf.c(45): warning #108: implicitly-signed bit field of length 1
int no_write:1;
^
mbuf.c(46): warning #108: implicitly-signed bit field of length 1
int own_data:1;
^

animal: grebe warnings: 3
pg_buffercache_pages.c: In function 'pg_buffercache_pages':
pg_buffercache_pages.c:125: warning: implicit declaration of function
'_check_lock'
pg_buffercache_pages.c:145: warning: implicit declaration of function
'_clear_lock'

which is probably the same problem as already discussed

animal: mongoose warnings: 46
btreefuncs.c(218): warning #186: pointless comparison of unsigned
integer with zero
CHECK_RELATION_BLOCK_RANGE(rel, blkno);
^
btreefuncs.c(341): warning #186: pointless comparison of unsigned
integer with zero
CHECK_RELATION_BLOCK_RANGE(uargs->rel, blkno);
^
pg_buffercache_pages.c(116): warning #188: enumerated type mixed with
another type
LWLockAcquire(FirstBufMappingLock + i, LW_SHARED);
^
pg_buffercache_pages.c(150): warning #188: enumerated type mixed with
another type
LWLockRelease(FirstBufMappingLock + i);
^
crypt-des.c(257) : (col. 2) remark: LOOP WAS AUTO-PARALLELIZED.
mbuf.c(45): warning #108: implicitly-signed bit field of length 1
int no_write:1;
^
mbuf.c(46): warning #108: implicitly-signed bit field of length 1
int own_data:1;
^
xpath.c(212): warning #167: argument of type "xmlChar={unsigned char} *"
is incompatible with parameter of type "const char *"
ressize = strlen(tt);
^
xpath.c(268): warning #167: argument of type "xmlChar={unsigned char} *"
is incompatible with parameter of type "const char *"
xmlBufferWriteChar(buf, plainsep);
^
xpath.c(612): warning #167: argument of type "xmlChar={unsigned char} *"
is incompatible with parameter of type "const char *"
ressize = strlen(xpresstr);
^
xpath.c(738): warning #556: a value of type "char *" cannot be assigned
to an entity of type "xmlChar={unsigned char} *"
pos = xpathset;
^
xpath.c(742): warning #167: argument of type "xmlChar={unsigned char} *"
is incompatible with parameter of type "const char *"
pos = strstr(pos, pathsep);
^
xpath.c(742): warning #167: argument of type "xmlChar={unsigned char} *"
is incompatible with parameter of type "const char *"
pos = strstr(pos, pathsep);
^
xpath.c(742): warning #556: a value of type "char *" cannot be assigned
to an entity of type "xmlChar={unsigned char} *"
pos = strstr(pos, pathsep);
^
xpath.c(904): warning #556: a value of type "xmlChar={unsigned char} *"
cannot be assigned to an entity of type "char *"
values[j + 1] = resstr;
^
xslt_proc.c(105): warning #167: argument of type "char *" is
incompatible with parameter of type "const xmlChar={unsigned char} *"
stylesheet = xsltParseStylesheetFile(GET_STR(ssheet));
^

Stefan

Kris Jurka wrote:
>
>
> On Thu, 12 Jul 2007, Tom Lane wrote:
>
>> static int pam_passwd_conv_proc(int num_msg, const struct pam_message
>> ** msg,
>> struct pam_response ** resp, void *appdata_ptr);
>>
>> which exactly matches what my Fedora 6 pam header file says it should
>> be. What is it on those Solaris machines?
>
> struct pam_conv {
> int (*conv)(int, struct pam_message **,
> struct pam_response **, void *);
> void *appdata_ptr; /* Application data ptr */
> };
>
> So pam_message ** isn't const.
>

Yes, according to X/Open XSSO Standard - see
http://www.opengroup.org/onlinepubs/008329799/
http://www.opengroup.org/onlinepubs/008329799/pam_start.htm#tagcjh_07_32

Zdenek

Stefan Kaltenbrunner wrote:
> 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).
>

For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
want to determine warning tags for each warning add -errtags.

Zdenek

Zdenek Kotala wrote:
> Stefan Kaltenbrunner wrote:
>> 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).
>>
>
> For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
> want to determine warning tags for each warning add -errtags.

Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
Studio 8,11) we have on the farm ?

Stefan

Tom Lane wrote:
> Kris Jurka <books(at)ejurka(dot)com> writes:
>> So pam_message ** isn't const.
>
> Ah, thanks. I see luna_moth is giving the same warning, so it's still
> not const in Solaris 11 either.
>
> Is it worth working around this? It's strictly cosmetic AFAICS.
>
> The main issue in my mind would be how to determine whether to use
> const or not. If all Solaris releases are like this, and can be
> expected to stay that way,

I think yes. It is defined as X/Open standard says.

> I'd be inclined to just put a "#define
> PAM_CONV_PROC_NOT_CONST" in include/port/solaris.h and drive the
> function declaration off that. If there's a version dependency
> involved then it gets a lot more complicated, and might not be worth
> the trouble.

Following patch works for me, but I did not test it on other platform.

retrieving revision 1.153
diff -r1.153 auth.c
61c61
< &pam_passwd_conv_proc,
---
> (int (*)())pam_passwd_conv_proc,

Zdenek

Stefan Kaltenbrunner wrote:
> Zdenek Kotala wrote:

>> For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
>> want to determine warning tags for each warning add -errtags.
>
> Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
> Studio 8,11) we have on the farm ?

Yes. Also on SS12.

Zdenek

On Fri, 13 Jul 2007, Zdenek Kotala wrote:

> Tom Lane wrote:
>> Kris Jurka <books(at)ejurka(dot)com> writes:
>>> So pam_message ** isn't const.
>>
>> Ah, thanks. I see luna_moth is giving the same warning, so it's still
>> not const in Solaris 11 either.
>>
>> Is it worth working around this? It's strictly cosmetic AFAICS.
>>
>> The main issue in my mind would be how to determine whether to use
>> const or not. If all Solaris releases are like this, and can be
>> expected to stay that way,
>
> I think yes. It is defined as X/Open standard says.
>

Not according to the link you sent earlier. My reading says that Solaris
has it defined wrong and pg has it right.

Kris Jurka

Kris Jurka wrote:
>
>
> On Fri, 13 Jul 2007, Zdenek Kotala wrote:
>
>> Tom Lane wrote:
>>> Kris Jurka <books(at)ejurka(dot)com> writes:
>>>> So pam_message ** isn't const.
>>>
>>> Ah, thanks. I see luna_moth is giving the same warning, so it's still
>>> not const in Solaris 11 either.
>>>
>>> Is it worth working around this? It's strictly cosmetic AFAICS.
>>>
>>> The main issue in my mind would be how to determine whether to use
>>> const or not. If all Solaris releases are like this, and can be
>>> expected to stay that way,
>>
>> I think yes. It is defined as X/Open standard says.
>>
>
> Not according to the link you sent earlier. My reading says that
> Solaris has it defined wrong and pg has it right.

If I look there
http://www.opengroup.org/onlinepubs/008329799/chap5.htm#tagcjh_06

in "Call Back Information" section. The structure is defined as

struct pam_conv{ int (*conv) (int, struct pam_message **, struct
pam_response **, void *); void *appdata_ptr; };

I don't see any "const" keyword there.

Zdenek

Zdenek Kotala wrote:
> If I look there
> http://www.opengroup.org/onlinepubs/008329799/chap5.htm#tagcjh_06
>
> in "Call Back Information" section. The structure is defined as
>
> struct pam_conv{ int (*conv) (int, struct pam_message **, struct
> pam_response **, void *); void *appdata_ptr; };
>
>
> I don't see any "const" keyword there.

Right after that:

> where int conv(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr);

How confusing...

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> Zdenek Kotala wrote:
>> If I look there
>> http://www.opengroup.org/onlinepubs/008329799/chap5.htm#tagcjh_06
>>
>> in "Call Back Information" section. The structure is defined as
>>
>> struct pam_conv{ int (*conv) (int, struct pam_message **, struct
>> pam_response **, void *); void *appdata_ptr; };
>>
>>
>> I don't see any "const" keyword there.
>
> Right after that:
>
>> where int conv(int num_msg, const struct pam_message **msg,
>> struct pam_response **resp, void *appdata_ptr);

Ups, I overlooked it.

> How confusing...

Yes agree.

Zdenek

Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
> Zdenek Kotala wrote:
>> I don't see any "const" keyword there.

> Right after that:

>> where int conv(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr);

> How confusing...

And the pam_start page he cited earlier has a different set of typos in
its version of the struct :-(. Still, that's two out of three places
that say it's const, and Solaris appears to be the only implementation
that has chosen to read it as not const.

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
>> Zdenek Kotala wrote:
>>> I don't see any "const" keyword there.
>
>> Right after that:
>
>>> where int conv(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr);
>
>> How confusing...
>
> And the pam_start page he cited earlier has a different set of typos in
> its version of the struct :-(. Still, that's two out of three places
> that say it's const, and Solaris appears to be the only implementation
> that has chosen to read it as not const.

Yes, Agree. I try to send request to security team for explanation.
Maybe original author also overlooked it as me today :-).

Zdenek

stefan(at)kaltenbrunner(dot)cc (Stefan Kaltenbrunner) writes:

> Tom Lane wrote:
> [...]
>>> animal: clownfish warnings: 12
>>> "dynloader.c", line 4: warning: empty translation unit
>>> "postgres.c", line 3758: warning: loop not entered at top
>>
>> The first of these is not a bug, the second seems to be some weird
>> aberration in their statement-not-reached detection.
>
> will see about filtering out those
>
>>
>>> animal: grebe warnings: 45
>>> xlog.c:651: warning: implicit declaration of function '_check_lock'
>>> xlog.c:654: warning: implicit declaration of function '_clear_lock'
>>> hba.c:1449: warning: implicit declaration of function 'getpeereid'
>>
>> Someone needs to find out which system headers declare these functions
>> on AIX.

Hmm. Logging onto grebe:

/usr/include/sys/socket.h:int getpeereid(int, uid_t *__restrict__, gid_t *__restrict__);

ydb1.int.libertyrms.com(cbbrowne): /home/cbbrowne # egrep '_(check|clear)_lock' /usr/include/*/*.h
/usr/include/sys/atomic_op.h:boolean_t _check_lock();
/usr/include/sys/atomic_op.h:void _clear_lock();
/usr/include/sys/atomic_op.h:void _clear_lock_mem();
/usr/include/sys/atomic_op.h:boolean_t _check_lock(atomic_p, int, int);
/usr/include/sys/atomic_op.h:void _clear_lock(atomic_p, int);
/usr/include/sys/atomic_op.h:void _clear_lock_mem(atomic_p, int);

Do those seem apropos?

>>> ip.c: In function 'getaddrinfo_unix':
>>> ip.c:254: warning: large integer implicitly truncated to unsigned type
>>
>> This is complaining about
>>
>> #ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
>> unp->sun_len = sizeof(struct sockaddr_un);
>> #endif
>>
>> I don't know how wide sun_len is on this platform. It's probably uint8,
>> but if we explicitly cast the sizeof to 8 bits, we could conceivably
>> break things on other platforms. Are there any where sockaddr_un is
>> longer than 255 bytes? Anyway I'm inclined to leave this alone.
>
> no idea on AIX but I have added christopher to the CC list - maybe he
> can shed some light on those things.

/* According to RFC3493 sockaddr_storage structure should be greater than or
equal to the largest sockaddr struct. The size of sockaddr_un structure changed to 1025 in order to support long user names. Change _SS_MAXSIZE accordingly
inorder to main compliance to the RFC */
#define _SS_MAXSIZE 1280 /* Implementation specific max size */

Actually, you can take a look at doc/FAQ_AIX; that reports that the
size was updated to 1028 back in 2005, as a result, in fact, of my bug
submission :-).

The comment in the #include seems somewhat nonsensical; the reason for
increasing sockaddr_un was to support IPv6 addresses. I didn't think
it had anything to do with user names...

[Aside: Sorry, I don't have any flames about EDB/CMD today. Boy, you
miss reading -advocacy for half a day, and sometimes you miss
something big...]
--
output = reverse("gro.mca" "@" "enworbbc")
http://www3.sympatico.ca/cbbrowne/linuxxian.html
"One of my most often repeated quips was the one I made when former
Presidents Carter, Ford and Nixon stood by each other at a White House
event. 'There they are,' I said. 'See no evil, hear no evil, and ...
evil.'" -- Bob Dole, 1983

Zdenek Kotala wrote:
> Stefan Kaltenbrunner wrote:
>> Zdenek Kotala wrote:
>
>>> For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
>>> want to determine warning tags for each warning add -errtags.
>>
>> Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
>> Studio 8,11) we have on the farm ?
>
> Yes. Also on SS12.

hmm - sure about that ? I was about to submit a patch to disable some
compiler warnings but then I noted the following discussion thread:

http://forum.java.sun.com/thread.jspa?threadID=5163903&messageID=9637391

which seems to indicate that at least the compiler installed on kudu:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kudu&dt=2007-07-15%2003:30:01

does NOT support turning of specific warnings.

Stefan

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
> Tom Lane wrote:
>> What I suspect is happening is that lionfish is running the buildfarm
>> script in a non-C locale, in which flex finds that some high-bit-set
>> characters are case-folded by tolower() and accordingly issues this
>> complaint. Now the statements that "it assumes you meant the literal
>> numeric range" and that the behavior is fully determined at compile time
>> (ie, no run-time invocations of tolower(), as indeed are not to be seen
>> in pl_scan.c) seem to mean that we'll get the behavior we want anyway.
>> But the warning is a bit nervous-making.

> hmmm - note that lionfish is not the only box reporting that kind of
> warning - also affected are:
> rosella (which is definitly running in a non-C locale as all the errors
> are in german there)
> wildebeest

I looked at the flex source code and it seems that indeed we *should*
expect to see that warning if we run flex in a locale in which any
characters in the range \200-\377 are letters that case-fold to plain
ASCII. Turkish ought to meet that criterion (the ol dotted vs dotless
i business) but I'm not sure about German. I could not get the warning
on plpgsql's scan.l with a local build of flex 2.5.33, though, no matter
what locale I ran it in. Odd.

Anyway, I tweaked plpgsql's Makefile to force LC_CTYPE=C, which
theoretically should silence this warning.

regards, tom lane

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Anyway, I tweaked plpgsql's Makefile to force LC_CTYPE=C, which
> theoretically should silence this warning.

This doesn't mean that people were previousy able to use any of these "exotic"
characters like aßertion or explaïn if they happened to compile in the wrong
(or right) locale and now they won't be able to does it?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc> writes:
> and this is the initial list for contrib(excluding a lot of duplicate
> warnings and stuff that is a result of invalid compiler flags which I
> will mention seperatly):

I fixed most of these, I believe. A couple remain untouched:

> animal: cuckoo warnings: 9
> y.tab.c: In function 'yy_reduce_print':
> y.tab.c:764: warning: passing argument 3 of 'yy_symbol_print' from
> incompatible pointer type

I don't see this on either PPC or Intel Mac. I think the problem is
probably an old bison version on this buildfarm member.

> animal: dugong warnings: 21
> ../../src/include/storage/s_lock.h(246): warning #167: argument of type
> "volatile slock_t={unsigned int} *" is incompatible with
> parameter of type "void *"
> ret = _InterlockedExchange(lock,1);
> ^

I see this is not just contrib but throughout the core too on this
machine. We could possibly suppress it by casting away volatile in the
tas() function, but I wonder if that might have bad side-effects. Any
thoughts?

> pg_buffercache_pages.c(116): warning #188: enumerated type mixed with
> another type
> LWLockAcquire(FirstBufMappingLock + i, LW_SHARED);
> ^

This warning occurs in too many places to want to fix, also.

regards, tom lane

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> Anyway, I tweaked plpgsql's Makefile to force LC_CTYPE=3DC, which
>> theoretically should silence this warning.

> This doesn't mean that people were previousy able to use any of these "exot=
> ic"
> characters like a=DFertion or expla=EFn if they happened to compile in the =
> wrong
> (or right) locale and now they won't be able to does it?

No.

regards, tom lane

Chris Browne <cbbrowne(at)acm(dot)org> writes:
> Tom Lane wrote:
>>> animal: grebe warnings: 45
>>> xlog.c:651: warning: implicit declaration of function '_check_lock'
>>> xlog.c:654: warning: implicit declaration of function '_clear_lock'
>>> hba.c:1449: warning: implicit declaration of function 'getpeereid'

>> Someone needs to find out which system headers declare these functions
>> on AIX.

> Hmm. Logging onto grebe:

> /usr/include/sys/socket.h:int getpeereid(int, uid_t *__restrict__, gid_t *__restrict__);

That's pretty strange, because hba.c definitely includes <sys/socket.h>.
Perhaps getpeereid is hidden within some #ifdef that we aren't setting?

> /usr/include/sys/atomic_op.h:boolean_t _check_lock();
> /usr/include/sys/atomic_op.h:void _clear_lock();

OK, I'll try putting <sys/atomic_op.h> into the AIX part of s_lock.h.

regards, tom lane

Do any of the build farm machines not support 64-bit integers? I just added a
--enable-bigint flag to configure.in and tested building without it and got an
error at xlog.c:

xlog.c: In function 'ValidXLOGHeader':
xlog.c:3240: error: 'UINT64_FORMAT' undeclared (first use in this function)
xlog.c:3240: error: (Each undeclared identifier is reported only once
xlog.c:3240: error: for each function it appears in.)

snprintf(fhdrident_str, sizeof(fhdrident_str), UINT64_FORMAT,
longhdr->xlp_sysid);
snprintf(sysident_str, sizeof(sysident_str), UINT64_FORMAT,
ControlFile->system_identifier);

It's possible I've done the autoconf hackery wrong though. Should
UINT64_FORMAT still be defined if there's no int64?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> It's possible I've done the autoconf hackery wrong though. Should
> UINT64_FORMAT still be defined if there's no int64?

Yes.

regards, tom lane

Stefan Kaltenbrunner wrote:
> Zdenek Kotala wrote:
>> Stefan Kaltenbrunner wrote:
>>> Zdenek Kotala wrote:
>>>> For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
>>>> want to determine warning tags for each warning add -errtags.
>>> Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
>>> Studio 8,11) we have on the farm ?
>> Yes. Also on SS12.
>
> hmm - sure about that ? I was about to submit a patch to disable some
> compiler warnings but then I noted the following discussion thread:
>
> http://forum.java.sun.com/thread.jspa?threadID=5163903&messageID=9637391
>
> which seems to indicate that at least the compiler installed on kudu:
>
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kudu&dt=2007-07-15%2003:30:01
>
> does NOT support turning of specific warnings.
>

I tested it on cc version 5.3 and it works. See

-bash-3.00$ SUNWspro/SC6.2/bin/cc -V
cc: Sun WorkShop 6 update 2 C 5.3 Patch 111680-09 2003/05/18
usage: cc [ options] files. Use 'cc -flags' for details
-bash-3.00$ /ws/onnv-tools/SUNWspro/SC6.2/bin/cc pokus.c
"pokus.c", line 5: warning: statement not reached
-bash-3.00$ /ws/onnv-tools/SUNWspro/SC6.2/bin/cc -errtags pokus.c
"pokus.c", line 5: warning: statement not reached (E_STATEMENT_NOT_REACHED)
-bash-3.00$ /ws/onnv-tools/SUNWspro/SC6.2/bin/cc
-erroff=E_STATEMENT_NOT_REACHED pokus.c
-bash-3.00$

It works since Sun Workshop 4.2 (cc: WorkShop Compilers 4.2 26 Jun 1997
C 4.2 patch 105062-01). I tested it also on SunWorkshop 2.0.1 and it
does not work there, but I belive that nobody uses ten years old
compiler :-).

Please, can you send me a cc -V output ( I think It should be added in log).

Zdenek

Zdenek Kotala wrote:
> Stefan Kaltenbrunner wrote:
>> Zdenek Kotala wrote:
>>> Stefan Kaltenbrunner wrote:
>>>> Zdenek Kotala wrote:
>>>>> For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
>>>>> want to determine warning tags for each warning add -errtags.
>>>> Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
>>>> Studio 8,11) we have on the farm ?
>>> Yes. Also on SS12.
>>
>> hmm - sure about that ? I was about to submit a patch to disable some
>> compiler warnings but then I noted the following discussion thread:
>>
>> http://forum.java.sun.com/thread.jspa?threadID=5163903&messageID=9637391
>>
>> which seems to indicate that at least the compiler installed on kudu:
>>
>> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kudu&dt=2007-07-15%2003:30:01
>>
>>
>> does NOT support turning of specific warnings.
>>
>
> I tested it on cc version 5.3 and it works. See

ah cool - thanks for testing!

so on my box we would need to add
-erroff=E_EMPTY_TRANSLATION_UNIT,E_STATEMENT_NOT_REACHED,E_END_OF_LOOP_CODE_NOT_REACHED,E_FUNC_HAS_NO_RETURN_STMT,E_LOOP_NOT_ENTERED_AT_TOP

to CFLAGS to get down to the following 2 warnings:

"pgstat.c", line 652: warning: const object should have initializer:
all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
"pgstat.c", line 2118: warning: const object should have initializer:
all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)

the open question is if that is what want or if we would be simply
adding (unnecessary) complexity (or confusion).

comments ?

Stefan

"Stefan Kaltenbrunner" <stefan(at)kaltenbrunner(dot)cc> writes:

> "pgstat.c", line 652: warning: const object should have initializer:
> all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
> "pgstat.c", line 2118: warning: const object should have initializer:
> all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)

Man, even these are bogus. And that would be an interesting warning too if
they made it not fire when it's bogus. bleagh.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Stefan Kaltenbrunner napsal(a):
> Zdenek Kotala wrote:
>> Stefan Kaltenbrunner wrote:
>>> Zdenek Kotala wrote:
>>>> Stefan Kaltenbrunner wrote:
>>>>> Zdenek Kotala wrote:
>>>>>> For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
>>>>>> want to determine warning tags for each warning add -errtags.
>>>>> Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
>>>>> Studio 8,11) we have on the farm ?
>>>> Yes. Also on SS12.
>>> hmm - sure about that ? I was about to submit a patch to disable some
>>> compiler warnings but then I noted the following discussion thread:
>>>
>>> http://forum.java.sun.com/thread.jspa?threadID=5163903&messageID=9637391
>>>
>>> which seems to indicate that at least the compiler installed on kudu:
>>>
>>> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kudu&dt=2007-07-15%2003:30:01
>>>
>>>
>>> does NOT support turning of specific warnings.
>>>
>> I tested it on cc version 5.3 and it works. See
>
> ah cool - thanks for testing!
>
> so on my box we would need to add
> -erroff=E_EMPTY_TRANSLATION_UNIT,E_STATEMENT_NOT_REACHED,E_END_OF_LOOP_CODE_NOT_REACHED,E_FUNC_HAS_NO_RETURN_STMT,E_LOOP_NOT_ENTERED_AT_TOP
>
> to CFLAGS to get down to the following 2 warnings:
>
> "pgstat.c", line 652: warning: const object should have initializer:
> all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
> "pgstat.c", line 2118: warning: const object should have initializer:
> all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
>
> the open question is if that is what want or if we would be simply
> adding (unnecessary) complexity (or confusion).
>
> comments ?

E_STATEMENT_NOT_REACHED,E_END_OF_LOOP_CODE_NOT_REACHED, E_EMPTY_TRANSLATION_UNIT
are probably ok to ignore.
E_FUNC_HAS_NO_RETURN_STMT is there because main is leaved by exit() instead
return. And In another case It should be regular warning.

I think good solution is compare previous warning log with latest build and make
a diff. If some new warning appears it is probably regular warning.

Zdenek

Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> E_FUNC_HAS_NO_RETURN_STMT is there because main is leaved by exit() instead
> return. And In another case It should be regular warning.

That should be gone now; I changed the two places that triggered it.
I'd suggest not disabling that warning.

regards, tom lane

Tom Lane napsal(a):
> Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
>> E_FUNC_HAS_NO_RETURN_STMT is there because main is leaved by exit() instead
>> return. And In another case It should be regular warning.
>
> That should be gone now; I changed the two places that triggered it.
> I'd suggest not disabling that warning.

Yes I agree. Did you also clean up on old branches? If not I think we can live
with this warning on old branches.

Zdenek

Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> Tom Lane napsal(a):
>> That should be gone now; I changed the two places that triggered it.
>> I'd suggest not disabling that warning.

> Yes I agree. Did you also clean up on old branches?

No, I'm not interested in doing that kind of fiddling on old branches.
I think we only care about warnings in HEAD. (Unless an actual bug is
exposed, of course, in which case we'd back-patch the fix as appropriate.)

regards, tom lane