abort in bfd_cache_lookup_worker
Nick Clifton
nickc@redhat.com
Wed Oct 26 10:08:00 GMT 2005
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.
More information about the Binutils
mailing list