On 02/06/2014 05:36 PM, Pavel Hrdina wrote:
On 6.2.2014 16:48, Eric Blake wrote:
On 02/06/2014 08:18 AM, Pavel Hrdina wrote:
diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 994b300..8ff3b1d 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque) if (!dev) goto cleanup;
+ /* coverity[freed_arg] */ if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0 || virPCIDeviceDetach(dev, NULL, NULL) < 0) goto cleanup;
Is this something where an sa_assert() could do the trick in a more tool-agnostic manner?
I'm looking at the code again and probably the issue found by coverity could happen if the STRDUP fails inside of the virPCIDeviceSetStubDriver.
If STRDUP fails, then SetStubDriver should return -1 and we wouldn't be calling DeviceDetach. It would be nice if we could 'fix' virPCIDeviceSetStubDriver in a way coverity would understand (fixing both complaints, but still checking if we're not calling virKModLoad with a NULL argument). Does this patch make it any happier? Jan diff --git a/src/util/virpci.c b/src/util/virpci.c index c3d211f..88002c9 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1657,7 +1657,7 @@ int virPCIDeviceSetStubDriver(virPCIDevicePtr dev, const char *driver) { VIR_FREE(dev->stubDriver); - return driver ? VIR_STRDUP(dev->stubDriver, driver) : 0; + return VIR_STRDUP(dev->stubDriver, driver); } const char *