From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60337) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze96C-0007U4-GF for qemu-devel@nongnu.org; Mon, 21 Sep 2015 17:58:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ze96B-0000li-Ah for qemu-devel@nongnu.org; Mon, 21 Sep 2015 17:58:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46746) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze96B-0000lP-3Z for qemu-devel@nongnu.org; Mon, 21 Sep 2015 17:58:11 -0400 From: Eric Blake Date: Mon, 21 Sep 2015 15:57:23 -0600 Message-Id: <1442872682-6523-8-git-send-email-eblake@redhat.com> In-Reply-To: <1442872682-6523-1-git-send-email-eblake@redhat.com> References: <1442872682-6523-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Michael Roth , marcandre.lureau@redhat.com, DirtY.iCE.hu@gmail.com, armbru@redhat.com, ehabkost@redhat.com Due to the existing semantics of the error_set() family, generated sequences in the qapi visitors such as: visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err); if (!err) { visit_type_FOO_fields(m, obj, errp); visit_end_implicit_struct(m, &err); } error_propagate(errp, err); are risky: if visit_type_FOO_fields() sets errp, and then visit_end_implicit_struct() also encounters an error, the attempt to overwrite the first error will cause an abort(). Obviously, we weren't triggering this situation (none of the existing callbacks for visit_end_implicit_struct() currently try to set an error), but it is better to not even cause the problem in the first place. Meanwhile, in spite of the poor documentation of the qapi visitors, we want to guarantee that if a visit_start_*() succeeds, then the matching visit_end_*() will be called. The options are to either propagate and clear a local err multiple times: visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err); if (!err) { visit_type_FOO_fields(m, obj, &err); if (err) { error_propagate(errp, err); err = NULL; } visit_end_implicit_struct(m, &err); } error_propagate(errp, err); or, as this patch does, just pass in NULL to ignore further errors once an error has occurred. visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err); if (!err) { visit_type_FOO_fields(m, obj, &err); visit_end_implicit_struct(m, err ? NULL : &err); } error_propagate(errp, err); Signed-off-by: Eric Blake --- scripts/qapi-visit.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 97343cf..d911b20 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -51,8 +51,8 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error * visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err); if (!err) { - visit_type_%(c_type)s_fields(m, obj, errp); - visit_end_implicit_struct(m, &err); + visit_type_%(c_type)s_fields(m, obj, &err); + visit_end_implicit_struct(m, err ? NULL : &err); } error_propagate(errp, err); } @@ -143,9 +143,9 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); if (!err) { if (*obj) { - visit_type_%(c_name)s_fields(m, obj, errp); + visit_type_%(c_name)s_fields(m, obj, &err); } - visit_end_struct(m, &err); + visit_end_struct(m, err ? NULL : &err); } error_propagate(errp, err); } @@ -175,9 +175,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err); } - error_propagate(errp, err); - err = NULL; - visit_end_list(m, &err); + visit_end_list(m, err ? NULL : &err); out: error_propagate(errp, err); } @@ -231,9 +229,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error abort(); } out_end: - error_propagate(errp, err); - err = NULL; - visit_end_implicit_struct(m, &err); + visit_end_implicit_struct(m, err ? NULL : &err); out: error_propagate(errp, err); } @@ -330,10 +326,8 @@ out_obj: error_propagate(errp, err); err = NULL; visit_end_union(m, !!(*obj)->data, &err); - error_propagate(errp, err); - err = NULL; } - visit_end_struct(m, &err); + visit_end_struct(m, err ? NULL : &err); out: error_propagate(errp, err); } -- 2.4.3