[libvirt PATCH 0/2] coverity unchecked returns

For #2, eventually we'd convert QEMUCaps to g_object and fix all the copy functions, but that's too big of a yak to shave now. Ján Tomko (2): qemu: json: check return value of virJSONValueObjectGetBoolean testsutilsqemu: check return value of virQEMUCapsNewCopy src/qemu/qemu_monitor_json.c | 6 ++++-- tests/testutilsqemu.c | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) -- 2.26.2

Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 7555a554706e8a6d492804debbacfaae9f5d8b05 --- src/qemu/qemu_monitor_json.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 723bdb4426..0428ed0b6f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1370,8 +1370,10 @@ qemuMonitorJSONHandleMemoryFailure(qemuMonitorPtr mon, } if (flagsjson) { - virJSONValueObjectGetBoolean(flagsjson, "action-required", &ar); - virJSONValueObjectGetBoolean(flagsjson, "recursive", &recursive); + if (virJSONValueObjectGetBoolean(flagsjson, "action-required", &ar) < 0) + return; + if (virJSONValueObjectGetBoolean(flagsjson, "recursive", &recursive) < 0) + return; } mf.recipient = recipient; -- 2.26.2

On Fri, Nov 20, 2020 at 14:46:06 +0100, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 7555a554706e8a6d492804debbacfaae9f5d8b05 --- src/qemu/qemu_monitor_json.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
See the discussion at: https://www.redhat.com/archives/libvir-list/2020-November/msg00859.html

On a Friday in 2020, Peter Krempa wrote:
On Fri, Nov 20, 2020 at 14:46:06 +0100, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 7555a554706e8a6d492804debbacfaae9f5d8b05 --- src/qemu/qemu_monitor_json.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
See the discussion at:
https://www.redhat.com/archives/libvir-list/2020-November/msg00859.html
The discussion revolves around silencing coverity. This patch aims to fix the bug. Jano

On Fri, Nov 20, 2020 at 15:08:31 +0100, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
On Fri, Nov 20, 2020 at 14:46:06 +0100, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 7555a554706e8a6d492804debbacfaae9f5d8b05 --- src/qemu/qemu_monitor_json.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
See the discussion at:
https://www.redhat.com/archives/libvir-list/2020-November/msg00859.html
The discussion revolves around silencing coverity.
This patch aims to fix the bug.
Well, would you care to explain the bug then? Ideally in the commit message of all places.

On Fri, Nov 20, 2020 at 15:11:19 +0100, Peter Krempa wrote:
On Fri, Nov 20, 2020 at 15:08:31 +0100, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
On Fri, Nov 20, 2020 at 14:46:06 +0100, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 7555a554706e8a6d492804debbacfaae9f5d8b05 --- src/qemu/qemu_monitor_json.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
See the discussion at:
https://www.redhat.com/archives/libvir-list/2020-November/msg00859.html
The discussion revolves around silencing coverity.
This patch aims to fix the bug.
Well, would you care to explain the bug then? Ideally in the commit message of all places.
To be more specific, if you think it's a bug if the two booleans are missing from the event, you should also report the warning as all other missing fields from that event do in the function.

While for virQEMUCapsNew this should not be needed (the possible failures in VIR_CLASS_NEW are only hit on bad API usage which we don't do here), virQEMUCapsNewCopy calls into many other functions, some of which actually fail. Check the return value of both. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/testutilsqemu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 906fdf5c1a..cea4f84b14 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -349,6 +349,9 @@ int qemuTestCapsCacheInsert(virFileCachePtr cache, tmpCaps = virQEMUCapsNew(); } + if (!tmpCaps) + return -1; + if (!virQEMUCapsHasMachines(tmpCaps)) { const char *defaultRAMid = NULL; -- 2.26.2

On Fri, Nov 20, 2020 at 14:46:07 +0100, Ján Tomko wrote:
While for virQEMUCapsNew this should not be needed (the possible failures in VIR_CLASS_NEW are only hit on bad API usage which we don't do here), virQEMUCapsNewCopy calls into many other functions, some of which actually fail.
Check the return value of both.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/testutilsqemu.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Ján Tomko
-
Peter Krempa