Re: [LTP] [PATCH 4/7] openposix: sem_timedwait/11-1: Fix

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Martin Doucha <mdoucha@suse.cz> writes:

> Hi,
> small suggestion below, otherwise it looks good.
>
> Reviewed-by: Martin Doucha <mdoucha@suse.cz>
>
> On 20. 06. 22 11:21, Cyril Hrubis wrote:
>> Actually run both of the cases (valid timeout and invalid timeout).
>> 
>> The timeout is not actually invalid, but rather in the past, which is
>> important to test as the system has to try to lock the semaphore first
>> and only if that fails it should check the timeout.
>> 
>> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>> ---
>>  .../conformance/interfaces/sem_timedwait/11-1.c          | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
>> index f87afaa43..663edd836 100644
>> --- a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
>> +++ b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
>> @@ -24,7 +24,7 @@
>>  #include "posixtest.h"
>>  
>>  #define TIMEOUT 2
>> -#define INVALIDTIMEOUT -2
>> +#define NEGATIVETIMEOUT -2
>>  #define TEST "11-1"
>>  #define FUNCTION "sem_timedwait"
>>  #define ERROR_PREFIX "unexpected error: " FUNCTION " " TEST ": "
>> @@ -45,7 +45,7 @@ int main(void)
>>  			ts[i].tv_sec = time(NULL) + TIMEOUT;
>>  			ts[i].tv_nsec = 0;
>>  		} else if (i == 1) {
>> -			ts[i].tv_sec = time(NULL) + INVALIDTIMEOUT;
>> +			ts[i].tv_sec = time(NULL) + NEGATIVETIMEOUT;
>>  			ts[i].tv_nsec = 0;
>>  		}
>>  		/* Lock Semaphore */
>> @@ -63,13 +63,14 @@ int main(void)
>>  
>>  		/* Checking if the value of the Semaphore decremented by one */
>>  		if ((val[i] == 0) && (sts[i] == 0)) {
>> -			puts("TEST PASSED");
>>  			sem_destroy(&mysemp[i]);
>
> It'd be better to move sem_destroy() above the condition. See code
> example at the end.
>
>> -			return PTS_PASS;
>>  		} else {
>>  			puts("TEST FAILED");
>>  			sem_destroy(&mysemp[i]);
>>  			return PTS_FAIL;
>>  		}
>>  	}
>> +
>> +	puts("TEST PASSED");
>> +	return PTS_PASS;
>>  }
>
> ...
>
> 	sem_destroy(&mysemp[i]);

And with that also:

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>
> 	/* Checking if the value of the Semaphore decremented by one */
> 	if ((val[i] != 0) || (sts[i] != 0)) {
> 		puts("TEST FAILED");
> 		return PTS_FAIL;
> 	}
>
> 	puts("TEST PASSED");
> 	return PTS_PASS;
> }
>
> -- 
> Martin Doucha   mdoucha@suse.cz
> QA Engineer for Software Maintenance
> SUSE LINUX, s.r.o.
> CORSO IIa
> Krizikova 148/34
> 186 00 Prague 8
> Czech Republic


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp