[libvirt PATCH v5 4/6] qemu: add monitor functions for handling file descriptors

[Laine Stump]

On 10/21/20 4:54 PM, John Ferlan wrote:
>
> On 10/14/20 1:08 PM, Jonathon Jongsma wrote:
>> add-fd, remove-fd, and query-fdsets provide functionality that can be
>> used for passing fds to qemu and closing fdsets that are no longer
>> necessary.
>>
>> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
>> ---
>>   src/qemu/qemu_monitor.c      |  93 +++++++++++++++++++
>>   src/qemu/qemu_monitor.h      |  41 +++++++++
>>   src/qemu/qemu_monitor_json.c | 173 +++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_monitor_json.h |  12 +++
>>   4 files changed, 319 insertions(+)
>>
> Coverity indicated a possible RESOURCE_LEAK
>
>> +/* if fdset is negative, qemu will create a new fdset and add the fd to that */
>> +int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon,
>> +                                      int fd,
>> +                                      int fdset,
>> +                                      const char *opaque,
>> +                                      qemuMonitorAddFdInfoPtr fdinfo)
>> +{
>> +    virJSONValuePtr args = NULL;
>> +    g_autoptr(virJSONValue) reply = NULL;
>> +    g_autoptr(virJSONValue) cmd = NULL;
>> +
>> +    if (virJSONValueObjectCreate(&args, "S:opaque", opaque, NULL) < 0)
>> +        return -1;
>> +
>> +    if (fdset >= 0)
>> +        if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0)
> Leaks @args


Yeah, I think args needs to be g_autoptr(virJSONValue)...


>
>> +            return -1;
> I'm surprised the code style gremlins didn't complain about not having {
> } or combining the conditions


(Wasn't watching close enough. I have to admit my eyes glazed over a bit 
:-P)


>
>> +
>> +    if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", args)))
>> +        return -1;


... and then here it can be passed as g_steal_pointer(&args) - 
qemuMonitorJSONMakeCommandInternal() will free it no matter what the 
outcome.


> I think at this point @args is consumed within @cmd ... which really
> confuses Coverity, but I have a bunch of hacks to handle that...
>
> John
>
>> +
>> +    if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply) < 0)
>> +        return -1;
>> +
>> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>> +        return -1;
>> +
>> +    if (qemuAddfdInfoParse(reply, fdinfo) < 0)
>> +        return -1;
>> +
>> +    return 0;
>> +}
>> +
> [...]
>