Re: [commit] Properly cast sentinels for concat()

[Daniel Jacobowitz <drow@false.org>]

On Tue, Jul 05, 2005 at 01:08:22AM +0200, Eli Zaretskii wrote:
> > Date: Mon, 4 Jul 2005 15:36:05 +0200
> > From: Mark Kettenis <kettenis@jive.nl>
> > 
> > This fixes a few warnings with GCC 4.0 on OpenBSD.  You'll probably
> > won't see them on other systems, since they only show up if NULL is
> > defined as an integer instead of a pointer constant (both are valid
> > according to C standard).  The stddef.h that comes with GCC defines
> > NULL as (void *)NULL, but we don't use that one on OpenBSD.
> > 
> > Anyway, I committed the attached patch as obvious.
> 
> Actually, it's not at all obvious, it's IMNSHO simply wrong.  Casting
> NULL to _anything_ should never be needed, unless NULL is abused
> (i.e. used in a place where a pointer cannot be).  Let's not decide
> that a patch is ``obvious'' just because it happens to shut up the
> compiler!

You must cast NULL to a pointer type if you use it as the sentinel in a
varargs list, in portable code, because 0 is a legal definition of NULL
(according to the C standard, which is quite clear on the subject).  On
an I32 LP64 system, passing 0 to a varargs function may take 32 bits on
the stack while passing NULL takes 64 bits.  If you pass the wrong one,
you'll wander up the stack to nowhere.

This is often called "the execl bug", because execl is the most
frequently used function in the standard library which takes NULL as a
terminator for a list of string constants.

Even more C++ implementations use 0, which makes the problem even
worse.  ISTR that (void *)0 is not an acceptable definition of NULL in
C++.

-- 
Daniel Jacobowitz
CodeSourcery, LLC