aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarkus Armbruster <armbru@redhat.com>2015-06-20 09:33:56 +0200
committerMarkus Armbruster <armbru@redhat.com>2015-09-10 13:48:06 +0200
commit08e64640357cd9517aa30fd49840f05f0f2ee3a4 (patch)
tree1d4de3c1c8df39ca0d1d20d65eecdfa8a5d6dfd8
parente7cf59e84767e30b507b6bd7c1347072ec12b636 (diff)
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 <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
-rw-r--r--qga/vss-win32.c1
-rw-r--r--qga/vss-win32/requester.cpp3
-rw-r--r--qga/vss-win32/requester.h2
3 files changed, 4 insertions, 2 deletions
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index e1f539835c..d75d7bbf09 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 b130fee934..aae0d5f2bf 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 0a8d048874..34be5c1d11 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);