Jim Meyering wrote:
> Eric Blake wrote:
>> On 05/17/2010 11:22 AM, Jim Meyering wrote:
>>> This addresses another coverity-spotted "flaw".
>>> However, since "cgroup" is never NULL after that initial "if" stmt,
>>> the only penalty is that the useless cleanup test would make a reviewer
>>> try to figure out how cgroup could be NULL there.
>>
>> ACK.
>
> Thanks.
>
>>> cleanup:
>>> - if (cgroup)
>>> - virCgroupFree(&cgroup);
>>> + virCgroupFree(&cgroup);
>>
>> Is this something that the useless-if-before-free test could have
>> caught, rather than waiting for coverity?
>
> Very good idea.
> Here's a patch to do that.
> With this, "make sc_avoid_if_before_free" (part of make syntax-check)
> will now prevent any new useless checks.
> The above was the sole offender.
Actually, this is just the tip of the iceberg.
A couple minutes work have shown that there are many more:
Running this shows there are potentially at least 100
candidate functions:
aid free|grep '^vi'
Looking at the first few on the list, I've found that most are
indeed free-like:
N virBufferFreeAndReset
y virCPUDefFree
Y virCapabilitiesFree
y virCapabilitiesFreeGuest
y virCapabilitiesFreeGuestDomain
y virCapabilitiesFreeGuestFeature
y virCapabilitiesFreeGuestMachine
y virCapabilitiesFreeHostNUMACell
y virCapabilitiesFreeMachines
N virCapabilitiesFreeNUMAInfo (should probably fix)
y virCgroupFree
N virConfFree (diagnoses the "error")
y virConfFreeList
y virConfFreeValue
So far, most of the exceptions can be "fixed"
to also be free-like.
Using those few additions uncovered these:
src/conf/storage_conf.c: if (pool->newDef)
virStoragePoolDefFree(pool->newDef)
src/security/virt-aa-helper.c: if (ctl->caps)
virCapabilitiesFree(ctl->caps)
src/util/conf.c: if (cur->value) {
virConfFreeValue(cur->value);
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list