[PATCH 2/3] qemu: Move qemuAgentFSInfo array free into qemuDomainGetFSInfo()

[Michal Privoznik]

On 2/15/21 6:00 PM, Ján Tomko wrote:
> On a Monday in 2021, Michal Privoznik wrote:
>> When qemuDomainGetFSInfo() is called it calls
>> qemuDomainGetFSInfoAgent() which executes 'guest-get-fsinfo'
>> guest agent command, parses returned JSON and returns an array of
>> qemuAgentFSInfo structures (well, pointers to those structs).
>> Then it grabs a domain job and tries to do some matching of guest
>> returned info against domain definition. This matching is done in
>> virDomainFSInfoFormat() which also frees the array of
>> qemuAgentFSInfo structures allocated earlier.
>>
>> But this is not just. If acquiring the domain job fails (or
>> domain activeness check executed right after that fails) then
>> virDomainFSInfoFormat() is not called, leaking the array of
>> structs.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 21 +++++++++++++--------
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index f59f9e13ba..d30cf75b73 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -18977,14 +18977,14 @@ virDomainFSInfoFormat(qemuAgentFSInfoPtr 
>> *agentinfo,
>>     ret = nagentinfo;
>>
>>  cleanup:
>> -    for (i = 0; i < nagentinfo; i++) {
>> -        qemuAgentFSInfoFree(agentinfo[i]);
>> -        /* if there was an error, free any memory we've allocated for 
>> the
>> -         * return value */
>> -        if (info_ret)
>> +    if (info_ret) {
>> +        for (i = 0; i < nagentinfo; i++) {
>> +            /* if there was an error, free any memory we've allocated 
>> for the
>> +             * return value */
>>             virDomainFSInfoFree(info_ret[i]);
>> +        }
>> +        g_free(info_ret);
>>     }
>> -    g_free(info_ret);
>>     return ret;
>> }
> 
> This hunk is unrelated and just cosmetic.

I'm not sure what you mean by 'unrelated'. The freeing must be moved in 
one commit, otherwise either the array is leaked or double freed.

Michal