[libvirt] [PATCH 1/3] qemu: agent: fix uninitialized var case in qemuAgentGetFSInfo

[Nikolay Shirokovskiy]

On 08.12.2016 19:38, John Ferlan wrote:
> 
> 
> On 11/24/2016 04:19 AM, Nikolay Shirokovskiy wrote:
>> In case of 0 filesystems *info is not set while according
>> to virDomainGetFSInfo contract user should call free on it even
>> in case of 0 filesystems. Thus we need to properly set
>> it. NULL will be enough as free eats NULLs ok.
>> ---
>>  src/qemu/qemu_agent.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
>> index ec8d47e..c5cf403 100644
>> --- a/src/qemu/qemu_agent.c
>> +++ b/src/qemu/qemu_agent.c
>> @@ -1872,6 +1872,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
>>      ndata = virJSONValueArraySize(data);
>>      if (!ndata) {
>>          ret = 0;
>> +        *info = NULL;
> 
> ACK - although there are more ways above this hunk that allow us to get
> to cleanup without setting *info = NULL;  Currently each of the callers

These are error paths. The caller have no obligations to free info in these cases.

> sets the input info to NULL before calling here

qemuDomainGetFSInfo does not set...

> 
> IOW: We could also move that *info = NULL up before the call to
> virAgentMakeCommand
> 

I looked here and there in the libvirt code and found out that it does not zero out 
output parameter immediately. I guess it makes sense. Output parameter can be 
actually filled in deep below the call stack. Thus if one starts with convention 
to zero out immediately one have to do it in every function on the path.

I guess caller itself can zero out output parameter to simplify its cleanup logic.
And some callers are not zero out, they cleanup conditionally for example - these 
rely on function to properly set output parameter.

Nikolay