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 *