[LTP] [PATCH v2] Clean up hugemmap02 testcase

[LTP] [PATCH v2] Clean up hugemmap02 testcase

[shuang.qiu]

From: Shuang Qiu <shuang.qiu@oracle.com>

* Remove continue in the loop because it skips the cleanup

* munmap() addr2 for 32-bit

* close nfildes fd in the end of loop

Signed-off-by: Shuang Qiu <shuang.qiu@oracle.com>
---
 testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c
index 1a44993..39032f5 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c
@@ -155,24 +155,20 @@ int main(int ac, char **av)
 		addr2 = mmap((void *)low_addr2, map_sz, PROT_READ | PROT_WRITE,
 			     MAP_SHARED, fildes, 0);
 #if __WORDSIZE == 64		/* 64-bit process */
-		if (addr2 == MAP_FAILED) {
+		if (addr2 == MAP_FAILED)
 			tst_resm(TFAIL | TERRNO, "huge mmap failed unexpectedly"
 				 " with %s (64-bit)", TEMPFILE);
-			close(fildes);
-			continue;
-		} else {
+		else {
 			tst_resm(TPASS, "huge mmap succeeded (64-bit)");
 		}
 #else /* 32-bit process */
 		if (addr2 == MAP_FAILED)
 			tst_resm(TFAIL | TERRNO, "huge mmap failed unexpectedly"
 				 " with %s (32-bit)", TEMPFILE);
-		else if (addr2 > 0) {
+		else if (addr2 > 0)
 			tst_resm(TCONF,
 				 "huge mmap failed to test the scenario");
-			close(fildes);
-			continue;
-		} else if (addr == 0)
+		else if (addr == 0)
 			tst_resm(TPASS, "huge mmap succeeded (32-bit)");
 #endif
 
@@ -183,13 +179,12 @@ int main(int ac, char **av)
 					 "munmap of addrlist[%d] failed", i);
 		}
 
-#if __WORDSIZE == 64
 		if (munmap(addr2, map_sz) == -1)
 			tst_brkm(TFAIL | TERRNO, NULL, "huge munmap failed");
-#endif
 		if (munmap(addr, page_sz) == -1)
 			tst_brkm(TFAIL | TERRNO, NULL, "munmap failed");
 
+		close(nfildes);
 		close(fildes);
 	}
 
-- 
1.7.9.5

[LTP] [PATCH v2] Clean up hugemmap02 testcase

[Cyril Hrubis]

Hi!
>  #if __WORDSIZE == 64		/* 64-bit process */
> -		if (addr2 == MAP_FAILED) {
> +		if (addr2 == MAP_FAILED)
>  			tst_resm(TFAIL | TERRNO, "huge mmap failed unexpectedly"
>  				 " with %s (64-bit)", TEMPFILE);
> -			close(fildes);
> -			continue;
> -		} else {
> +		else {
>  			tst_resm(TPASS, "huge mmap succeeded (64-bit)");
>  		}
>  #else /* 32-bit process */
>  		if (addr2 == MAP_FAILED)
>  			tst_resm(TFAIL | TERRNO, "huge mmap failed unexpectedly"
>  				 " with %s (32-bit)", TEMPFILE);
> -		else if (addr2 > 0) {
> +		else if (addr2 > 0)
>  			tst_resm(TCONF,
>  				 "huge mmap failed to test the scenario");
> -			close(fildes);
> -			continue;
> -		} else if (addr == 0)
> +		else if (addr == 0)
>  			tst_resm(TPASS, "huge mmap succeeded (32-bit)");

The LKML coding style says that we should add curly braces to both else
branches if they are around one of them. Or if the function call after
if spans across more lines. So please do not remove them.

Otherwise the change looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] Clean up hugemmap02 testcase

[Shuang Qiu]

On 02/10/2016 08:31 PM, Cyril Hrubis wrote:
> Hi!
>>   #if __WORDSIZE == 64		/* 64-bit process */
>> -		if (addr2 == MAP_FAILED) {
>> +		if (addr2 == MAP_FAILED)
>>   			tst_resm(TFAIL | TERRNO, "huge mmap failed unexpectedly"
>>   				 " with %s (64-bit)", TEMPFILE);
>> -			close(fildes);
>> -			continue;
>> -		} else {
>> +		else {
>>   			tst_resm(TPASS, "huge mmap succeeded (64-bit)");
>>   		}
>>   #else /* 32-bit process */
>>   		if (addr2 == MAP_FAILED)
>>   			tst_resm(TFAIL | TERRNO, "huge mmap failed unexpectedly"
>>   				 " with %s (32-bit)", TEMPFILE);
>> -		else if (addr2 > 0) {
>> +		else if (addr2 > 0)
>>   			tst_resm(TCONF,
>>   				 "huge mmap failed to test the scenario");
>> -			close(fildes);
>> -			continue;
>> -		} else if (addr == 0)
>> +		else if (addr == 0)
>>   			tst_resm(TPASS, "huge mmap succeeded (32-bit)");
> The LKML coding style says that we should add curly braces to both else
> branches if they are around one of them. Or if the function call after
> if spans across more lines. So please do not remove them.
Thanks for review and correct me it.
Will send a new one.

Thanks
Shuang
>
> Otherwise the change looks good.
>