From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34179) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WmQst-0000NY-Cw for qemu-devel@nongnu.org; Mon, 19 May 2014 12:58:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WmQsi-0002OX-Au for qemu-devel@nongnu.org; Mon, 19 May 2014 12:57:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33357) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WmQsi-0002Nu-4P for qemu-devel@nongnu.org; Mon, 19 May 2014 12:57:44 -0400 From: Markus Armbruster Date: Mon, 19 May 2014 18:57:35 +0200 Message-Id: <1400518658-2515-3-git-send-email-armbru@redhat.com> In-Reply-To: <1400518658-2515-1-git-send-email-armbru@redhat.com> References: <1400518658-2515-1-git-send-email-armbru@redhat.com> Subject: [Qemu-devel] [PATCH v2 2/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: pbonzini@redhat.com, kraxel@redhat.com, aliguori@amazon.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 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 --- 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.9.0