Re: PATCH: Return NULL on NULL bfd (Problem with linker with binutils-040414)

[Ian Lance Taylor <ian at wasabisystems dot com>]

"Dave Korn" <dk@artimi.com> writes:

>   Surely if it is done like this, what will happen is that the segv will be
> deferred to some point in the code where the filename is actually used,
> possibly much further on in the execution flow, making it more difficult to
> locate the site of the underlying error that has resulted in a NULL bfd
> pointer being passed around in the first place?

That is reasonable.  However, other alternatives are also reasonable.
Different people want different choices.  For example, some software
must never crash.  Those people would will be writing their code with
error checks.  If a NULL pointer slips in somehow, they would prefer
to get an error indication than a crash.

Given that, I think libraries should remain flexible.  Crashing is not
flexible.  Returning an error indication is flexible--it permits the
caller to decide what to do.  For example:
    n = bfd_archive_filename (abfd);
    assert (n != NULL);

Note also that your example above presumes that nobody checked the
return value of the function for an error indication.  That is sloppy.
Some functions can never fail.  Clearly bfd_archive_filename is not
one of them.  (At least, it is clear to me, though the documentation
does not mention the possibility of failure).

I actually see that bfd_archive_filename hides a bfd_malloc failure by
returning an incorrect value.  I also see that it unpredictably causes
a space leak.  I think this function needs some more attention in any
case.  If the comment is true that it is only used when building error
messages, then none of this matters very much.

Ian