Re: [libvirt] [PATCH] lxc: remove redundant check

On Thu, Oct 27, 2011 at 04:33:50PM +0800, Alex Jia wrote: > On 10/27/2011 04:20 PM, Daniel P. Berrange wrote: >> On Thu, Oct 27, 2011 at 03:18:01PM +0800, ajia(a)redhat.com wrote: >>> From: Alex Jia&lt;ajia(a)redhat.com&gt; >>> >>> Detected by cppcheck, the previous function VIR_REALLOC_N(mounts, nmounts+1) >>> can make sure mounts is a none null value, so it is redundant to check if >>> mounts is null. >> VIR_REALLOC_N is only executed inside the loop. Although it is unlikely, >> in theory there can be zero iterations of the loop, in which case >> 'mounts' will be NULL at the point qsort is called. >> >>> * src/lxc/lxc_container.c: remove redundant check. >>> >>> Signed-off-by: Alex Jia&lt;ajia(a)redhat.com&gt; >>> --- >>> src/lxc/lxc_container.c | 5 ++--- >>> 1 files changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c >>> index 06ccf7e..f4879c3 100644 >>> --- a/src/lxc/lxc_container.c >>> +++ b/src/lxc/lxc_container.c >>> @@ -879,9 +879,8 @@ static int lxcContainerUnmountOldFS(void) >>> } >>> endmntent(procmnt); >>> >>> - if (mounts) >>> - qsort(mounts, nmounts, sizeof(mounts[0]), >>> - lxcContainerChildMountSort); >>> + qsort(mounts, nmounts, sizeof(mounts[0]), >>> + lxcContainerChildMountSort); >>> >>> for (i = 0 ; i< nmounts ; i++) { >>> VIR_DEBUG("Umount %s", mounts[i]); >> NACK, the check for NULL is correct >> >> >> Daniel > Hi Daniel, > My comment is wrong, but the following judgement should make sure > mounts is non null: > <snip> > if (!(mounts[nmounts++] = strdup(mntent.mnt_dir))) {

> endmntent(procmnt); > virReportOOMError(); > return -1; > } > </snip> That is ensuring that the pointer in the array element value is non-NULL. The "if (mounts) qsort(...)" check is against the array itself, rather than the element. Regards, Daniel