| Message ID | 20210318162409.9871-1-mdoucha@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | syscalls/semctl09: Skip libc test if SEM_STAT_ANY not defined | expand |
Hi Martin, > The libc test variant should run only if system headers define SEM_STAT_ANY. > Skip it if we're falling back to the LAPI definition. Reviewed-by: Petr Vorel <pvorel@suse.cz> Thanks! ... > +#if !HAVE_DECL_SEM_STAT_ANY nit: I'd prefer #ifndef HAVE_DECL_SEM_STAT_ANY > + if (tst_variant == 1) > + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); > +#endif Although I understand why you want to quit only tests with root (only these fail), it's a bit confusing to test with user nobody and then quit the same testing with root. Kind regards, Petr
Hi Petr > Hi Martin, > >> The libc test variant should run only if system headers define SEM_STAT_ANY. >> Skip it if we're falling back to the LAPI definition. > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Thanks! > > ... >> +#if !HAVE_DECL_SEM_STAT_ANY > nit: I'd prefer > #ifndef HAVE_DECL_SEM_STAT_ANY +1 >> + if (tst_variant == 1) >> + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); >> +#endif > Although I understand why you want to quit only tests with root > (only these fail), it's a bit confusing to test with user nobody > and then quit the same testing with root. I don't get this. Martin only wants to skip libc test when undefined and it doesn't matter which user we use. > Kind regards, > Petr >
Hi Xu, > > > + if (tst_variant == 1) > > > + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); > > > +#endif > > Although I understand why you want to quit only tests with root > > (only these fail), it's a bit confusing to test with user nobody > > and then quit the same testing with root. > I don't get this. Martin only wants to skip libc test when undefined and it > doesn't matter which user we use. if (tst_variant == 1) tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); means: # /semctl09 tst_test.c:1289: TINFO: Timeout per run is 0h 05m 00s semctl09.c:76: TINFO: Test SYS_semctl syscall semctl09.c:141: TINFO: Test SEM_STAT_ANY with nobody user semctl09.c:163: TPASS: SEM_INFO returned valid index 19 to semid 19 semctl09.c:173: TPASS: Counted used = 1 semctl09.c:121: TPASS: semset_cnt = 1 semctl09.c:128: TPASS: sen_cnt = 2 semctl09.c:141: TINFO: Test SEM_STAT_ANY with root user semctl09.c:163: TPASS: SEM_INFO returned valid index 19 to semid 19 semctl09.c:173: TPASS: Counted used = 1 semctl09.c:121: TPASS: semset_cnt = 1 semctl09.c:128: TPASS: sen_cnt = 2 tst_test.c:1289: TINFO: Timeout per run is 0h 05m 00s semctl09.c:191: TCONF: libc does not support semctl(SEM_STAT_ANY) i.e. run first test with user nobody and skip the second. It's a bit confusing to test anything with SEM_STAT_ANY and then state TCONF: libc does not support semctl(SEM_STAT_ANY) :) Kind regards, Petr
Hi Petr > Hi Xu, > >>>> + if (tst_variant == 1) >>>> + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); >>>> +#endif >>> Although I understand why you want to quit only tests with root >>> (only these fail), it's a bit confusing to test with user nobody >>> and then quit the same testing with root. > >> I don't get this. Martin only wants to skip libc test when undefined and it >> doesn't matter which user we use. > > if (tst_variant == 1) > tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); > > means: > > # /semctl09 > tst_test.c:1289: TINFO: Timeout per run is 0h 05m 00s > semctl09.c:76: TINFO: Test SYS_semctl syscall > semctl09.c:141: TINFO: Test SEM_STAT_ANY with nobody user > semctl09.c:163: TPASS: SEM_INFO returned valid index 19 to semid 19 > semctl09.c:173: TPASS: Counted used = 1 > semctl09.c:121: TPASS: semset_cnt = 1 > semctl09.c:128: TPASS: sen_cnt = 2 > semctl09.c:141: TINFO: Test SEM_STAT_ANY with root user > semctl09.c:163: TPASS: SEM_INFO returned valid index 19 to semid 19 > semctl09.c:173: TPASS: Counted used = 1 > semctl09.c:121: TPASS: semset_cnt = 1 > semctl09.c:128: TPASS: sen_cnt = 2 > tst_test.c:1289: TINFO: Timeout per run is 0h 05m 00s > semctl09.c:191: TCONF: libc does not support semctl(SEM_STAT_ANY) > > i.e. run first test with user nobody and skip the second. > It's a bit confusing to test anything with SEM_STAT_ANY and then state > TCONF: libc does not support semctl(SEM_STAT_ANY) :) Thanks. I got it. I guess we can move this check into test_info as below: --- a/testcases/kernel/syscalls/ipc/semctl/semctl09.c +++ b/testcases/kernel/syscalls/ipc/semctl/semctl09.c @@ -77,6 +77,9 @@ static void test_info(void) break; case 1: tst_res(TINFO, "Test libc semctl()"); +#ifndef HAVE_DECL_SEM_STAT_ANY + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); +#endif break; Then output as below: # ./semctl09 tst_test.c:1291: TINFO: Timeout per run is 0h 05m 00s semctl09.c:76: TINFO: Test SYS_semctl syscall semctl09.c:144: TINFO: Test SEM_STAT_ANY with nobody user semctl09.c:167: TPASS: SEM_INFO returned valid index 34 to semid 34 semctl09.c:176: TPASS: Counted used = 1 semctl09.c:124: TPASS: semset_cnt = 1 semctl09.c:131: TPASS: sen_cnt = 2 semctl09.c:144: TINFO: Test SEM_STAT_ANY with root user semctl09.c:167: TPASS: SEM_INFO returned valid index 34 to semid 34 semctl09.c:176: TPASS: Counted used = 1 semctl09.c:124: TPASS: semset_cnt = 1 semctl09.c:131: TPASS: sen_cnt = 2 tst_test.c:1291: TINFO: Timeout per run is 0h 05m 00s semctl09.c:79: TINFO: Test libc semctl() semctl09.c:81: TCONF: libc does not support semctl(SEM_STAT_ANY) > > Kind regards, > Petr >
On 18. 03. 21 19:30, Petr Vorel wrote: > Hi Martin, > >> The libc test variant should run only if system headers define SEM_STAT_ANY. >> Skip it if we're falling back to the LAPI definition. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Thanks! > > ... >> +#if !HAVE_DECL_SEM_STAT_ANY > nit: I'd prefer > #ifndef HAVE_DECL_SEM_STAT_ANY That will not work. AC_CHECK_DECLS() will always define HAVE_DECL_SEM_STAT_ANY, either with the value of 0 (macro not defined) or 1 (defined). >> + if (tst_variant == 1) >> + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); >> +#endif > Although I understand why you want to quit only tests with root > (only these fail), it's a bit confusing to test with user nobody > and then quit the same testing with root. tst_variant has nothing to do with UID. tst_variant == 0 => run the test using tst_syscall(__NR_semctl) tst_variant == 1 => run the test again using libc semctl() But if you want to make the test output a little more clear, I could move the test_info() call at the end of setup() a few lines up before the new #if.
Hi Martin, > >> +#if !HAVE_DECL_SEM_STAT_ANY > > nit: I'd prefer > > #ifndef HAVE_DECL_SEM_STAT_ANY > That will not work. AC_CHECK_DECLS() will always define > HAVE_DECL_SEM_STAT_ANY, either with the value of 0 (macro not defined) > or 1 (defined). Correct, I'm sorry, thanks for catching my error. > >> + if (tst_variant == 1) > >> + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); > >> +#endif > > Although I understand why you want to quit only tests with root > > (only these fail), it's a bit confusing to test with user nobody > > and then quit the same testing with root. > tst_variant has nothing to do with UID. > tst_variant == 0 => run the test using tst_syscall(__NR_semctl) > tst_variant == 1 => run the test again using libc semctl() Again, correct, sorry for wrong report. > But if you want to make the test output a little more clear, I could > move the test_info() call at the end of setup() a few lines up before > the new #if. +1 Kind regards, Petr
diff --git a/configure.ac b/configure.ac index 37bafb93d..136d82d09 100644 --- a/configure.ac +++ b/configure.ac @@ -38,6 +38,7 @@ AC_PREFIX_DEFAULT(/opt/ltp) AC_CHECK_DECLS([IFLA_NET_NS_PID],,,[#include <linux/if_link.h>]) AC_CHECK_DECLS([MADV_MERGEABLE],,,[#include <sys/mman.h>]) AC_CHECK_DECLS([PR_CAPBSET_DROP, PR_CAPBSET_READ],,,[#include <sys/prctl.h>]) +AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>]) AC_CHECK_HEADERS_ONCE([ \ asm/ldt.h \ diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl09.c b/testcases/kernel/syscalls/ipc/semctl/semctl09.c index a933cff0e..fb7d49f3d 100644 --- a/testcases/kernel/syscalls/ipc/semctl/semctl09.c +++ b/testcases/kernel/syscalls/ipc/semctl/semctl09.c @@ -186,6 +186,11 @@ static void setup(void) nobody_uid = ltpuser->pw_uid; root_uid = 0; +#if !HAVE_DECL_SEM_STAT_ANY + if (tst_variant == 1) + tst_brk(TCONF, "libc does not support semctl(SEM_STAT_ANY)"); +#endif + sem_id = SAFE_SEMGET(IPC_PRIVATE, 2, IPC_CREAT | 0600); test_info(); }
The libc test variant should run only if system headers define SEM_STAT_ANY. Skip it if we're falling back to the LAPI definition. Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- configure.ac | 1 + testcases/kernel/syscalls/ipc/semctl/semctl09.c | 5 +++++ 2 files changed, 6 insertions(+)