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

["Dave Korn" <dk at artimi dot com>]

> -----Original Message-----
> From: Nick Clifton 
> Sent: 21 April 2004 11:58

> Well we can argue this back and forth all day. :-)  

  Heh, careful, or I *will* take that as an invitation!  Of course, pretty
much all the suggestions are pretty much reasonable in pretty much most
respects, but hell, I'm happy to argue obscure and trivial points endlessly
JFTHOIT..... <g>

> Perhaps the best compromise would be for bfd_archive_filename() to 
> display its own error message (using _bfd_error_handler) when it is 
> given a NULL input, but then to continue on and return NULL.  That way 
> the error handler will (presumably) stop the output from being 
> generated, but bfd_archive_filename() will still execute so that the 
> calling function will be able to display its error/warning message, 
> which will further help to locate the problem that is plaguing the
library.

  Your compromise design strikes me as a good one, but I'm still not sure
that the motivation is clearly thought through.

  As you say, letting b_a_f return NULL would generally let the calling
function continue far enough to display the error/warning, thereby helping
to locate which particular caller of b_a_f is at fault.  However, I don't
recall ever seeing anything that specifies that it's valid to pass a NULL
pointer to printf for a %s argument.  Fair enough, glibc (and indeed every
other libc I've ever seen) tests for this and handles it (because it's such
a common error), but I don't think that we could actually guarantee that it
wouldn't count as undefined behaviour on some systems.

  As to the pointfulness of doing so, ISTM unlikely that the knowledge (of
which function was calling b_a_f) on its own would ever be enough for
someone to fix a bug, so the usefulness of supplying that knowledge on its
own (by letting b_a_f return NULL) seems dubious to me.  I suppose that some
bugs might be so immediately obvious from the source code of the calling
function that just eyeballing it is enough to fix them, but I rather expect
that >95% of bugs will require someone to get out the debugger and have it
breakpoint at the failure, and at that point you get to see not just what
the calling function is but also all its internal variables, arguments it
was called with, etc etc.

  So it's supplying information that, while valid, is unlikely to ever be of
any actual help, at the cost of perhaps-sometimes-maybe invoking undefined
behaviour on some systems.

  Anyway, that's my take on it.  2c and all that.  But it's all rather
academic, and I don't think it matters a _great_ deal which solution is
applied in the end.  I'm happy with the abort, but basically the only *hard*
requirements I'd put on the solution myself are that it mustn't silently
generate invalid output files, and it should make it clear that the error is
an internal coding error requiring debugging, rather than one caused by (eg)
bad inputs.


    cheers,
       DaveK
-- 
Can't think of a witty .sigline today....