Re: abort in bfd_cache_lookup_worker

[Nick Clifton <nickc at redhat dot com>]

Hi Alan,

>   In http://sources.redhat.com/bugzilla/show_bug.cgi?id=136, you fixed a
> problem with bfd_cache_lookup returning NULL and resulting segfaults, by
> aborting when bfd_cache_lookup_worker couldn't reopen a file for some
> reason.  Having looked at this today, I'm inclined to think that an
> abort is inappropriate, because a bfd_cache_lookup_worker failure could
> easily be due to user actions such as deleting the file, or the
> application using BFD truncating the file or opening too many files.
> "BFD: internal error" and "BFD: Please report this bug" are likely to
> mislead people into thinking that the problem is in BFD, when it's more
> likely elsewhere.
>
> So, how about we remove the abort but continue to squark about the error?

Hmm, maybe what we should do is not abort, but issue a descriptive error 
message ("file <foo> can no longer be found") and then exit ?  That way 
we do not have to check all potential users of bfd_cache_lookup.

I have no real objection to bfd_cache_lookup() returning NULL however. 
I just chose to avoid that with my original change as it made the patch 
simpler and I like to keep things simple.  Adding checks for NULL 
returns bloats the code (a little bit) and leaves open the possibility 
of seg-fault if we forget to check somewhere.  On the other hand, 
paranoia (in programming) is good and every function ought to check the 
return value of any sibling it calls, especially if they return a pointer.

So yes, if you do not like the idea of bfd_lookup_cache() calling exit, 
please do apply your patch.

Cheers
  Nick

> 	* cache.c (bfd_cache_lookup_worker): Don't abort on failing to
> 	reopen file.
> 	(cache_btell, cache_bseek, cache_bflush, cache_bstat): Return -1 on
> 	bfd_cache_lookup failure.
> 	(cache_bread, cache_bwrite): Return 0 on the same.
> 	* bfdwin.c (bfd_get_file_window): Likewise.
> 	* hppabsd-core.c (hppabsd_core_core_file_p): Likewise.
> 	* sco5-core.c (sco5_core_file_p): Likewise.
> 	* trad-core.c (trad_unix_core_file_p): Likewise.