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

[David Gibson]

On Wed, Feb 03, 2016 at 10:48:53AM +0100, Markus Armbruster wrote:
> 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.

I'm fine with that, but I still see no real difference between (b) and
(c).

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature