execl() sentinel

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&gt;.

--
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&gt;.

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/&gt;, 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.