[libvirt PATCH 1/2] qemu: fix memory leak reported by coverity

Let g_autoptr clean up on early return. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_monitor_json.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cba9ec7b19..2491cbf9b8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4013,7 +4013,7 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon, const char *opaque, qemuMonitorAddFdInfoPtr fdinfo) { - virJSONValuePtr args = NULL; + g_autoptr(virJSONValue) args = NULL; g_autoptr(virJSONValue) reply = NULL; g_autoptr(virJSONValue) cmd = NULL; @@ -4024,7 +4024,8 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon, if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0) return -1; - if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", args))) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", + g_steal_pointer(&args)))) return -1; if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply) < 0) -- 2.26.2

Trivial fix to improve readability by combining these into a compound conditional. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_monitor_json.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2491cbf9b8..8d3b6c98e3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4020,9 +4020,9 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon, if (virJSONValueObjectCreate(&args, "S:opaque", opaque, NULL) < 0) return -1; - if (fdset >= 0) - if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0) - return -1; + if (fdset >= 0 && + virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0) + return -1; if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", g_steal_pointer(&args)))) -- 2.26.2

On 10/23/20 5:40 PM, Jonathon Jongsma wrote:
Trivial fix to improve readability by combining these into a compound conditional.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_monitor_json.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2491cbf9b8..8d3b6c98e3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4020,9 +4020,9 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon, if (virJSONValueObjectCreate(&args, "S:opaque", opaque, NULL) < 0) return -1;
- if (fdset >= 0) - if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0) - return -1; + if (fdset >= 0 && + virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0) + return -1;
In this case, I've added braces rather than combining the lines, since each is a separate subexpression, rather than just pieces of a single simple expression, as was the case in the previous patch. Reviewed-by: Laine Stump <laine@redhat.com> and pushed.
if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", g_steal_pointer(&args))))

On 10/23/20 5:40 PM, Jonathon Jongsma wrote:
Let g_autoptr clean up on early return.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_monitor_json.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cba9ec7b19..2491cbf9b8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4013,7 +4013,7 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon, const char *opaque, qemuMonitorAddFdInfoPtr fdinfo) { - virJSONValuePtr args = NULL; + g_autoptr(virJSONValue) args = NULL; g_autoptr(virJSONValue) reply = NULL; g_autoptr(virJSONValue) cmd = NULL;
@@ -4024,7 +4024,8 @@ int qemuMonitorJSONAddFileHandleToSet(qemuMonitorPtr mon, if (virJSONValueObjectAdd(args, "j:fdset-id", fdset, NULL) < 0) return -1;
- if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", args))) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("add-fd", + g_steal_pointer(&args)))) return -1;
The examples about curly braces in the coding guidelines say that curly braces around the body in this case are optional, but the text itself says that they are required. I'm not sure why this ambiguity exists, but because combining the two lines of the conditional only increases the line length to 86 (based on the sentiments expressed during a recent discussion on that topic on the mailing list), I'm going to do that instead of adding the curly braces. Reviewed-by: Laine Stump <laine@redhat.com> and pushed.
if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply) < 0)
participants (2)
-
Jonathon Jongsma
-
Laine Stump