On 19/12/2022 19:32, Beata Michalska wrote:

Simplify semctl03 testcases by dropping the variadic wrappers which might spin out of control if not handled correctly. Also, it seems that the level of indirection for the valid union semun argument went slightly too far, resulting in passing over an address of the above mentioned union instead it's value, so fix that one as well.

s/instead it's/instead of its/

That's a good catch BTW, worth upstreaming.

Signed-off-by: Beata Michalska beata.michalska@arm.com

.../kernel/syscalls/ipc/semctl/semctl03.c | 50 +++++++------------ 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl03.c b/testcases/kernel/syscalls/ipc/semctl/semctl03.c index f9f335e4a..4686b366b 100644 --- a/testcases/kernel/syscalls/ipc/semctl/semctl03.c +++ b/testcases/kernel/syscalls/ipc/semctl/semctl03.c @@ -16,35 +16,14 @@ #include "lapi/sem.h" #include "libnewipc.h" #include "lapi/syscalls.h" +#include <stdbool.h> static int sem_id = -1; static int bad_id = -1; static struct semid_ds sem_ds; -static union semun sem_un = {.buf = &sem_ds}; -static void *semds_ptr = &sem_un; +static void *semds_ptr = &sem_ds; static void *bad_ptr; -static union semun arg = {0};

-static int libc_semctl(int semid, int semnum, int cmd, ...) -{

• va_list ap;
• va_start(ap, cmd);
• arg = va_arg(ap, union semun);
• va_end(ap);
• return semctl(semid, semnum, cmd, arg);

-}

-static int sys_semctl(int semid, int semnum, int cmd, ...) -{

• va_list ap;
• va_start(ap, cmd);
• arg = va_arg(ap, union semun);
• va_end(ap);
• return tst_syscall(__NR_semctl, semid, semnum, cmd, arg);

-} static struct tcases { int *sem_id; @@ -61,12 +40,12 @@ static struct tcases { static struct test_variants {

• int (*semctl)(int semid, int semnum, int cmd, ...);

• bool syscall_wrapper; char *desc;

} variants[] = {

• { .semctl = libc_semctl, .desc = "libc semctl()"},

• { true, .desc = "libc semctl()"},

#if (__NR_sys_semctl != __LTP__NR_INVALID_SYSCALL)

• { .semctl = sys_semctl, .desc = "__NR_semctl syscall"},

• { false, .desc = "__NR_semctl syscall"},

#endif }; @@ -74,14 +53,19 @@ static void verify_semctl(unsigned int n) { struct tcases *tc = &tests[n]; struct test_variants *tv = &variants[tst_variant];

• if (tc->error == EFAULT && tv->semctl == libc_semctl) {

• tst_res(TCONF, "EFAULT is skipped for libc variant");
  

• return;
  

• union semun arg = {.buf = *(tc->buf)};
• if (tv->syscall_wrapper) {

• if (tc->error == EFAULT)
  

• 	tst_res(TCONF, "EFAULT is skipped for libc variant");
  

• else
  

• 	TST_EXP_FAIL(semctl(*(tc->sem_id), 0, tc->ipc_cmd, arg),
  

• 			    tc->error, "semctl() with %s", tc->message);
  

• } else {

• TST_EXP_FAIL(tst_syscall(__NR_semctl, *(tc->sem_id), 0,
  

• 			 tc->ipc_cmd, arg), tc->error,
  

• 			 "semctl() with %s", tc->message);
  

  }

• TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, *(tc->buf)),

• tc->error, "semctl() with %s", tc->message);
  

I'm thinking, couldn't we keep the original approach mostly unchanged, and simply make libc_semctl/sys_semctl non-variadic? We don't need them to be. That would solve our original issue, and avoid duplicating the call in this function. Likely upstream will like a smaller diff too :)

Kevin

} static void setup(void)