[PATCH] Fixes sentinel warnings on GCC4

[Chris Shoemaker]

On Mon, Dec 05, 2005 at 05:26:34PM -0500, Derek Atkins wrote:
> Quoting Chris Shoemaker <c.shoemaker at cox.net>:
> 
> >On Mon, Dec 05, 2005 at 04:38:40PM -0500, Derek Atkins wrote:
> >>If you were on IRC you'd have more information on this.
> >>
> >>Yes, it is 64-bit only.  The problem is that on some versions of gcc4
> >>stddef.h #defined NULL as (int)0 which causes a compiler warning when
> >>passing that into a char*.
> >
> >I'm no expert, and I don't actually know what gcc #defines NULL to,
> >but I don't think this is quite right.  I don't think this is a gcc
> >problem.  No compiler can infer the correct type for variadic
> >arguments.  In a variadic argument list, a compiler *must* interpret
> >an unqualified "0" (which is probably what NULL is) as integer type.
> >Our code is just wrong for any machine where the null pointer is not a
> >32 bit zero, even if it's some (imaginary) 32-bit machine.
> 
> The problem is that you cannot pass a 32-bit integer into a 64-bit adddress.
> On my system (32-bit, gcc-3.4.3) NULL is either defined as 0, (void*)0, or
> __null depending on other #ifdefs (which I didn't go examine).  However
> if the callee is expecting a 64-bit pointer and you pass it a 32-bit
> integer, the compiler WILL (and SHOULD) complain.  With -Werror this
> causes the build to blow out.

This is good.

> 
> >>The fix (which David is working on -- which you'd know if you were in
> >>IRC) is to #define GNC_NULL ((gpointer)0) and then use that instead of
> >>NULL in these places.
> >
> >Hmm.  Seems a bit heavy-handed to me, but it would work.  I hope it
> >doesn't result in gratuitous use of GNC_NULL in the 99% of places
> >where NULL is fine.
> 
> I think it would only be used in the particular cases where this is an
> issue.  HOWEVER....
> 
> >IMHO, it's better that programmers remain aware that the use of NULL
> >as a sentinel is "special".  IOW, *not* hiding this behavior gives a
> >better picture of what the code actually does, which tends to make for
> >better code and better programmers in the long run.  One problem I see
> >with GNC_NULL, is visibility.  If it's too visible, people use it
> >gratuitously; if it's not visible enough, people forget to use it when
> >they should; How to choose?  Better just to /understand/ what's going
> >on.
> 
> ... we're still trying to analyze the issue and figure out what's really
> going on.  I.e., there's no plan to make any changes until we understand
> the problem.  David just went and tried to reproduce this on an amd64
> machine and could not reproduce it.  Then we hear back that these errors
> are on a 32-bit machine..  (all this is on IRC, and all in the last
> few minutes)..   So..  We're still trying to figure out what the REAL
> problem is before we consider this a gnucash  bug.
> 
> I agree with you that this SHOULDN'T be a problem, and we SHOULDN'T
> need to make this change.

You misunderstand me.  This *is* a problem, and we *should* fix it.
Our code is just *wrong*.  It's wrong on 32-bit machines and on 64-bit
machines.  (Although it happens to be a benign bug anywhere ((char *)
NULL) is a 32-bit zero, which is almost everywhere.)  It's just wrong
C.  '0' is only equivalent to the typed null pointer in a pointer
context (that's even true when a typed null pointer is not a 32-bit
null; it's part of the language definition of a null pointer.)  An
argument list to a variadic function is no different than a function
with no prototype - the arguments are NOT in pointer context.

The C-language rule is "always cast NULL when used as a sentinel to a
variadic function, otherwise you're accidentally passing integer
zero, which is wrong."

-chris