| Message ID | 20220620092146.7604-5-chrubis@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | openposix: Fix 'no return in nonvoid function' warnings | expand |
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]); /* 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; }
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
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]); - return PTS_PASS; } else { puts("TEST FAILED"); sem_destroy(&mysemp[i]); return PTS_FAIL; } } + + puts("TEST PASSED"); + return PTS_PASS; }
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(-)