Re: [patch/ob]

[Andrew Cagney <ac131313@cygnus.com>]

> On Tue, Dec 11, 2001 at 10:18:05AM -0800, Andrew Cagney wrote:
> 
>> Just FYI,
>> 
>> I've checked the attached in as, er, obvious.  It fixes a
>> -Wuninitialized warning.
> 
> 
> I agree with the "er, obvious".
> 
> Would there be anything wrong with:
> 
> 
>> *************** value_fn_field (value_ptr *arg1p, struct
>> *** 971,983 ****
>> struct minimal_symbol *msym;
> 
> 
> adding = NULL to the line above?


6 of one, half a dozen of the other.

My personal preference is to do an initialization as:

	if (expr)
	  foo = val1;
	  bar = valb;
	  baz = valz;
	else if (expr2)
	  foo = val2;
	  bar = valxx;
	  bas = val3;
	else
	  foo = defaultval;
	  bar = devbar;
	  baz = baxdev;

rather than:

	bar = devbar;
	baz = baxdev;
	...
	...
	...
	...
	...
	...
	if (expr)
	  foo = val1;
	  bar = valb;
	else if (expr2)
	  foo = val2;
	  baz = val3;

This is because I consider the former to have clear pre/post assertions 
and exploits the compiler's -Wuninitialized facility - you know all 
branches have initialized foo, bar and baz (I'm not sure about the 
latter example mind :-).  This becomes especially useful when handling 
initialization in in very long switches and if/elif chains.

> +       gdb_assert (sym == NULL);
> 
> 
> This assert in particular bugs me.  Adding asserts that the compiler
> can obviously eliminate, since sym isn't volatile...


Here, sorry, I'm lost.  Assertions are added to code for many reasons - 
one being that it helps ``prove'' correctness, another is that it can 
clarify the intention and assumptions of the developer.  I don't think 
compiler has much to do with this.

Keep in mind that we use GCC's -Werror messages as a tool.  Not as an 
end in themselves.

enjoy,
Andrew