| Message ID | 20200325184723.2029630-4-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Memory leak fixes | expand |
On 3/25/20 7:47 PM, 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.lureau@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) > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
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.lureau@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 <pbonzini@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.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@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) I queued this patch and your other two. Thanks, Paolo
Paolo Bonzini <pbonzini@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.lureau@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 <pbonzini@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.lureau@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@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 <armbru@redhat.com> > I queued this patch and your other two. Thanks, > > Paolo
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)
If object-add failed, no need to create a return value that may later be leaked. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- qom/qom-qmp-cmds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)