qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 4/7] qga/vss-win32: Document the DLL requires non-null errp
Date: Thu, 10 Sep 2015 15:32:25 +0200	[thread overview]
Message-ID: <1441891948-28169-5-git-send-email-armbru@redhat.com> (raw)
In-Reply-To: <1441891948-28169-1-git-send-email-armbru@redhat.com>

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 <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qga/vss-win32.c             | 1 +
 qga/vss-win32/requester.cpp | 3 ++-
 qga/vss-win32/requester.h   | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index e1f5398..d75d7bb 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -154,6 +154,7 @@ void qga_vss_fsfreeze(int *nr_volume, Error **errp, bool freeze)
         .errp = errp,
     };
 
+    g_assert(errp);             /* requester.cpp requires it */
     func = (QGAVSSRequesterFunc)GetProcAddress(provider_lib, func_name);
     if (!func) {
         error_setg_win32(errp, GetLastError(), "failed to load %s from %s",
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index b130fee..aae0d5f 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -25,8 +25,9 @@
 
 #define err_set(e, err, fmt, ...) \
     ((e)->error_setg_win32((e)->errp, err, fmt, ## __VA_ARGS__))
+/* Bad idea, works only when (e)->errp != NULL: */
 #define err_is_set(e) ((e)->errp && *(e)->errp)
-
+/* To lift this restriction, error_propagate(), like we do in QEMU code */
 
 /* Handle to VSSAPI.DLL */
 static HMODULE hLib;
diff --git a/qga/vss-win32/requester.h b/qga/vss-win32/requester.h
index 0a8d048..34be5c1 100644
--- a/qga/vss-win32/requester.h
+++ b/qga/vss-win32/requester.h
@@ -27,7 +27,7 @@ typedef void (*ErrorSetFunc)(struct Error **errp, int win32_err,
                              const char *fmt, ...) GCC_FMT_ATTR(3, 4);
 typedef struct ErrorSet {
     ErrorSetFunc error_setg_win32;
-    struct Error **errp;
+    struct Error **errp;        /* restriction: must not be null */
 } ErrorSet;
 
 STDAPI requester_init(void);
-- 
2.4.3

  parent reply	other threads:[~2015-09-10 13:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-10 13:32 [Qemu-devel] [PULL 0/7] error: On abort, report where the error was created Markus Armbruster
2015-09-10 13:32 ` [Qemu-devel] [PULL 1/7] error: De-duplicate code creating Error objects Markus Armbruster
2015-09-10 13:32 ` [Qemu-devel] [PULL 2/7] error: Make error_setg() a function Markus Armbruster
2015-09-10 13:32 ` [Qemu-devel] [PULL 3/7] qga: Clean up unnecessarily dirty casts Markus Armbruster
2015-09-10 13:32 ` Markus Armbruster [this message]
2015-09-10 13:32 ` [Qemu-devel] [PULL 5/7] error: error_set_errno() is unused, drop Markus Armbruster
2015-09-10 13:32 ` [Qemu-devel] [PULL 6/7] error: Revamp interface documentation Markus Armbruster
2015-09-10 13:32 ` [Qemu-devel] [PULL 7/7] error: On abort, report where the error was created Markus Armbruster
2015-09-10 16:21   ` Eric Blake
2015-09-10 16:38     ` Eric Blake
2015-09-10 16:00 ` [Qemu-devel] [PULL 0/7] " Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1441891948-28169-5-git-send-email-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).