Re: [LTP] [PATCH 2/2] setitimer03: convert to new API

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Li Wang <liwang@redhat.com> writes:

> On Thu, Oct 20, 2022 at 5:30 PM Li Wang <liwang@redhat.com> wrote:
>
>  Richard Palethorpe <rpalethorpe@suse.de> wrote:
>   
>  > -static struct itimerval *value;
>  > +static struct itimerval *value, *ovalue;
>  > +
>  > +static struct tcase {
>  > +       int which;
>  > +       struct itimerval **val;
>  > +       struct itimerval **oval;
>  > +       int exp_errno;
>
>  There is a whitespace error here (see checkpatch/make check)
>
>  yes, thanks.
>
>   
>  
>  > +} tcases[] = {
>  > +       {ITIMER_REAL,    &value, &ovalue, EFAULT},
>  > +       {ITIMER_VIRTUAL, &value, &ovalue, EFAULT},
>  > +       {-ITIMER_PROF,   &value, &ovalue, EINVAL},
>  > +};
>
>  Why do we need value and ovalue in the struct?
>
>  Becuase it does not allow parsing an invalid pointer address
>  from a structure, we have to give a valid address which pointer
>  to save an invalid address. Otherwise segement fault will
>  be hit in execution.
>
> On the other side, it also does not allow to initializer element
> is not constant in structure. So the two-level pointer is only the
> way I can make all things comprised here.

I'm not sure what you mean and I don't understand why we need this
struct at all. The following works and produces better output:

@@ -20,17 +20,6 @@

 static struct itimerval *value, *ovalue;

-static struct tcase {
-       int which;
-       struct itimerval **val;
-       struct itimerval **oval;
-       int exp_errno;
-} tcases[] = {
-       {ITIMER_REAL,    &value, &ovalue, EFAULT},
-       {ITIMER_VIRTUAL, &value, &ovalue, EFAULT},
-       {-ITIMER_PROF,   &value, &ovalue, EINVAL},
-};
-
 static int sys_setitimer(int which, void *new_value, void *old_value)
 {
 	return tst_syscall(__NR_setitimer, which, new_value, old_value);
@@ -38,13 +27,17 @@ static int sys_setitimer(int which, void *new_value, void *old_value)

 static void verify_setitimer(unsigned int i)
 {
-        struct tcase *tc = &tcases[i];
-
-	if (tc->exp_errno == EFAULT)
-		*(tc->oval) = (struct itimerval *)-1;
-
-	TST_EXP_FAIL(sys_setitimer(tc->which, *(tc->val), *(tc->oval)),
-			tc->exp_errno);
+	switch (i) {
+	case 0:
+		TST_EXP_FAIL(sys_setitimer(ITIMER_REAL, value, (void *)-1), EFAULT);
+		break;
+	case 1:
+		TST_EXP_FAIL(sys_setitimer(ITIMER_VIRTUAL, value, (void *)-1), EFAULT);
+		break;
+	case 2:
+		TST_EXP_FAIL(sys_setitimer(-ITIMER_PROF, value, ovalue), EINVAL);
+		break;
+	}
 }

 static void setup(void)
@@ -56,7 +49,7 @@ static void setup(void)
 }

 static struct tst_test test = {
-        .tcnt = ARRAY_SIZE(tcases),
+        .tcnt = 3,
 	.test = verify_setitimer,
 	.setup = setup,
 	.bufs = (struct tst_buffers[]) {

This prints

setitimer02.c:32: TPASS: sys_setitimer(ITIMER_REAL, value, (void *)-1) : EFAULT (14)
setitimer02.c:35: TPASS: sys_setitimer(ITIMER_VIRTUAL, value, (void *)-1) : EFAULT (14)
setitimer02.c:38: TPASS: sys_setitimer(-ITIMER_PROF, value, ovalue) : EINVAL (22)

instead of

setitimer02.c:46: TPASS: sys_setitimer(tc->which, *(tc->val), *(tc->oval)) : EFAULT (14)
setitimer02.c:46: TPASS: sys_setitimer(tc->which, *(tc->val), *(tc->oval)) : EFAULT (14)
setitimer02.c:46: TPASS: sys_setitimer(tc->which, *(tc->val), *(tc->oval)) : EINVAL (22)

The same values are passed to the syscall according to strace.

-- 
Thank you,
Richard.

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