Eric Blake <ebl...@redhat.com> writes: > All other successful clients of visit_start_struct() were paired > with an unconditional visit_end_struct(); but the generated > code for events was relying on qmp_output_visitor_cleanup() to > work on an incomplete visit. Alter the code to guarantee that > the struct is completed, which will make a future patch to > split visit_end_struct() easier to reason about. While at it, > drop some assertions and comments that are not present in other > uses of the qmp output visitor, and pass NULL rather than "" as > the 'kind' parameter (matching most other uses where obj is NULL). > > The changes to the generated code look like: > > | qov = qmp_output_visitor_new(); > |- g_assert(qov); > |- > | v = qmp_output_get_visitor(qov); > |- g_assert(v); > | > |- /* Fake visit, as if all members are under a structure */ > |- visit_start_struct(v, NULL, "", "ACPI_DEVICE_OST", 0, &err); > |+ visit_start_struct(v, NULL, NULL, "ACPI_DEVICE_OST", 0, &err); > | if (err) { > | goto out; > | } > | visit_type_ACPIOSTInfo(v, &info, "info", &err); > | if (err) { > |- goto out; > |+ goto out_obj; > | } > |- visit_end_struct(v, &err); > |+out_obj: > |+ visit_end_struct(v, err ? NULL : &err);
Slightly awkward example, because out_obj is pointless in this degenerated case. You could pick one with multiple members (thus multiple goto out_obj), or do pseudo-code hinting at multiple members. > | if (err) { > | goto out; > | } > | > | obj = qmp_output_get_qobject(qov); > |- g_assert(obj != NULL); > |+ g_assert(obj); > | > | qdict_put_obj(qmp, "data", obj); > | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err); > | > |out: > | qmp_output_visitor_cleanup(qov); > | error_propagate(errp, err); > > Note that the 'goto out_obj' with no intervening code before the > label, as well as the construct of 'err ? NULL : &err', are both > a bit unusual but also temporary; they get fixed in a later patch > that splits visit_end_struct() to drop its errp parameter by moving > some checking before the label. But until that time, this was the > simplest way to avoid the appearance of passing a possibly-set > error to visit_end_struct(), even though actual code inspection > shows that visit_end_struct() for a QMP output visitor will never > set an error. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v10: avoid appearance of &err misuse; enhance commit message > v9: save churn in declaration order for later series on boxed params, > drop Marc-Andre's R-b > v8: no change > v7: place earlier in series, adjust handling of 'kind' > v6: new patch > > If desired, I can defer the hunk re-ordering the declaration of > obj to later in the series where it actually comes in handy. Patch looks good.