From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50010) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WgBcM-0000qu-Hl for qemu-devel@nongnu.org; Fri, 02 May 2014 07:27:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WgBcB-0007XU-T6 for qemu-devel@nongnu.org; Fri, 02 May 2014 07:27:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2997) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WgBcB-0007X5-Kc for qemu-devel@nongnu.org; Fri, 02 May 2014 07:26:51 -0400 From: Markus Armbruster Date: Fri, 2 May 2014 13:26:37 +0200 Message-Id: <1399030002-27222-10-git-send-email-armbru@redhat.com> In-Reply-To: <1399030002-27222-1-git-send-email-armbru@redhat.com> References: <1399030002-27222-1-git-send-email-armbru@redhat.com> Subject: [Qemu-devel] [PATCH v3 09/14] qapi: Clean up fragile use of error_is_set() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: pbonzini@redhat.com, mdroth@linux.vnet.ibm.com, lcapitulino@redhat.com 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 --- 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 187af56..168b083 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; } -- 1.8.1.4