This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]