build failure on netbsd-5 with gtk2+-2.20.1nb

[Greg Troxel]

Bastien Nocera <hadess at hadess.net> writes:

> On Sat, 2010-06-26 at 08:19 -0400, Greg Troxel wrote: 
>> Bastien Nocera <hadess at hadess.net> writes:
> <snip> 
>> > Did you compile glib (or did you get a glib from your distro) without
>> > warnings enabled?
>> 
>> glib2 is from pkgsrc, and I am pretty sure it doesn't disable warnings.
>> 
>> I read gmessage.h and I see what you mean.  However, it's reasonable for
>> a compiler to err on the safe side and complain about a variable which
>> is not 100% clearly only used when initialized.
>> 
>> Plus, unless geoclue checks and fails to build if warnings are off, it
>> needs to check properly. It seems g_return_val_if_fail is meant to
>> support eiffel-style design-by-contract, and to document the rules
>> (always) and enforce them (warnings on, which seems default).
>> 
>> It seems easy enough to set the offending variable to NULL to avoid
>> this.
>
> Except that's working around a compiler bug. You're more than welcome
> patching this for your distribution of the package, but I don't see why
> we should put work-arounds that might hide bugs later in geoclue. 

A simple assignment to NULL will not hide bugs; it just causes the
routine to return NULL instead of undefined if for some reason the
g_return_val_if_fail invocation doesn't do what is expected.

It's not really a compiler bug; it's a failure to do enough static
analysis to show that something that is on its face unsafe is actually
ok.

(If you're really concerned about the future and bugs, it would be good
to have comments documenting the preconditions and postconditions of
procedures.  And perhaps abort() rather than continuing when they are
violated.)

Patch attached, or just set it to NULL on declaration without a comment
(which is what I would do).  I don't see how this can cause problems for
anyone.