Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort

[Markus Armbruster]

David Gibson <address@hidden> writes:

> On Tue, Feb 02, 2016 at 10:47:35PM +0100, Thomas Huth wrote:
>> On 02.02.2016 19:53, Markus Armbruster wrote:
>> > Lluís Vilanova <address@hidden> writes:
>> ...
>> 
>> >> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
>> >> index 7ab2355..6c2f142 100644
>> >> --- a/include/qemu/error-report.h
>> >> +++ b/include/qemu/error-report.h
>> >> @@ -43,4 +43,23 @@ void error_report(const char *fmt, ...) 
>> >> GCC_FMT_ATTR(1, 2);
>> >>  const char *error_get_progname(void);
>> >>  extern bool enable_timestamp_msg;
>> >>  
>> >> +/* Report message and exit with error */
>> >> +void QEMU_NORETURN error_vreport_fatal(const char *fmt, va_list ap) 
>> >> GCC_FMT_ATTR(1, 0);
>> >> +void QEMU_NORETURN error_report_fatal(const char *fmt, ...) 
>> >> GCC_FMT_ATTR(1, 2);
>> > 
>> > This lets people write things like
>> > 
>> >     error_report_fatal("The sky is falling");
>> > 
>> > instead of
>> > 
>> >     error_report("The sky is falling");
>> >     exit(1);
>> > 
>> > or
>> > 
>> >     fprintf(stderr, "The sky is falling\n");
>> >     exit(1);
>> > 
>> > I don't think that's an improvement in clarity.
>> 
>> The problem is not the existing code, but that in a couple of new
>> patches, I've now already seen that people are trying to use
>> 
>>      error_setg(&error_fatal, ... );
>
> So, I don't actually see any real advantage to error_report_fatal(...)
> over error_setg(&error_fatal, ...).

I do.  Compare:

(a) error_report(...);
    exit(1);

(b) error_report_fatal(...);

(c) error_setg(&error_fatal, ...);

In my opinion, (a) is clearest: even a relatively clueless reader will
know what exit(1) does, can guess what error_report() approximately
does, and doesn't need to know what it does exactly.  (b) is slightly
less obvious, and (c) is positively opaque.

Let's stick to the obvious (a) and be done with it.

[...]