Bastien Nocera <[email protected]> writes: > On Sat, 2010-06-26 at 06:20 -0400, Greg Troxel wrote: >> Ross Burton <[email protected]> writes: >> >> > On Fri, 2010-06-25 at 20:27 -0400, Greg Troxel wrote: >> >> I got a complaint about gvalue being used undefined, and sure enough the >> >> code looks like that's possible at first glance. The inference that one >> >> or the other if branch will be taken because of g_return_val_if_fail >> >> returning from the function is apparently too subtle for >> >> >> >> Using built-in specs. >> >> Target: i386--netbsdelf >> >> Configured with: /usr/src/tools/gcc/../../gnu/dist/gcc4/configure >> >> --enable-long-long --disable-multilib --enable-threads --disable-symvers >> >> --build=x86_64-unknown-netbsd4.99.72 --host=i386--netbsdelf >> >> --target=i386--netbsdelf --enable-__cxa_atexit >> >> Thread model: posix >> >> gcc version 4.1.3 20080704 prerelease (NetBSD nb2 20081120) >> >> >> >> Here's a diff that points out the issues and makes it build for me. >> > >> > I'd actually prefer initialising value to NULL so that if you pass an >> > invalid gconfvalue without warnings on you get a NULL pointer instead of >> > a uninitialized gvalue. >> >> Sure - what I tried to say is: >> >> I don't understand geoclue's code enough to know the right fix. My >> diff merely points out where a compiler doesn't figure out that the >> value can't ever be uninitialized. It would be cool if someone could >> fix this correctly. > > 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.
pgp025R8O0VWR.pgp
Description: PGP signature
_______________________________________________ GeoClue mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/geoclue
