Re: [RFA] mdebugread.c (psymtab_to_symtab_1): assert that 'fh' is not null

[Michael Snyder <msnyder@vmware.com>]

Jan Kratochvil wrote:
> On Fri, 04 Mar 2011 23:15:39 +0100, Michael Snyder wrote:
>> This occurs along a path where fh *could* be null, and then we
>> dereference it.
> 
> I agree there is a bug.
> 
> 
>> +      gdb_assert (fh);
> [...]
>>  	parse_external (ext_ptr, fh->fBigendian,
>>  			pst->section_offsets, pst->objfile);
> 
> But I find as even a worse bug to introduce such an assertion without any
> comment there.
> 
> I at least myself perceive any assertions that the programmer thinks such
> invariant is valid there.  That invariant says that if its negation happens
> then the programmer was wrong.
> 
> If there is a known bug you have found and you just do not intend to fix it
> now I find introducing such an assertion as misleading.  The reader then
> assumes such invariant and can do additional wrong conclusions from it.
> 
> In such case there should be primarily a "FIXME" comment, best even to
> reference a filed a Bug.  There can be also an assertion there but the "FIXME"
> I find essential there.


All these recent submissions of mine are coming from running Coverity
on gdb.  In this case, Coverity reports that we checked the variable fh
for null (suggesting that we think null is possible), but then 
dereferenced it on an unchecked path.

I figured that triggering an assert was more gracefull than just
crashing on a null pointer dereference, and gives the user the chance
to recover.