Re: [Qemu-devel] [PATCH v2 6/7] error: Revamp interface documentation

[Markus Armbruster]

"Dr. David Alan Gilbert" <address@hidden> writes:

> * Markus Armbruster (address@hidden) wrote:
>> "Dr. David Alan Gilbert" <address@hidden> writes:
>> 
>> > * Markus Armbruster (address@hidden) wrote:
>> >> Signed-off-by: Markus Armbruster <address@hidden>
>> >> Reviewed-by: Eric Blake <address@hidden>
>> >> ---
>> >>  include/qapi/error.h | 177 
>> >> ++++++++++++++++++++++++++++++++++++---------------
>> >>  1 file changed, 127 insertions(+), 50 deletions(-)
>> >> 
>> >> diff --git a/include/qapi/error.h b/include/qapi/error.h
>> >> index 8c3a7dd..7d808e8 100644
>> >> --- a/include/qapi/error.h
>> >> +++ b/include/qapi/error.h
>> >> @@ -2,13 +2,75 @@
>> >>   * QEMU Error Objects
>> >>   *
>> >>   * Copyright IBM, Corp. 2011
>> >> + * Copyright (C) 2011-2015 Red Hat, Inc.
>> >>   *
>> >>   * Authors:
>> >>   *  Anthony Liguori   <address@hidden>
>> >> + *  Markus Armbruster <address@hidden>
>> >>   *
>> >>   * This work is licensed under the terms of the GNU LGPL, version 2.  See
>> >>   * the COPYING.LIB file in the top-level directory.
>> >>   */
>> >> +
>> >> +/*
>> >> + * Error reporting system loosely patterned after Glib's GError.
>> >
>> > Excellent; it's great to have this documented.
>> 
>> Thanks!
>> 
>> > Do we have a note anywhere that says why we don't just use GError?
>> 
>> I'm not aware of such a note, and I don't know / remember why we didn't
>> use GError.  Perhaps one of the guys signing off the initial commit
>> does (cc'ing them):
>> 
>>     commit d5ec4f27c387c3b3200abb8656343b2519ea3047
>>     Author: Luiz Capitulino <address@hidden>
>>     Date:   Wed Jun 1 12:14:49 2011 -0500
>> 
>>         Introduce the new error framework
>> 
>>         New error-handling framework that allows for exception-like error
>>         propagation.
>> 
>>         Signed-off-by: Luiz Capitulino <address@hidden>
>>         Signed-off-by: Michael Roth <address@hidden>
>>         Signed-off-by: Anthony Liguori <address@hidden>
>> 
>> Here's a possible clue:
>> 
>>         GLib is an extremely common library that has a portable thread 
>> implementatiocommit e18df14185e817ba735bce57ecdef9a55fb3d093
>>     Author: Anthony Liguori <address@hidden>
>>     Date:   Tue Jul 19 14:50:29 2011 -0500
>> 
>>         Add hard build dependency on glib
>> [...]
>>         Signed-off-by: Anthony Liguori <address@hidden>
>>         Signed-off-by: Michael Roth <address@hidden>
>>         Signed-off-by: Luiz Capitulino <address@hidden>
>> 
>> Error predates our use of GLib.
>
> By a month!

"Knapp vorbei ist auch daneben."

> Are there semantics close enough that we could typedef Error to GError and 
> then
> slowly replace the use of our functions with standard glib calls?
>
> It would just end up as one chunk less of code to maintain, and
> something that new people wouldn't have to learn if they already new glib.

There are obvious differences:

* GError uses (GQuark domain, gint code), we use ErrorClass (and regret
  it).

* GError lacks &error_abort.

* In g_propagate_error(&dest, src), dest must be null.
  error_propagate() throws away src then.

* More subtle differences may well exist.

Can't say offhand whether the obvious differences plus the ones I might
be missing make replacement impractical.

Can say that my appetite for tree-wide cleanups has limits %-}

error.[ch] weigh in at 167 SLOC.  Includes convenience function we'd
have to carry forward to GError.