Simplify semctl03 testcases by dropping the variadicness from test 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.
Signed-off-by: Beata Michalska beata.michalska@arm.com --- v2: - restore test wrappers but drop variadicness from those
.../kernel/syscalls/ipc/semctl/semctl03.c | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl03.c b/testcases/kernel/syscalls/ipc/semctl/semctl03.c index f9f335e4a..dacd929b6 100644 --- a/testcases/kernel/syscalls/ipc/semctl/semctl03.c +++ b/testcases/kernel/syscalls/ipc/semctl/semctl03.c @@ -21,28 +21,16 @@ 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, ...) +static int libc_semctl(int semid, int semnum, int cmd, union semun arg) { - 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, ...) +static int sys_semctl(int semid, int semnum, int cmd, union semun arg) { - 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); }
@@ -61,7 +49,7 @@ static struct tcases {
static struct test_variants { - int (*semctl)(int semid, int semnum, int cmd, ...); + int (*semctl)(int semid, int semnum, int cmd, union semun arg); char *desc; } variants[] = { { .semctl = libc_semctl, .desc = "libc semctl()"}, @@ -74,13 +62,14 @@ static void verify_semctl(unsigned int n) { struct tcases *tc = &tests[n]; struct test_variants *tv = &variants[tst_variant]; + union semun arg = {.buf = *(tc->buf)};
if (tc->error == EFAULT && tv->semctl == libc_semctl) { tst_res(TCONF, "EFAULT is skipped for libc variant"); return; }
- TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, *(tc->buf)), + TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, arg), tc->error, "semctl() with %s", tc->message); }
On 20/12/2022 11:12, Beata Michalska wrote:
Simplify semctl03 testcases by dropping the variadicness from test 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.
Signed-off-by: Beata Michalska beata.michalska@arm.com
Looks great to me, I feel that Kevin had a good point.
Téo
v2:
- restore test wrappers but drop variadicness from those
.../kernel/syscalls/ipc/semctl/semctl03.c | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl03.c b/testcases/kernel/syscalls/ipc/semctl/semctl03.c index f9f335e4a..dacd929b6 100644 --- a/testcases/kernel/syscalls/ipc/semctl/semctl03.c +++ b/testcases/kernel/syscalls/ipc/semctl/semctl03.c @@ -21,28 +21,16 @@ 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, ...) +static int libc_semctl(int semid, int semnum, int cmd, union semun arg) {
- 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, ...) +static int sys_semctl(int semid, int semnum, int cmd, union semun arg) {
- 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); }
@@ -61,7 +49,7 @@ static struct tcases { static struct test_variants {
- int (*semctl)(int semid, int semnum, int cmd, ...);
- int (*semctl)(int semid, int semnum, int cmd, union semun arg); char *desc; } variants[] = { { .semctl = libc_semctl, .desc = "libc semctl()"},
@@ -74,13 +62,14 @@ static void verify_semctl(unsigned int n) { struct tcases *tc = &tests[n]; struct test_variants *tv = &variants[tst_variant];
- union semun arg = {.buf = *(tc->buf)};
if (tc->error == EFAULT && tv->semctl == libc_semctl) { tst_res(TCONF, "EFAULT is skipped for libc variant"); return; }
- TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, *(tc->buf)),
- TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, arg), tc->error, "semctl() with %s", tc->message); }
On 20/12/2022 13:08, Teo Couprie Diaz wrote:
On 20/12/2022 11:12, Beata Michalska wrote:
Simplify semctl03 testcases by dropping the variadicness from test 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.
Signed-off-by: Beata Michalska beata.michalska@arm.com
Looks great to me, I feel that Kevin had a good point.
LGTM too, ideally with the wording fix I mentioned in v1.
Kevin
Téo
v2: - restore test wrappers but drop variadicness from those
.../kernel/syscalls/ipc/semctl/semctl03.c | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl03.c b/testcases/kernel/syscalls/ipc/semctl/semctl03.c index f9f335e4a..dacd929b6 100644 --- a/testcases/kernel/syscalls/ipc/semctl/semctl03.c +++ b/testcases/kernel/syscalls/ipc/semctl/semctl03.c @@ -21,28 +21,16 @@ 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, ...) +static int libc_semctl(int semid, int semnum, int cmd, union semun arg) { - 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, ...) +static int sys_semctl(int semid, int semnum, int cmd, union semun arg) { - 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); } @@ -61,7 +49,7 @@ static struct tcases { static struct test_variants { - int (*semctl)(int semid, int semnum, int cmd, ...); + int (*semctl)(int semid, int semnum, int cmd, union semun arg); char *desc; } variants[] = { { .semctl = libc_semctl, .desc = "libc semctl()"}, @@ -74,13 +62,14 @@ static void verify_semctl(unsigned int n) { struct tcases *tc = &tests[n]; struct test_variants *tv = &variants[tst_variant]; + union semun arg = {.buf = *(tc->buf)}; if (tc->error == EFAULT && tv->semctl == libc_semctl) { tst_res(TCONF, "EFAULT is skipped for libc variant"); return; } - TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, *(tc->buf)), + TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, arg), tc->error, "semctl() with %s", tc->message); }
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
linux-morello-ltp@op-lists.linaro.org
-
Beata Michalska -
Kevin Brodsky -
Teo Couprie Diaz