[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 3/3] object-add: don't create return value if failed
From: |
Markus Armbruster |
Subject: |
Re: [PATCH for-5.0 3/3] object-add: don't create return value if failed |
Date: |
Mon, 30 Mar 2020 16:48:59 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Paolo Bonzini <address@hidden> 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 <address@hidden>
>> ---
>> 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 <address@hidden>
> 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 <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
>
> 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 <address@hidden>
> I queued this patch and your other two. Thanks,
>
> Paolo