[libvirt] [PATCH] lxc: remove redundant check
Alex Jia
ajia at redhat.com
Thu Oct 27 09:54:20 UTC 2011
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 at redhat.com wrote:
>>>> From: Alex Jia<ajia at 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 at 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))) {
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
More information about the libvir-list
mailing list