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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Sun, 06 Mar 2011 19:46:23 +0100, Michael Snyder wrote:
> 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.

I agree from the user point of view it may be (slightly) better (*).

But from the GDB developers point of view this change without any specific
code comment is not acceptable (as explained in my mail) as it confuses the
code readers.

I would find perfect:

  /* FIXME: FH may be NULL here but the code below cannot handle it.  */
  gdb_assert (fh);


Thanks,
Jan


(*) In fact I like the crash from the Fedora point of view better as ABRT
    (Automatic Bug Reporting Tool) will automatically report it.  In the case
    of a failed gdb_assert the user may or may not allow the core dump and
    therefore the crash bugreport by ABRT may be suppressed.
	A problem internal to GDB has been detected,
	further debugging may prove unreliable.
	Create a core file of GDB? (y or n) n
    This is just FYI and it should not be related to FSF GDB maintenance.