Re: [libvirt] [PATCH] lxcFreezeContainer: avoid test-after-deref of never-NULL pointer
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);
}