Paolo Bonzini <pbonz...@redhat.com> writes: > On 25/03/20 19:47, Marc-André Lureau wrote: >> If object-add failed, no need to create a return value that may later >> be leaked. >> >> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> >> --- >> qom/qom-qmp-cmds.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c >> index 435193b036..6bd137ccbf 100644 >> --- a/qom/qom-qmp-cmds.c >> +++ b/qom/qom-qmp-cmds.c >> @@ -287,8 +287,8 @@ void qmp_object_add(QDict *qdict, QObject **ret_data, >> Error **errp) >> visit_free(v); >> if (obj) { >> object_unref(obj); >> + *ret_data = QOBJECT(qdict_new()); >> } >> - *ret_data = QOBJECT(qdict_new()); >> } >> >> void qmp_object_del(const char *id, Error **errp) >> > > It can be slightly simplified: > > ------------------- 8< ---------------------- > From: Paolo Bonzini <pbonz...@redhat.com> > Subject: [PATCH] object-add: don't create return value if failed > > No need to return an empty value from object-add (it would also leak > if the command failed). While at it, remove the "if" around object_unref > since object_unref handles NULL arguments just fine. > > Reported-by: Marc-André Lureau <marcandre.lur...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > > diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c > index 435193b036..e47ebe8ed1 100644 > --- a/qom/qom-qmp-cmds.c > +++ b/qom/qom-qmp-cmds.c > @@ -285,10 +285,7 @@ void qmp_object_add(QDict *qdict, QObject **ret_data, > Error **errp) > v = qobject_input_visitor_new(QOBJECT(qdict)); > obj = user_creatable_add_type(type, id, qdict, v, errp); > visit_free(v); > - if (obj) { > - object_unref(obj); > - } > - *ret_data = QOBJECT(qdict_new()); > + object_unref(obj); > } > > void qmp_object_del(const char *id, Error **errp)
Yes, that's better. Reviewed-by: Markus Armbruster <arm...@redhat.com> > I queued this patch and your other two. Thanks, > > Paolo