On Sun, Oct 28, 2007 at 01:51:34PM +0100, Gerald Pfeifer wrote: > In tools/widl/typegen.c we have the following snippet > > static void write_user_tfs(FILE *file, type_t *type, unsigned int *tafsoff) > { > unsigned int start, absoff, flags; > unsigned int align = 0, ualign = 0; > const char *name; > type_t *utype = get_user_type(type, &name); > > which passes name to get_user_type without initializing it. get_user_type > in return has the following piece of code: > > static type_t *get_user_type(const type_t *t, const char **pname) > { > for (;;) > { > type_t *ut = get_attrp(t->attrs, ATTR_WIREMARSHAL); > if (ut) > { > if (pname) > *pname = t->name; > return ut; > } > > It is unclear to this reader (and various versions of GCC I tested), > whether we don't really use *name (*pname) uninitialized here. Now > it is possible that through some of the, hmm, tricky logic in these > two functions combined with some environmental guarantees it happens > that we don't, but it seems better to be proactive here.
The logic is as follows: We wouldn't be in write_user_tfs if the predicate is_user_type wasn't true for that type, which means *pname (name) will always be initialized since it gets set precisely when get_user_type returns non-NULL (which is the is_user_type test). I don't think initializing it to NULL is helpful since it would crash just the same as if it weren't initialized. name *must* have a valid (non-NULL) value. This may silence a warning, but it's making the logic of the code more confusing by setting the variable to a value that doesn't make any sense in that context. get_user_type is the initialization. Better than this would be to put "assert(is_user_type(type));" above the initializations to convince the programmer at least that name will get initialized correctly in get_user_type. If that doesn't convince the compiler then I don't think it's worth it to try to placate it since we're supposed to be wrting code for humans, not computers. Some projects use a macro like "name IF_LINT(= NULL);" to quiet warnings while informing the reader that the initialization is not necessary. I suppose that's slightly better as well. I think the real problem is that the code is just not clear enough. I've been meaning to add asserts. Where asserts are impractical: comments.