[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com wrote:
> >>From: Alex Jia<ajia redhat com>
> >>
> >>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<ajia redhat com>
> >>---
> >>  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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]