Re: [PATCH 1/3 v4] Introduce common/errors.h

[Gary Benson <gbenson at redhat dot com>]

Doug Evans wrote:
> ...it's odd to then not have just one copy of perror_with_name and
> malloc_failure given that their function comment says:
> 
> perror_with_name:
> /* A special case of "error" which constructs the error message by ...
> 
> and
> 
> malloc_failure:
> /* A special case of internal error used to handle memory allocation ...
> 
> When I read "special case of ..." the first thing that comes to mind
> is that the function is a utility wrapper around the corresponding
> function, and given that it's not I'm left wondering "Why not?".
[snip]
> So, *if* you want to keep *this* part of the patch basically as-is
> (which is fine by me) I would rephrase this comment to something
> like:
> 
> /* Call this function to handle memory allocation failures.
>    This function does not return.   */
> 
> I don't have too strong opinion on the wording.  If someone wants a
> different wording, go for it.

I pretty much agree with you 100% here.  I wasn't happy with the
comments for either of these, and I'll gladly change them.

> In the case of malloc_failure, it's not really an *internal* error,

No--and certainly not by the description of internal_error earlier in
the file.

> even if in the case of gdb it calls internal_error, though arguably
> it should do something different - it's more of a "fatal". :-)
> Though I'm not suggesting trying to go down that path.  :-)

I spoke with Pedro about this in Cambridge.  malloc failure is a funny
one: sometimes it's fatal, other times it's not an issue; it depends
entirely on what the memory you were trying to allocate was for.

> It might be possible come up with a name other than "fatal" that
> could apply to both gdb and gdbserver so that malloc_failure could
> call it, but no need to try to do that now.

I don't know if you saw but I removed "fatal" from GDB the other day
(http://tinyurl.com/k6neuwd) so the path is clear to add a "fatal" if
we want one.  It would be nice if such a function were smart enough to
work even if called before exceptions and cleanups were set up.  It
might be a nice general thing if all error-handling functions worked
that way (error and internal_error could work like gdbserver's "fatal"
if called early.)  That might even be a prerequisite to moving
exceptions and cleanups into common code and making gdbserver use
them.

(I'm not planning to do any of this now, this is just thinking aloud!)

Cheers,
Gary

-- 
http://gbenson.net/