Bastien Nocera <[email protected]> writes: > On Sat, 2010-06-26 at 08:19 -0400, Greg Troxel wrote: >> Bastien Nocera <[email protected]> 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. >From 7f210869e0af4610f5364baa9d4a3396818d1841 Mon Sep 17 00:00:00 2001 From: Greg Troxel <[email protected]> Date: Wed, 30 Jun 2010 08:12:23 -0400 Subject: [PATCH] Initialize variable for safety and to avoid warnings. In gconf_value_to_value, initialize gvalue to NULL, because glib documentation says not to rely on runtime warning checks and because compilers might not infer that gvalue is always initialized on return. If glib checks are enabled, this changed will have no runtime effect because the checks already return NULL. --- src/main.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/src/main.c b/src/main.c index adbe8a8..ffd4ac5 100644 --- a/src/main.c +++ b/src/main.c @@ -53,6 +53,19 @@ gconf_value_to_value (GConfValue *value) g_return_val_if_fail (value->type == GCONF_VALUE_STRING || value->type == GCONF_VALUE_INT, NULL); + /* Initialize gvalue for two reasons: + * 1. Avoid compiler warnings about use of uninitialized + * values for compilers that do not infer from the default + * expansion of the above macro that type is STRING or INT. + * 2. Protect against the case that the above warnings are + * disabled, causing the function to return NULL instead of + * an undefined value. + * (Note that compilers that can infer that gvalue would never + * be used uninitialized can be expected to optimize out this + * statement.) + */ + gvalue = NULL; + if (value->type == GCONF_VALUE_STRING) { const char *str; -- 1.7.0.5 _______________________________________________ GeoClue mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/geoclue
