Re: [PATCH] lxc: remove redundant check

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

 



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@xxxxxxxxxx wrote:
From: Alex Jia<ajia@xxxxxxxxxx>

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@xxxxxxxxxx>
---
  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))) {

Thanks for your comment.

Alex

             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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]