Re: [PATCH 01/15 v3] Introduce common/errors.h

[Doug Evans <dje@google.com>]

On Fri, Jul 18, 2014 at 11:44 AM, Gary Benson <gbenson@redhat.com> wrote:
> Doug Evans wrote:
>> On Fri, Jul 18, 2014 at 10:06 AM, Gary Benson <gbenson@redhat.com> wrote:
>> > [...]
>> > /* Throw a fatal error, constructing the message using a printf-style
>> >    format string and a printf- or vprintf-style argument list.  This
>> >    function does not return.  The application will exit.  */
>> >
>> > extern void fatal (const char *fmt, ...)
>> >      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>> >
>> > extern void vfatal (const char *fmt, va_list args)
>> >      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
>> > [...]
>>
>> Remember gdb doesn't exit on fatal().
>> fatal() in gdb is essentially ^c (quit() calls fatal()!).
>>
>> Can I repeat my request to please document this in the function
>> comment.
>
> I wanted to avoid putting implementation details here,

I realize that.

> but I see this
> isn't going to happen.  Is the attached ok?

I'm trying to not impose on you too much churn and back-and-forth in
what is clearly a stepping stone.
Eventually I think fatal needs to disappear from the gdb side (renamed
or whatever).
Until that happens (i.e., *if* you just want to keep the patch
basically as is), then at the least I don't want to lie to the reader,
and I want to make the reader aware of the issue.  That's the high
order bit for me as far as "fatal" goes (within the context of trying
to keep the patch basically as is).

If you want to take on the task of getting this 100% correct in this
pass (or at least within some fraction thereof :-)), then we can take
a different route.  Your choice IMO.  [And I mean that sincerely - I
*am* trying to help you make progress here without being too
pedantic.]

> /* Throw a fatal error, constructing the message using a printf-style
>    format string and a printf- or vprintf-style argument list.  This
>    function does not return.  Fatal errors cause GDB to return to the
>    command level.  Fatal errors cause gdbserver to exit.  */
>
> extern void fatal (const char *fmt, ...)
>      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>
> extern void vfatal (const char *fmt, va_list args)
>      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);

I would tweak that a little, and instead just point out that gdb's
usage of the name "fatal" is broken.  I don't care too much about
the wording, I just want to make sure we don't lie to the reader
and make the reader aware of the issue.

/* Throw a fatal error, constructing the message using a printf-style
   format string and a printf- or vprintf-style argument list.  This
   function does not return.  Fatal errors cause the app to exit,
   except in the case of gdb where it just throws a RETURN_QUIT
   exception.  */