Peter N. Lundblad wrote:
> On Sun, 20 Nov 2005, Erik Huelsmann wrote:
>>I get a gcc warning about base_props being used uninitialized in
Erik, if you "make clean" and compile the whole of Subversion, how many of that
type of warning do you get? Which gcc?
> With gcc 4.0.2 I gets a lot of such warnings all over the code. The topic
> of shutting up warnings has been up for discussion before and some people
> have objected because just setting a value to NULL (or some other bogus
> value) might just hide real bugs. Said more as a general comment to
> current threads. The below doesn't necessarily do that. Problem is that
> different compilers seem to produce this warning in different situations.
> In this case, the warning is bogus, AFAICT.
> > I propose this patch to fix it:
>>--- subversion/libsvn_wc/props.c (revision 17455)
>>+++ subversion/libsvn_wc/props.c (working copy)
>>@@ -387,11 +387,14 @@
>> if (base_props_p)
>> *base_props_p = base_props;
>>+ base_props = NULL;
>> if (props_p)
>> if (has_propcaching && ! entry->prop_mods && entry->has_props)
>>- *props_p = apr_hash_copy (pool, base_props);
>>+ *props_p = base_props
>>+ ? apr_hash_copy (pool, base_props) : apr_hash_make (pool);
> If I'm not missing something (which might well be true, since I wrote this
> code), base_props can never be NULL here. I think it is confusing to add
> code that will never be executed. (An assert is better if that's desired.)
Yes, it's definitely bad to add that second part of the patch if you know
"base_props" is never null there. (Yes, "assert" is better, but hardly
The first part, making sure to initialise the variable (which could be done
either as shown or, less obtrusively, at its declaration) is a tough call. My
current feeling is that if there are lots of these cases as you say, then it's
probably not worth adding any code to shut up the warning: the ugliness would
outweigh the benefit. Disable the warning instead; it's one of those warnings
that is only useful for certain styles of coding.
 "assert" is hardly worthwhile because the pointer is soon dereferenced here
anyway. The potential benefit of "assert" is that it's like documentation: the
programmer declares that the pointer is intended to be non-null. However,
dereferencing a pointer is really just as good a declaration of intent.
There's little reason for a reader to believe that the "assert" is any more
correct and up-to-date than the subsequent code that uses the pointer.
To unsubscribe, e-mail: firstname.lastname@example.org
For additional commands, e-mail: email@example.com
Received on Mon Nov 21 01:56:40 2005