Re: [libvirt] [PATCH] lxcFreezeContainer: avoid test-after-deref of never-NULL pointer

17 May 2010

      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);
            }