[Qemu-commits] [qemu/qemu] 552375: error: De-duplicate code creating Error objects

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 552375088a832fd5945ede92d01f98977b4eca13
      
https://github.com/qemu/qemu/commit/552375088a832fd5945ede92d01f98977b4eca13
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M util/error.c

  Log Message:
  -----------
  error: De-duplicate code creating Error objects

Duplicated when commit 680d16d added error_set_errno(), and again when
commit 20840d4 added error_set_win32().

Make the original copy in error_set() reusable by factoring out
error_setv(), then rewrite error_set_errno() and error_set_win32() on
top of it.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: a9499ddd82a99c66cc72a08e72427c423acfea1c
      
https://github.com/qemu/qemu/commit/a9499ddd82a99c66cc72a08e72427c423acfea1c
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M include/qapi/error.h
    M util/error.c

  Log Message:
  -----------
  error: Make error_setg() a function

Saves a tiny amount of code at every call site.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: e7cf59e84767e30b507b6bd7c1347072ec12b636
      
https://github.com/qemu/qemu/commit/e7cf59e84767e30b507b6bd7c1347072ec12b636
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M include/qapi/error.h
    M qga/vss-win32.c
    M qga/vss-win32/requester.cpp
    M qga/vss-win32/requester.h
    M util/error.c

  Log Message:
  -----------
  qga: Clean up unnecessarily dirty casts

qga_vss_fsfreeze() casts error_set_win32() from

    void (*)(Error **, int, ErrorClass, const char *, ...)

to

    void (*)(void **, int, int, const char *, ...)

The result is later called.  Since the two types are not compatible,
the call is undefined behavior.  It works in practice anyway.

However, there's no real need for trickery here.  Clean it up as
follows:

* Declare struct Error, and fix the first parameter.

* Switch to error_setg_win32().  This gets rid of the troublesome
  ErrorClass parameter.  Requires converting error_setg_win32() from
  macro to function, but that's trivially easy, because this is the
  only user of error_set_win32().

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 08e64640357cd9517aa30fd49840f05f0f2ee3a4
      
https://github.com/qemu/qemu/commit/08e64640357cd9517aa30fd49840f05f0f2ee3a4
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M qga/vss-win32.c
    M qga/vss-win32/requester.cpp
    M qga/vss-win32/requester.h

  Log Message:
  -----------
  qga/vss-win32: Document the DLL requires non-null errp

requester.cpp uses this pattern to receive an error and pass it on to
the caller (err_is_set() macro peeled off for clarity):

    ... code that may set errset->errp ...
    if (errset->errp && *errset->errp) {
  ... handle error ...
    }

This breaks when errset->errp is null.  As far as I can tell, it
currently isn't, so this is merely fragile, not actually broken.

The robust way to do this is to receive the error in a local variable,
then propagate it up, like this:

    Error *err = NULL;

    ... code that may set err ...
    if (err)
  ... handle error ...
  error_propagate(errset->errp, err);
    }

See also commit 5e54769, 0f230bf, a903f40.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 4463dcb85c9f992f0c4d93f2142c8d64dcc85c5c
      
https://github.com/qemu/qemu/commit/4463dcb85c9f992f0c4d93f2142c8d64dcc85c5c
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M include/qapi/error.h
    M util/error.c

  Log Message:
  -----------
  error: error_set_errno() is unused, drop

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: edf6f3b3358597d37da0cf636ce3ed8a546d0f26
      
https://github.com/qemu/qemu/commit/edf6f3b3358597d37da0cf636ce3ed8a546d0f26
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M include/qapi/error.h

  Log Message:
  -----------
  error: Revamp interface documentation

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 1e9b65bb1bad51735cab6c861c29b592dccabf0e
      
https://github.com/qemu/qemu/commit/1e9b65bb1bad51735cab6c861c29b592dccabf0e
  Author: Markus Armbruster <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M include/qapi/error.h
    M qga/vss-win32.c
    M qga/vss-win32/requester.cpp
    M qga/vss-win32/requester.h
    M util/error.c

  Log Message:
  -----------
  error: On abort, report where the error was created

This is particularly useful when we abort in error_propagate(),
because there the stack backtrace doesn't lead to where the error was
created.  Looks like this:

    Unexpected error in parse_block_error_action() at .../qemu/blockdev.c:322:
    qemu-system-x86_64: -drive if=none,werror=foo: 'foo' invalid write error 
action
    Aborted (core dumped)

Note: to get this example output, I monkey-patched drive_new() to pass
&error_abort to blockdev_init().

To keep the error handling boiler plate from growing even more, all
error_setFOO() become macros expanding into error_setFOO_internal()
with additional __FILE__, __LINE__, __func__ arguments.  Not exactly
pretty, but it works.

The macro trickery breaks down when you take the address of an
error_setFOO().  Fortunately, we do that in just one place: qemu-ga's
Windows VSS provider and requester DLL wants to call
error_setg_win32() through a function pointer "to avoid linking glib
to the DLL".  Use error_setg_win32_internal() there.  The use of the
function pointer is already wrapped in a macro, so the churn isn't
bad.

Code size increases by some 35KiB for me (0.7%).  Tolerable.  Could be
less if we passed relative rather than absolute source file names to
the compiler, or forwent reporting __func__.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Acked-by: Laszlo Ersek <address@hidden>


  Commit: fe556410cf09d6181a4e694c6db31562fdcfbeba
      
https://github.com/qemu/qemu/commit/fe556410cf09d6181a4e694c6db31562fdcfbeba
  Author: Peter Maydell <address@hidden>
  Date:   2015-09-10 (Thu, 10 Sep 2015)

  Changed paths:
    M include/qapi/error.h
    M qga/vss-win32.c
    M qga/vss-win32/requester.cpp
    M qga/vss-win32/requester.h
    M util/error.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2015-09-10' into 
staging

error: On abort, report where the error was created

# gpg: Signature made Thu 10 Sep 2015 13:01:39 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"

* remotes/armbru/tags/pull-error-2015-09-10:
  error: On abort, report where the error was created
  error: Revamp interface documentation
  error: error_set_errno() is unused, drop
  qga/vss-win32: Document the DLL requires non-null errp
  qga: Clean up unnecessarily dirty casts
  error: Make error_setg() a function
  error: De-duplicate code creating Error objects

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/fbf054cb0a8c...fe556410cf09