Eric Blake <ebl...@redhat.com> 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 = 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 = 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 <ebl...@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> ---
> 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 = NULL;
>
> visit_start_struct(v, (void **)obj, "%(name)s", name,
> sizeof(%(c_name)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 = 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.
