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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
