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

On 10/27/2011 05:26 PM, Daniel P. Berrange wrote: >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))) { If the previous VIR_REALLOC_N can't make sure 'mounts' is non null, whether we need to check 'mounts' to avoid dereferencing a NULL pointer in here again, for instance: if (!mounts || !(mounts[nmounts++] = strdup(mntent.mnt_dir))) {