[libvirt] [PATCH 00/22] Resolve Coverity and build issues

[libvirt] [PATCH 0/3] allow fixing...

[libvirt] [PATCH 0/3] Three random...

John Ferlan

Wednesday, 22 March 2017 Wed, 22 Mar '17

9:21 a.m.

Hopefully the quantity doesn't scare anyone off... These are mostly innocuous, but separated each out to appease the masses. The first patch fixes a recently uncovered Coverity warning regarding FORWARD_NULL of the 'formatStr'... According to an IRC response from pkrempa the format should be set up properly in qemuDomainSnapshotPrepare Patches 2-22 resolve various different build issues in the Coverity environment that sets "lv_cv_static_analysis=yes" during the build. This is 'reproducible' if one sets that as part of their autogen.sh processing - something that we should probably do in the CI build environment so we can get all those ATTRIBUTE_NONNULL checks to be run on a daily basis. John Ferlan (22): qemu: Always format formatStr for blockdev-snapshot-sync util: Remove NONNULL(1) for virNetDevGetName conf: Remove NONNULL(2) for virNetDevBandwidthParse conf: Remove NONNULL(1,2) for virNetDevBandwidthFormat conf: Remove NONNULL(1) for virDomainNumaGetNodeCount network: Remove null newBandwidth check from networkBandwidthUpdate qemu: Remove non null 'vm' check from qemuMonitorOpen qemu: Remove NONNULL(1) for qemu_monitor prototypes util: Change return argument for virBitmapParseUnlimited util: Remove NONNULL(1) for virBitmapParseUnlimited util: Remove NONNULL(2,3) for virHostdevReAttachPCIDevices util: Remove NONNULL(2,3) for virHostdevReAttachUSBDevices util: Remove NONNULL(2,3) for virHostdevReAttachSCSIVHostDevices util: Remove NONNULL(1) for virHostdevPrepareDomainDevices util: Remove NONNULL(1) for virHostdevReAttachDomainDevices util: Remove NONNULL(1) for virNetDevOpenvswitchSetMigrateData util: Remove NONNULL(1,3,4) from virTypedParamsFilter util: Remove NONNULL(2) for virNetDevBandwidthPlug util: Remove NONNULL(1) for virNetDevOpenvswitchGetVhostuserIfname util: Remove NONNULL's for virNetDevVPortProfile[Associate|Disassociate] util: Remove NONNULL(1) for virNetDevMacVLanDeleteWithVPortProfile cpu: Remove NONNULL(1) for cpuBaseline src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 5 ++--- src/conf/numa_conf.h | 4 ++-- src/cpu/cpu.h | 3 +-- src/network/bridge_driver.c | 2 +- src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_monitor.c | 3 +-- src/qemu/qemu_monitor.h | 8 ++++---- src/util/virbitmap.c | 28 +++++++++++++--------------- src/util/virbitmap.h | 6 ++---- src/util/virhostdev.c | 9 ++++++--- src/util/virhostdev.h | 10 +++++----- src/util/virnetdev.h | 2 +- src/util/virnetdevbandwidth.h | 3 +-- src/util/virnetdevmacvlan.h | 2 +- src/util/virnetdevopenvswitch.h | 4 ++-- src/util/virnetdevvportprofile.h | 10 ++++------ src/util/virtypedparam.h | 3 +-- tests/testutils.c | 2 +- tests/virbitmaptest.c | 2 +- 20 files changed, 51 insertions(+), 60 deletions(-) -- 2.9.3

Show replies by date

John Ferlan

Wednesday, 22 March Wed, 22 Mar

9:21 a.m.

New subject: [libvirt] [PATCH 01/22] qemu: Always format formatStr for blockdev-snapshot-sync

The qemuDomainSnapshotPrepare should always set a > 0 format value anyway, so remove the check. Found by Coverity. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0e06508..6762952 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14072,8 +14072,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, dd->prepared = true; /* create the actual snapshot */ - if (dd->src->format) - formatStr = virStorageFileFormatTypeToString(dd->src->format); + formatStr = virStorageFileFormatTypeToString(dd->src->format); /* The monitor is only accessed if qemu doesn't support transactions. * Otherwise the following monitor command only constructs the command. -- 2.9.3

Peter Krempa

9:51 a.m.

New subject: [libvirt] [PATCH 01/22] qemu: Always format formatStr for blockdev-snapshot-sync

On Wed, Mar 22, 2017 at 10:21:14 -0400, John Ferlan wrote:

...

dd->src->format will be always non-zero so formatStr is guaranteed not to return NULL. The value is assigned in qemuDomainSnapshotPrepare The condition is probably useless here.

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 02/22] util: Remove NONNULL(1) for virNetDevGetName

The 'ifindex' argument is not a pointer, so no need for NONNULL in prototype Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virnetdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 01e9c5b..9c9daf1 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -158,7 +158,7 @@ int virNetDevSetName(const char *ifname, const char *newifname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; char *virNetDevGetName(int ifindex) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_RETURN_CHECK; int virNetDevGetIndex(const char *ifname, int *ifindex) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 03/22] conf: Remove NONNULL(2) for virNetDevBandwidthParse

Since the code checks and handles a NULL 'node' before proceeding there's no need for the prototype with the NONNULL(2). Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/netdev_bandwidth_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index cdeac09..c687427 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -32,7 +32,7 @@ int virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, xmlNodePtr node, int net_type) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevBandwidthFormat(virNetDevBandwidthPtr def, virBufferPtr buf) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 04/22] conf: Remove NONNULL(1, 2) for virNetDevBandwidthFormat

Since the code checks and handles NULL parameters, remove the NONNULL from the prototype. Also fix the comment in the source to reference the right name. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 8824332..50138fd 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -241,7 +241,7 @@ virNetDevBandwidthRateFormat(virNetDevBandwidthRatePtr def, } /** - * virNetDevBandwidthDefFormat: + * virNetDevBandwidthFormat: * @def: Data source * @buf: Buffer to print to * diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index c687427..30f9889 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -34,8 +34,7 @@ int virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, int net_type) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevBandwidthFormat(virNetDevBandwidthPtr def, - virBufferPtr buf) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + virBufferPtr buf); void virDomainClearNetBandwidth(virDomainObjPtr vm) ATTRIBUTE_NONNULL(1); -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 05/22] conf: Remove NONNULL(1) for virDomainNumaGetNodeCount

Since the code checks and handles a NULL 'numa' parameter, remove the NONNULL from the prototype. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/numa_conf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 05529ba..b6a5354 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -85,8 +85,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, virBitmapPtr *retNodeset, int cellid); -size_t virDomainNumaGetNodeCount(virDomainNumaPtr numa) - ATTRIBUTE_NONNULL(1); +size_t virDomainNumaGetNodeCount(virDomainNumaPtr numa); + virBitmapPtr virDomainNumaGetNodeCpumask(virDomainNumaPtr numa, size_t node) ATTRIBUTE_NONNULL(1); -- 2.9.3

Peter Krempa

9:52 a.m.

New subject: [libvirt] [PATCH 05/22] conf: Remove NONNULL(1) for virDomainNumaGetNodeCount

On Wed, Mar 22, 2017 at 10:21:18 -0400, John Ferlan wrote:

...

ACK

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 06/22] network: Remove null newBandwidth check from networkBandwidthUpdate

The prototype requires a NONNULL argument and the only caller passes in a non-null parameter. Besides the "else if" condition would deref it anyway. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/network/bridge_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a753cd7..d975092 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5468,7 +5468,7 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, /* Okay, there are three possible scenarios: */ if (ifaceBand && ifaceBand->in && ifaceBand->in->floor && - newBandwidth && newBandwidth->in && newBandwidth->in->floor) { + newBandwidth->in && newBandwidth->in->floor) { /* Either we just need to update @floor .. */ if (virNetDevBandwidthUpdateRate(network->def->bridge, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 07/22] qemu: Remove non null 'vm' check from qemuMonitorOpen

The prototype requires not passing a NULL in the parameter and the callers all would fail far before this code would fail if 'vm' was NULL, so just remove the check. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/qemu/qemu_monitor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 79da472..8218029 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -910,8 +910,7 @@ qemuMonitorOpen(virDomainObjPtr vm, case VIR_DOMAIN_CHR_TYPE_UNIX: hasSendFD = true; if ((fd = qemuMonitorOpenUnix(config->data.nix.path, - vm ? vm->pid : 0, - timeout)) < 0) + vm->pid, timeout)) < 0) return NULL; break; -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 08/22] qemu: Remove NONNULL(1) for qemu_monitor prototypes

The 'mon' argument validity is checked in the QEMU_CHECK_MONITOR for the following functions, so they don't need the NONNULL on their prototype: qemuMonitorUpdateVideoMemorySize qemuMonitorUpdateVideoVram64Size qemuMonitorGetAllBlockStatsInfo qemuMonitorBlockStatsUpdateCapacity Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/qemu/qemu_monitor.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cf79424..888a03d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -281,11 +281,11 @@ void qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options) int qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, virDomainVideoDefPtr video, const char *videoName) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorUpdateVideoVram64Size(qemuMonitorPtr mon, virDomainVideoDefPtr video, const char *videoName) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, const char *cmd, int scm_fd, @@ -497,12 +497,12 @@ struct _qemuBlockStats { int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats, bool backingChain) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, virHashTablePtr stats, bool backingChain) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorBlockResize(qemuMonitorPtr mon, const char *dev_name, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 09/22] util: Change return argument for virBitmapParseUnlimited

Rather than returning an int and a *bitmap pointer, just return and check a NULL bitmap pointer Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virbitmap.c | 26 ++++++++++++-------------- src/util/virbitmap.h | 7 +++---- tests/testutils.c | 2 +- tests/virbitmaptest.c | 2 +- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 1b47d74..d263d73 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -545,7 +545,6 @@ virBitmapParse(const char *str, /** * virBitmapParseUnlimited: * @str: points to a string representing a human-readable bitmap - * @bitmap: a bitmap created from @str * * This function is the counterpart of virBitmapFormat. This function creates * a bitmap, in which bits are set according to the content of @str. @@ -556,20 +555,20 @@ virBitmapParse(const char *str, * to set, and ^N, which means to unset the bit, and N-M for ranges of bits * to set. * - * Returns 0 on success, or -1 in case of error. + * Returns @bitmap on success, or NULL in case of error */ -int -virBitmapParseUnlimited(const char *str, - virBitmapPtr *bitmap) +virBitmapPtr +virBitmapParseUnlimited(const char *str) { + virBitmapPtr bitmap; bool neg = false; const char *cur = str; char *tmp; size_t i; int start, last; - if (!(*bitmap = virBitmapNewEmpty())) - return -1; + if (!(bitmap = virBitmapNewEmpty())) + return NULL; if (!str) goto error; @@ -605,10 +604,10 @@ virBitmapParseUnlimited(const char *str, if (*cur == ',' || *cur == 0) { if (neg) { - if (virBitmapClearBitExpand(*bitmap, start) < 0) + if (virBitmapClearBitExpand(bitmap, start) < 0) goto error; } else { - if (virBitmapSetBitExpand(*bitmap, start) < 0) + if (virBitmapSetBitExpand(bitmap, start) < 0) goto error; } } else if (*cur == '-') { @@ -626,7 +625,7 @@ virBitmapParseUnlimited(const char *str, cur = tmp; for (i = start; i <= last; i++) { - if (virBitmapSetBitExpand(*bitmap, i) < 0) + if (virBitmapSetBitExpand(bitmap, i) < 0) goto error; } @@ -644,14 +643,13 @@ virBitmapParseUnlimited(const char *str, } } - return 0; + return bitmap; error: virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse bitmap '%s'"), str); - virBitmapFree(*bitmap); - *bitmap = NULL; - return -1; + virBitmapFree(bitmap); + return NULL; } /** diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 58e0ee6..3ba40ae 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -94,10 +94,9 @@ virBitmapParseSeparator(const char *str, char terminator, virBitmapPtr *bitmap, size_t bitmapSize); -int -virBitmapParseUnlimited(const char *str, - virBitmapPtr *bitmap) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +virBitmapPtr +virBitmapParseUnlimited(const char *str) + ATTRIBUTE_NONNULL(1); virBitmapPtr virBitmapNewCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1); diff --git a/tests/testutils.c b/tests/testutils.c index a596a83..13eff9e 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -926,7 +926,7 @@ int virTestMain(int argc, } if ((testRange = getenv("VIR_TEST_RANGE")) != NULL) { - if (virBitmapParseUnlimited(testRange, &testBitmap) < 0) { + if (!(testBitmap = virBitmapParseUnlimited(testRange))) { fprintf(stderr, "Cannot parse range %s\n", testRange); return EXIT_FAILURE; } diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 3ee07ff..e007efc 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -650,7 +650,7 @@ test12(const void *opaque ATTRIBUTE_UNUSED) TEST_MAP(151, "128"); virBitmapFree(map); - if (virBitmapParseUnlimited("34,1023", &map) < 0) + if (!(map = virBitmapParseUnlimited("34,1023"))) goto cleanup; TEST_MAP(1024, "34,1023"); -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 10/22] util: Remove NONNULL(1) for virBitmapParseUnlimited

The code checks and handles a NULL 'str', so just remove the NONNULL. Update the error message to add the NULLSTR() around 'str' also. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virbitmap.c | 2 +- src/util/virbitmap.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index d263d73..eac63d9 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -647,7 +647,7 @@ virBitmapParseUnlimited(const char *str) error: virReportError(VIR_ERR_INVALID_ARG, - _("Failed to parse bitmap '%s'"), str); + _("Failed to parse bitmap '%s'"), NULLSTR(str)); virBitmapFree(bitmap); return NULL; } diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 3ba40ae..36282af 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -95,8 +95,7 @@ virBitmapParseSeparator(const char *str, virBitmapPtr *bitmap, size_t bitmapSize); virBitmapPtr -virBitmapParseUnlimited(const char *str) - ATTRIBUTE_NONNULL(1); +virBitmapParseUnlimited(const char *str); virBitmapPtr virBitmapNewCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1); -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 11/22] util: Remove NONNULL(2, 3) for virHostdevReAttachPCIDevices

The called function uses a STRNEQ_NULLABLE anyway for both 'drv_name' and 'dom_name', so no need. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virhostdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 1202136..4ce5c4d 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -103,7 +103,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, virDomainHostdevDefPtr *hostdevs, int nhostdevs, const char *oldStateDir) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1); void virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, const char *drv_name, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 12/22] util: Remove NONNULL(2, 3) for virHostdevReAttachUSBDevices

The comparison code used STREQ_NULLABLE anyway for both 'drv_name' and 'dom_name', so no need. Add a NULLSTR on the 'dom_name' too. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virhostdev.c | 2 +- src/util/virhostdev.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 86ca8e0..c424a97 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1624,7 +1624,7 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr, if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) { VIR_WARN("Unable to reattach USB device %03d.%03d on domain %s", - usbsrc->bus, usbsrc->device, dom_name); + usbsrc->bus, usbsrc->device, NULLSTR(dom_name)); continue; } diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 4ce5c4d..76039bb 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -110,7 +110,7 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, const char *dom_name, virDomainHostdevDefPtr *hostdevs, int nhostdevs) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1); void virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, const char *drv_name, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 13/22] util: Remove NONNULL(2, 3) for virHostdevReAttachSCSIVHostDevices

The comparison code used STREQ_NULLABLE anyway for both 'drv_name' and 'dom_name', so no need. Add a NULLSTR on the 'dom_name' too. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virhostdev.c | 2 +- src/util/virhostdev.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index c424a97..fc0ebcb 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1756,7 +1756,7 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr mgr, if (!(host = virSCSIVHostDeviceNew(hostsrc->wwpn))) { VIR_WARN("Unable to reattach SCSI_host device %s on domain %s", - hostsrc->wwpn, dom_name); + hostsrc->wwpn, NULLSTR(dom_name)); virObjectUnlock(mgr->activeSCSIVHostHostdevs); return; } diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 76039bb..43ba705 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -124,7 +124,7 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr hostdev_mgr, const char *dom_name, virDomainHostdevDefPtr *hostdevs, int nhostdevs) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1); int virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, virDomainHostdevDefPtr *hostdevs, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 14/22] util: Remove NONNULL(1) for virHostdevPrepareDomainDevices

Since the code checks 'mgr == NULL' anyway, no need for the prototype to have the NONNULL arg check. Also add an error message to indicate what the failure is so that there isn't a failed for some reason error. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virhostdev.c | 5 ++++- src/util/virhostdev.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index fc0ebcb..a75d108 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1873,8 +1873,11 @@ virHostdevPrepareDomainDevices(virHostdevManagerPtr mgr, if (!def->nhostdevs) return 0; - if (mgr == NULL) + if (!mgr) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no host device manager defined")); return -1; + } if (flags & VIR_HOSTDEV_SP_PCI) { if (virHostdevPreparePCIDevices(mgr, driver, diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 43ba705..7ee0b43 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -157,7 +157,7 @@ virHostdevPrepareDomainDevices(virHostdevManagerPtr mgr, const char *driver, virDomainDefPtr def, unsigned int flags) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); void virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr, const char *driver, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 15/22] util: Remove NONNULL(1) for virHostdevReAttachDomainDevices

Since the function handles a NULL 'mgr' condition, no need for the NONNULL Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virhostdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 7ee0b43..45d1c2e 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -164,7 +164,7 @@ virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr, virDomainDefPtr def, unsigned int flags, const char *oldStateDir) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); bool virHostdevIsSCSIDevice(virDomainHostdevDefPtr hostdev) ATTRIBUTE_NONNULL(1); -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 16/22] util: Remove NONNULL(1) for virNetDevOpenvswitchSetMigrateData

The code checks and handles a NULL 'migrate', so no need for NONNULL Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virnetdevopenvswitch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 4f62be1..021eba9 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -51,7 +51,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 17/22] util: Remove NONNULL(1, 3, 4) from virTypedParamsFilter

The API checks each parameter for NULL anyway and would error, so need to add NONNULL on prototype. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virtypedparam.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 7ab143f..59d5fbc 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -79,8 +79,7 @@ virTypedParamsFilter(virTypedParameterPtr params, int nparams, const char *name, virTypedParameterPtr **ret) - ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4); + ATTRIBUTE_RETURN_CHECK; int virTypedParameterAssign(virTypedParameterPtr param, const char *name, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 18/22] util: Remove NONNULL(2) for virNetDevBandwidthPlug

Since the code checks and handles a NULL 'net_bandwidth' parameter, so no need for NONNNULL. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virnetdevbandwidth.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h index bceac2e..64f3537 100644 --- a/src/util/virnetdevbandwidth.h +++ b/src/util/virnetdevbandwidth.h @@ -59,8 +59,7 @@ int virNetDevBandwidthPlug(const char *brname, const virMacAddr *ifmac_ptr, virNetDevBandwidthPtr bandwidth, unsigned int id) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; int virNetDevBandwidthUnplug(const char *brname, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 19/22] util: Remove NONNULL(1) for virNetDevOpenvswitchGetVhostuserIfname

Since the code checks and handles a NULL 'path', no need for the NONNULL Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virnetdevopenvswitch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 021eba9..7380a2d 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -59,6 +59,6 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, int virNetDevOpenvswitchGetVhostuserIfname(const char *path, char **ifname) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */ -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 20/22] util: Remove NONNULL's for virNetDevVPortProfile[Associate|Disassociate]

The source code will check for NULL arguments for 'macvtap_macaddr' and 'vmuuid', so no need for the NONNULL in the prototypes. Following the stack for both arguments to virNetDevVPortProfileOpSetLink also shows called functions would handle a NULL value. Additionally, modified the prototype to use the same 'macvtap_macaddr' name as the source code for consistency. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virnetdevvportprofile.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index dc3e643..b706c41 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -95,23 +95,21 @@ int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result, int virNetDevVPortProfileAssociate(const char *ifname, const virNetDevVPortProfile *virtPort, - const virMacAddr *macaddr, + const virMacAddr *macvtap_macaddr, const char *linkdev, int vf, const unsigned char *vmuuid, virNetDevVPortProfileOp vmOp, bool setlink_only) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6) - ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; int virNetDevVPortProfileDisassociate(const char *ifname, const virNetDevVPortProfile *virtPort, - const virMacAddr *macaddr, + const virMacAddr *macvtap_macaddr, const char *linkdev, int vf, virNetDevVPortProfileOp vmOp) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) - ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_NETDEV_VPORT_PROFILE_H__ */ -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 21/22] util: Remove NONNULL(1) for virNetDevMacVLanDeleteWithVPortProfile

Since the source code checks 'ifname' for NULL before using, the prototype doesn't need the NONNULL Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virnetdevmacvlan.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index 9a85a65..c40f23e 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -88,7 +88,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, int mode, virNetDevVPortProfilePtr virtPortProfile, char *stateDir) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6) ATTRIBUTE_RETURN_CHECK; int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname, -- 2.9.3

John Ferlan

9:21 a.m.

New subject: [libvirt] [PATCH 22/22] cpu: Remove NONNULL(1) for cpuBaseline

Since the code checks and handles a NULL 'cpus' anyway, so no need for the NONNULL. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/cpu/cpu.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 7d6d3e9..d23409a 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -195,8 +195,7 @@ cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, unsigned int nmodels, - unsigned int flags) - ATTRIBUTE_NONNULL(1); + unsigned int flags); int virCPUUpdate(virArch arch, -- 2.9.3

Martin Kletzander

10:15 a.m.

On Wed, Mar 22, 2017 at 10:21:13AM -0400, John Ferlan wrote:

...

I was using that and I had some issues that were about more than just a build issue. Trying it now, it cleanly says it just needs some fixes for the NUNNULL parameters. That's nice. With optimizations this series might even fix things, let's see =) ACK to this series.

2802

days inactive

2802

days old

devel@lists.libvirt.org

Manage subscription

25 comments

3 participants

tags (0)

participants (3)

John Ferlan
Martin Kletzander
Peter Krempa

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] [PATCH 00/22] Resolve Coverity and build issues