From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn41i-0003Zj-IN for qemu-devel@nongnu.org; Wed, 21 May 2014 06:45:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn41c-0002Vx-EW for qemu-devel@nongnu.org; Wed, 21 May 2014 06:45:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11457) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn41c-0002Vb-5e for qemu-devel@nongnu.org; Wed, 21 May 2014 06:45:32 -0400 From: Gerd Hoffmann Date: Wed, 21 May 2014 12:45:13 +0200 Message-Id: <1400669115-17663-4-git-send-email-kraxel@redhat.com> In-Reply-To: <1400669115-17663-1-git-send-email-kraxel@redhat.com> References: <1400669115-17663-1-git-send-email-kraxel@redhat.com> Subject: [Qemu-devel] [PULL 3/5] char: 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: Markus Armbruster , Anthony Liguori , Gerd Hoffmann From: Markus Armbruster 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 qemu_chr_new_from_opts() is merely fragile, because the callers never pass 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 Signed-off-by: Gerd Hoffmann --- qemu-char.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 3eaefc9..5a7975f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3204,6 +3204,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, void (*init)(struct CharDriverState *s), Error **errp) { + Error *local_err = NULL; CharDriver *cd; CharDriverState *chr; GSList *i; @@ -3245,8 +3246,9 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, chr = NULL; backend->kind = cd->kind; if (cd->parse) { - cd->parse(opts, backend, errp); - if (error_is_set(errp)) { + cd->parse(opts, backend, &local_err); + if (local_err) { + error_propagate(errp, local_err); goto qapi_out; } } -- 1.8.3.1