From ee16ce93373dda1faa4aa1bb4d99bcdf158f7d7a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 2 May 2014 13:26:37 +0200 Subject: [PATCH] qapi: Clean up fragile use of error_is_set() Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). The error_is_set(errp) in do_qmp_dispatch() is merely fragile, because the caller never passes a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Michael Roth Signed-off-by: Luiz Capitulino --- qapi/qmp-dispatch.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 187af56b86..168b083c87 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -62,6 +62,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) static QObject *do_qmp_dispatch(QObject *request, Error **errp) { + Error *local_err = NULL; const char *command; QDict *args, *dict; QmpCommand *cmd; @@ -93,13 +94,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp) switch (cmd->type) { case QCT_NORMAL: - cmd->fn(args, &ret, errp); - if (!error_is_set(errp)) { - if (cmd->options & QCO_NO_SUCCESS_RESP) { - g_assert(!ret); - } else if (!ret) { - ret = QOBJECT(qdict_new()); - } + cmd->fn(args, &ret, &local_err); + if (local_err) { + error_propagate(errp, local_err); + } else if (cmd->options & QCO_NO_SUCCESS_RESP) { + g_assert(!ret); + } else if (!ret) { + ret = QOBJECT(qdict_new()); } break; }