monitor_qmp_dispatch_one() frees a QMPRequest manually, because it
needs to keep a reference to ->id.  Premature optimization.  Take an
additional reference so we can use qmp_request_free().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 10991757f6..94f5660c3c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4181,8 +4181,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
     id = req_obj->id;
     need_resume = req_obj->need_resume;
 
-    g_free(req_obj);
-
     old_mon = cur_mon;
     cur_mon = mon;
 
@@ -4191,14 +4189,14 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
     cur_mon = old_mon;
 
     /* Respond if necessary */
-    monitor_qmp_respond(mon, rsp, NULL, id);
+    monitor_qmp_respond(mon, rsp, NULL, qobject_ref(id));
 
     /* This pairs with the monitor_suspend() in handle_qmp_command(). */
     if (need_resume) {
         monitor_resume(mon);
     }
 
-    qobject_unref(req);
+    qmp_request_free(req_obj);
 }
 
 /*
-- 
2.17.1

On 07/02/2018 11:22 AM, Markus Armbruster wrote:
> monitor_qmp_dispatch_one() frees a QMPRequest manually, because it
> needs to keep a reference to ->id.  Premature optimization.  Take an
> additional reference so we can use qmp_request_free().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   monitor.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/monitor.c b/monitor.c
> index 10991757f6..94f5660c3c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4181,8 +4181,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
>       id = req_obj->id;
>       need_resume = req_obj->need_resume;
>   
> -    g_free(req_obj);
> -
>       old_mon = cur_mon;
>       cur_mon = mon;
>   
> @@ -4191,14 +4189,14 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
>       cur_mon = old_mon;
>   
>       /* Respond if necessary */
> -    monitor_qmp_respond(mon, rsp, NULL, id);
> +    monitor_qmp_respond(mon, rsp, NULL, qobject_ref(id));

This requires qobject_ref(NULL) to work (I thought at one point we were 
considering requiring it to be a non-NULL argument, but right now it 
looks like we are safe).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Eric Blake <eblake@redhat.com> writes:

> On 07/02/2018 11:22 AM, Markus Armbruster wrote:
>> monitor_qmp_dispatch_one() frees a QMPRequest manually, because it
>> needs to keep a reference to ->id.  Premature optimization.  Take an
>> additional reference so we can use qmp_request_free().
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   monitor.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>> diff --git a/monitor.c b/monitor.c
>> index 10991757f6..94f5660c3c 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -4181,8 +4181,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
>>       id = req_obj->id;
>>       need_resume = req_obj->need_resume;
>>   -    g_free(req_obj);
>> -
>>       old_mon = cur_mon;
>>       cur_mon = mon;
>>   @@ -4191,14 +4189,14 @@ static void
>> monitor_qmp_dispatch_one(QMPRequest *req_obj)
>>       cur_mon = old_mon;
>>         /* Respond if necessary */
>> -    monitor_qmp_respond(mon, rsp, NULL, id);
>> +    monitor_qmp_respond(mon, rsp, NULL, qobject_ref(id));
>
> This requires qobject_ref(NULL) to work (I thought at one point we
> were considering requiring it to be a non-NULL argument, but right now
> it looks like we are safe).

[PATCH v6 5/5] qobject: modify qobject_ref() to assert on NULL
Message-Id: <20180419150145.24795-6-marcandre.lureau@redhat.com>

I didn't merge it back then because I lacked the time to convince myself
all users of qobject_ref() the patch doesn't guard cannot pass null, and
also because I wasn't sure requiring qobject_ref()'s argument to be
non-null improves legibility of the code.  Right now we got bigger fish
to fry.