[libvirt PATCH 0/5] Various small fixes
Reported by Coverity (7 months ago) or cppcheck (today). Ján Tomko (5): util: event: check return value of virInitialize qemu: firmware: check virJSONValueObjectGet return value rpc: socket: properly call virSetCloseExec virsh: do not return bool in virshNetworkPortUUIDCompleter Use (un)signed printf specifiers correctly examples/c/domain/domtop.c | 2 +- examples/c/domain/suspend.c | 2 +- src/qemu/qemu_firmware.c | 9 ++++++++- src/rpc/virnetsocket.c | 2 +- src/util/virevent.c | 3 ++- tests/qemusecuritymock.c | 2 +- tests/virhashtest.c | 4 ++-- tests/virpcimock.c | 2 +- tools/virsh-completer-network.c | 2 +- 9 files changed, 18 insertions(+), 10 deletions(-) -- 2.26.2
This function can possibly fail. Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 2e07a1e14635ad25c57b66c13488feff4c8d2b0c --- src/util/virevent.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virevent.c b/src/util/virevent.c index 3477d63554..f6eb806faf 100644 --- a/src/util/virevent.c +++ b/src/util/virevent.c @@ -305,7 +305,8 @@ int virEventRegisterDefaultImpl(void) { VIR_DEBUG("registering default event implementation"); - virInitialize(); + if (virInitialize() < 0) + return -1; virResetLastError(); -- 2.26.2
If the mapping is not present, we should not try to access its elements. Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 8b5b80f4c5f7342eedce0747469223387ab709ef --- src/qemu/qemu_firmware.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index ffe2df20aa..480ce0b00d 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -434,10 +434,17 @@ qemuFirmwareMappingParse(const char *path, virJSONValuePtr doc, qemuFirmwarePtr fw) { - virJSONValuePtr mapping = virJSONValueObjectGet(doc, "mapping"); + virJSONValuePtr mapping; const char *deviceStr; int tmp; + if (!(mapping = virJSONValueObjectGet(doc, "mapping"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing mapping in '%s'"), + path); + return -1; + } + if (!(deviceStr = virJSONValueObjectGetString(mapping, "device"))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing device type in '%s'"), -- 2.26.2
cppcheck reports: style: Argument 'fd<0' to function virSetCloseExec is always 0 [knownArgument] Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 4b9919af4024a6fbc3d4ee996d8a4c27dbc44285 --- src/rpc/virnetsocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index ebdeadc4a0..f79a638775 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1388,7 +1388,7 @@ int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) } #ifndef F_DUPFD_CLOEXEC if (cloexec && - virSetCloseExec(fd < 0)) { + virSetCloseExec(fd) < 0) { int saveerr = errno; closesocket(fd); errno = saveerr; -- 2.26.2
On Tue, Sep 22, 2020 at 11:32:31PM +0200, Ján Tomko wrote:
cppcheck reports: style: Argument 'fd<0' to function virSetCloseExec is always 0 [knownArgument]
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 4b9919af4024a6fbc3d4ee996d8a4c27dbc44285 --- src/rpc/virnetsocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index ebdeadc4a0..f79a638775 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1388,7 +1388,7 @@ int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) } #ifndef F_DUPFD_CLOEXEC if (cloexec && - virSetCloseExec(fd < 0)) { + virSetCloseExec(fd) < 0) {
Ewww, so IIUC we were setting close-exec on fd 0 every time here. Lucky it didn't do anything worse. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
portability: Returning an integer in a function with pointer return type is not portable. [CastIntegerToAddressAtReturn] Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-completer-network.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c index a63d657477..73f7115ab2 100644 --- a/tools/virsh-completer-network.c +++ b/tools/virsh-completer-network.c @@ -103,7 +103,7 @@ virshNetworkPortUUIDCompleter(vshControl *ctl, return NULL; if (!(net = virshCommandOptNetwork(ctl, cmd, NULL))) - return false; + return NULL; if ((nports = virNetworkListAllPorts(net, &ports, flags)) < 0) return NULL; -- 2.26.2
Various places reported by cppcheck's invalidPrintfArgType_sint and invalidPrintfArgType_uint. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- examples/c/domain/domtop.c | 2 +- examples/c/domain/suspend.c | 2 +- tests/qemusecuritymock.c | 2 +- tests/virhashtest.c | 4 ++-- tests/virpcimock.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/c/domain/domtop.c b/examples/c/domain/domtop.c index 15611c586d..5228445b7c 100644 --- a/examples/c/domain/domtop.c +++ b/examples/c/domain/domtop.c @@ -115,7 +115,7 @@ parse_argv(int argc, char *argv[], } *milliseconds = val; if (*milliseconds != val) { - ERROR("Integer overflow: %ld", val); + ERROR("Integer overflow: %lu", val); exit(EXIT_FAILURE); } break; diff --git a/examples/c/domain/suspend.c b/examples/c/domain/suspend.c index 980c4584c7..3ff24f6861 100644 --- a/examples/c/domain/suspend.c +++ b/examples/c/domain/suspend.c @@ -105,7 +105,7 @@ parse_argv(int argc, char *argv[], } *seconds = val; if (*seconds != val) { - ERROR("Integer overflow: %ld", val); + ERROR("Integer overflow: %lu", val); return -1; } break; diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index 6da8a91a9e..d544ae56cd 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -265,7 +265,7 @@ mock_chown(const char *path, int ret = -1; if (gid >> 16 || uid >> 16) { - fprintf(stderr, "Attempt to set too high UID or GID: %lld %lld", + fprintf(stderr, "Attempt to set too high UID or GID: %llu %llu", (unsigned long long) uid, (unsigned long long) gid); abort(); } diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 4d05cbb0f8..af30791241 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -34,7 +34,7 @@ testHashInit(int size) } if (virHashTableSize(hash) != oldsize) { - VIR_TEST_DEBUG("hash grown from %zd to %zd", + VIR_TEST_DEBUG("hash grown from %zu to %zu", (size_t)oldsize, (size_t)virHashTableSize(hash)); } } @@ -313,7 +313,7 @@ testHashRemoveSet(const void *data G_GNUC_UNUSED) if (count != rcount) { VIR_TEST_VERBOSE("\nvirHashRemoveSet didn't remove expected number of" - " entries, %d != %u", + " entries, %d != %d", rcount, count); goto cleanup; } diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 787172d24c..469aa01bb0 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -120,7 +120,7 @@ struct pciDeviceAddress { unsigned int device; unsigned int function; }; -# define ADDR_STR_FMT "%04x:%02x:%02x.%d" +# define ADDR_STR_FMT "%04x:%02x:%02x.%u" struct pciDevice { struct pciDeviceAddress addr; -- 2.26.2
On Tue, Sep 22, 2020 at 23:32:28 +0200, Ján Tomko wrote:
Reported by Coverity (7 months ago) or cppcheck (today).
Ján Tomko (5): util: event: check return value of virInitialize qemu: firmware: check virJSONValueObjectGet return value rpc: socket: properly call virSetCloseExec virsh: do not return bool in virshNetworkPortUUIDCompleter Use (un)signed printf specifiers correctly
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa