From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53509) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLvEs-0007px-9Z for qemu-devel@nongnu.org; Wed, 20 Jan 2016 11:04:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLvEo-0001jb-7W for qemu-devel@nongnu.org; Wed, 20 Jan 2016 11:04:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53864) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLvEo-0001jM-0l for qemu-devel@nongnu.org; Wed, 20 Jan 2016 11:04:02 -0500 From: Markus Armbruster References: <1453219845-30939-1-git-send-email-eblake@redhat.com> <1453219845-30939-9-git-send-email-eblake@redhat.com> Date: Wed, 20 Jan 2016 17:03:58 +0100 In-Reply-To: <1453219845-30939-9-git-send-email-eblake@redhat.com> (Eric Blake's message of "Tue, 19 Jan 2016 09:10:16 -0700") Message-ID: <87k2n4p79t.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v9 08/37] qapi: Track all failures between visit_start/stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > Inside the generated code between visit_start_struct() and > visit_end_struct(), we were blindly setting the error into > the caller's errp parameter. But a future patch to split > visit_end_struct() will require that we take action based > on whether an error has occurred, which requires us to track > all actions through a local err. Rewrite the visits to be > more in line with the other generated calls. > > Generated code changes look like: > > |@@ -42,12 +42,18 @@ void visit_type_GuestAgentCommandInfo(Vi I'd drop this line. > | Error *err =3D NULL; > | > | visit_start_struct(v, (void **)obj, "GuestAgentCommandInfo", name, = sizeof(GuestAgentCommandInfo), &err); > |- if (!err) { > |- if (*obj) { > |- visit_type_GuestAgentCommandInfo_fields(v, obj, errp); > |- } > |- visit_end_struct(v, &err); > |+ if (err) { > |+ goto out; > | } > |+ if (!*obj) { err is clear here. > |+ goto out_obj; > |+ } > |+ visit_type_GuestAgentCommandInfo_fields(v, obj, &err); > |+out_obj: > |+ error_propagate(errp, err); > |+ err =3D NULL; If we come from goto out_obj, these two lines are no-ops. > |+ visit_end_struct(v, &err); > |+out: > | error_propagate(errp, err); > | } > > Signed-off-by: Eric Blake > Reviewed-by: Marc-Andr=C3=A9 Lureau > > --- > v9: enhance commit message > v8: no change > v7: place earlier in series > v6: based loosely on v5 7/46, but mostly a rewrite to get the last > generated code in the same form as all the others, so that the > later conversion to split visit_check_struct() will be easier > --- > scripts/qapi-visit.py | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index b93690b..4a4f67d 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -123,12 +123,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s *= *obj, const char *name, Error > Error *err =3D NULL; > > visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_nam= e)s), &err); > - if (!err) { > - if (*obj) { > - visit_type_%(c_name)s_fields(v, obj, errp); > - } > - visit_end_struct(v, &err); > + if (err) { > + goto out; > } > + if (!*obj) { > + goto out_obj; > + } > + visit_type_%(c_name)s_fields(v, obj, &err); > +out_obj: > + error_propagate(errp, err); > + err =3D NULL; > + visit_end_struct(v, &err); > +out: > error_propagate(errp, err); > } > ''', gen_visit_union(), the other generator of visit_start_struct(), already does it this way. It generates additional goto out_obj, so the placement of out_obj makes more sense there. I guess we want to place it in the same spot here to facilitate unifying the two.