Thread: execl() sentinel
Stefan Kaltenbrunner just let me know via Jabber that there's a warning in pg_regress.c: pg_regress.c: In function `spawn_process': pg_regress.c:914: warning: missing sentinel in function call This small patch would seem to fix it, according to http://www.linuxonly.nl/docs/sentinel/ -- Alvaro Herrera http://www.flickr.com/photos/alvherre/ "Los románticos son seres que mueren de deseos de vida"
Attachment
Alvaro Herrera wrote: > Stefan Kaltenbrunner just let me know via Jabber that there's a warning > in pg_regress.c: > > pg_regress.c: In function `spawn_process': > pg_regress.c:914: warning: missing sentinel in function call > > This small patch would seem to fix it, according to > http://www.linuxonly.nl/docs/sentinel/ You can apply this, but it sure seems like a compiler/include file bug to me, even with the 64-bit explaination. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Alvaro Herrera wrote:
>> pg_regress.c: In function `spawn_process':
>> pg_regress.c:914: warning: missing sentinel in function call
> You can apply this, but it sure seems like a compiler/include file bug
> to me, even with the 64-bit explaination.
There are lots of platforms where the include files and the compiler are
not all that compatible, gcc + vendor include files being the prototype
case. It's too bad that gcc doesn't have a
-Wno-snarkiness-about-system-headers-thank-you switch.
regards, tom lane
Tom Lane wrote: > It's too bad that gcc doesn't have a > -Wno-snarkiness-about-system-headers-thank-you switch. It does have a switch to *add* snarkiness about system headers, but does not do it by default. The problem in this case is that an uncast null pointer constant is not always a sufficient sentinel for variadic functions, as explained here: <http://c-faq.com/null/null2.html>. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes: > Tom Lane wrote: >> It's too bad that gcc doesn't have a >> -Wno-snarkiness-about-system-headers-thank-you switch. > It does have a switch to *add* snarkiness about system headers, but does > not do it by default. > The problem in this case is that an uncast null pointer constant is not > always a sufficient sentinel for variadic functions, as explained here: > <http://c-faq.com/null/null2.html>. Sure, but on a machine where it actually matters (ie one where int and pointer are of different sizes), I'd expect NULL to be #define'd as "((void *) 0)" not just "0". You should *not* have to inform the machine that NULL is a pointer. regards, tom lane
Am Mittwoch, 18. Juli 2007 16:16 schrieb Tom Lane: > You should *not* have to inform the machine that NULL is a pointer. For variadic functions, that expectation is invalid, AFAIK. It might be good to check the actual definition of NULL in this case, however, before wondering further. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: > Am Mittwoch, 18. Juli 2007 16:16 schrieb Tom Lane: > > You should *not* have to inform the machine that NULL is a pointer. > > For variadic functions, that expectation is invalid, AFAIK. No, what's invalid is that using an unadorned 0 is understood as a "null pointer" by the compiler. That would happen in a lot of places except on a variadic function. However, the platform may define NULL as it wishes, and indeed in our c.h it is defined (conditionally) as (void *)0. If the platform had such a definition then it would work without issues. I assume the platform in question does something like #define NULL 0 which would be silly. -- Alvaro Herrera http://www.advogato.org/person/alvherre "El conflicto es el camino real hacia la unión"
Am Mittwoch, 18. Juli 2007 17:16 schrieb Alvaro Herrera: > Peter Eisentraut wrote: > > Am Mittwoch, 18. Juli 2007 16:16 schrieb Tom Lane: > > > You should *not* have to inform the machine that NULL is a pointer. > > > > For variadic functions, that expectation is invalid, AFAIK. > > No, what's invalid is that using an unadorned 0 is understood as a "null > pointer" by the compiler. That would happen in a lot of places except > on a variadic function. > > However, the platform may define NULL as it wishes, and indeed in our > c.h it is defined (conditionally) as (void *)0. If the platform had > such a definition then it would work without issues. > > I assume the platform in question does something like > #define NULL 0 > which would be silly. I suggest that you read through <http://c-faq.com/null/>, which is at odds with your statements. -- Peter Eisentraut http://developer.postgresql.org/~petere/
On Wed, 2007-07-18 at 16:59 +0200, Peter Eisentraut wrote: > It might be good to check the actual definition of NULL in this case, however, > before wondering further. Well, the existing coding is plainly wrong, regardless of the NULL implementation used on any given machine (although it will usually work). The simple rule is "you need to cast NULL to a pointer type when passing arguments to a variadic function, or to a function whose prototype is not in scope". So +1 on this patch from me. -Neil
Neil Conway wrote: > On Wed, 2007-07-18 at 16:59 +0200, Peter Eisentraut wrote: > > It might be good to check the actual definition of NULL in this case, however, > > before wondering further. > > Well, the existing coding is plainly wrong, regardless of the NULL > implementation used on any given machine (although it will usually > work). The simple rule is "you need to cast NULL to a pointer type when > passing arguments to a variadic function, or to a function whose > prototype is not in scope". > > So +1 on this patch from me. Thanks, committed. I looked for other uses of execl(), execle() and execlp() and found a single one of execl() which is already OK. I wouldn't know how to look for other variadic functions using NULL sentinels though. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
On Wed, 2007-07-18 at 17:22 -0400, Alvaro Herrera wrote: > I wouldn't know how to look for other variadic functions using NULL > sentinels though. You would need something with more knowledge of C than "grep" has, at any rate. Perhaps you could teach sparse to do this analysis, if it can't do it already... -Neil
neilc@samurai.com (Neil Conway) writes: > On Wed, 2007-07-18 at 17:22 -0400, Alvaro Herrera wrote: >> I wouldn't know how to look for other variadic functions using NULL >> sentinels though. > > You would need something with more knowledge of C than "grep" has, at > any rate. Perhaps you could teach sparse to do this analysis, if it > can't do it already... sgrep might be smart enough... I quite like sgrep... -- "cbbrowne","@","acm.org" http://www3.sympatico.ca/cbbrowne/textsearch.html "Since a cat always lands on its feet, and a piece of buttered toast always lands buttered side down, if you strap a piece of buttered toast to the back of a cat, which side will it land on?" -- .sig file on rec.humor
Chris Browne wrote: > neilc@samurai.com (Neil Conway) writes: > > On Wed, 2007-07-18 at 17:22 -0400, Alvaro Herrera wrote: > >> I wouldn't know how to look for other variadic functions using NULL > >> sentinels though. > > > > You would need something with more knowledge of C than "grep" has, at > > any rate. Perhaps you could teach sparse to do this analysis, if it > > can't do it already... > > sgrep might be smart enough... I quite like sgrep... Interesting tool. However as for tools, I use cscope very succesfully for this kind of thing. But what we would need in this case is a list of variadic functions. Or were you thinking in getting a list of called functions in Postgres, and some way of figuring out which of these were variadic? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.