On 02/14/2011 04:37 PM, Eric Blake wrote:

clang complained that STREQ(group->controllers[i].mountPoint,...) was a NULL dereference when i==VIR_CGROUP_CONTROLLER_CPUSET, because it assumes the worst about virCgroupPathOfController. Marking the argument const doesn't yet have an effect, per this clang bug: http://llvm.org/bugs/show_bug.cgi?id=7758

So, we use sa_assert, which was designed to shut up false positives from tools like clang.

* src/util/cgroup.c (virCgroupMakeGroup): Teach clang that there is no NULL dereference. ---

I'm including enough context to show the STREQ that clang complained about.

And yes, I'm plowing through clang reports right now - there were less than 20, so it seemed worth tackling before 0.8.8.

This one is a one-liner fix (tested by re-running clang and no longer seeing a false positive, and sa_assert() is a no-op for gcc compilation), but I'd rather get an ACK before pushing.

src/util/cgroup.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/util/cgroup.c b/src/util/cgroup.c index de1fd8e..47c4633 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -496,24 +496,27 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,

VIR_DEBUG("Make group %s", group->path); for (i = 0 ; i< VIR_CGROUP_CONTROLLER_LAST ; i++) { char *path = NULL;

/* Skip over controllers that aren't mounted */ if (!group->controllers[i].mountPoint) continue;

rc = virCgroupPathOfController(group, i, "",&path); if (rc< 0) return rc; + /* As of Feb 2011, clang can't see that the above function + * call did not modify group. */ + sa_assert(group->controllers[i].mountPoint);

VIR_DEBUG("Make controller %s", path); if (access(path, F_OK) != 0) { if (!create || mkdir(path, 0755)< 0) { rc = -errno; VIR_FREE(path); break; } if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL&& (i == VIR_CGROUP_CONTROLLER_CPUSET || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) {

ACK.