All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check
@ 2022-10-09  6:55 Li Wang
  2022-10-09  6:55 ` [LTP] [PATCH 2/2] setitimer03: convert to new API Li Wang
  2022-10-20  8:11 ` [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check Richard Palethorpe
  0 siblings, 2 replies; 8+ messages in thread
From: Li Wang @ 2022-10-09  6:55 UTC (permalink / raw)
  To: ltp

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/syscalls/getitimer/getitimer02.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/getitimer/getitimer02.c b/testcases/kernel/syscalls/getitimer/getitimer02.c
index 863e8e0a8..cd98dcce8 100644
--- a/testcases/kernel/syscalls/getitimer/getitimer02.c
+++ b/testcases/kernel/syscalls/getitimer/getitimer02.c
@@ -27,8 +27,9 @@ static struct tcase {
        struct itimerval **val;
        int exp_errno;
 } tcases[] = {
-       {ITIMER_REAL, &invalid, EFAULT},
-       {-ITIMER_PROF, &value, EINVAL},
+       {ITIMER_REAL,    &invalid, EFAULT},
+       {ITIMER_VIRTUAL, &invalid, EFAULT},
+       {-ITIMER_PROF,   &value,   EINVAL},
 };
 
 static int sys_getitimer(int which, void *curr_value)
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [LTP] [PATCH 2/2] setitimer03: convert to new API
  2022-10-09  6:55 [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check Li Wang
@ 2022-10-09  6:55 ` Li Wang
  2022-10-20  8:17   ` Richard Palethorpe
  2022-10-20  8:11 ` [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check Richard Palethorpe
  1 sibling, 1 reply; 8+ messages in thread
From: Li Wang @ 2022-10-09  6:55 UTC (permalink / raw)
  To: ltp

Combine this EINVAL test into setitimer02 and add one additional
ITIMER_VIRTUAL verification.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 runtest/syscalls                              |   1 -
 .../kernel/syscalls/setitimer/.gitignore      |   1 -
 .../kernel/syscalls/setitimer/setitimer02.c   |  30 +++-
 .../kernel/syscalls/setitimer/setitimer03.c   | 158 ------------------
 4 files changed, 24 insertions(+), 166 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/setitimer/setitimer03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 61a7b7677..2d673836d 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1325,7 +1325,6 @@ sethostname03 sethostname03
 
 setitimer01 setitimer01
 setitimer02 setitimer02
-setitimer03 setitimer03
 
 setns01 setns01
 setns02 setns02
diff --git a/testcases/kernel/syscalls/setitimer/.gitignore b/testcases/kernel/syscalls/setitimer/.gitignore
index 048db9b31..35779a32c 100644
--- a/testcases/kernel/syscalls/setitimer/.gitignore
+++ b/testcases/kernel/syscalls/setitimer/.gitignore
@@ -1,3 +1,2 @@
 /setitimer01
 /setitimer02
-/setitimer03
diff --git a/testcases/kernel/syscalls/setitimer/setitimer02.c b/testcases/kernel/syscalls/setitimer/setitimer02.c
index 9ac9ce1fa..ccba231c9 100644
--- a/testcases/kernel/syscalls/setitimer/setitimer02.c
+++ b/testcases/kernel/syscalls/setitimer/setitimer02.c
@@ -18,17 +18,33 @@
 #include "tst_test.h"
 #include "lapi/syscalls.h"
 
-static struct itimerval *value;
+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);
 }
 
-static void verify_setitimer(void)
+static void verify_setitimer(unsigned int i)
 {
-	TST_EXP_FAIL(sys_setitimer(ITIMER_REAL, value, (struct itimerval *)-1),
-	             EFAULT);
+        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);
 }
 
 static void setup(void)
@@ -40,10 +56,12 @@ static void setup(void)
 }
 
 static struct tst_test test = {
-	.test_all = verify_setitimer,
+        .tcnt = ARRAY_SIZE(tcases),
+	.test = verify_setitimer,
 	.setup = setup,
 	.bufs = (struct tst_buffers[]) {
-		{&value, .size = sizeof(struct itimerval)},
+		{&value,  .size = sizeof(struct itimerval)},
+		{&ovalue, .size = sizeof(struct itimerval)},
 		{}
 	}
 };
diff --git a/testcases/kernel/syscalls/setitimer/setitimer03.c b/testcases/kernel/syscalls/setitimer/setitimer03.c
deleted file mode 100644
index 418ec71f0..000000000
--- a/testcases/kernel/syscalls/setitimer/setitimer03.c
+++ /dev/null
@@ -1,158 +0,0 @@
-/*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * NAME
- *	setitimer03.c
- *
- * DESCRIPTION
- *	setitimer03 - check that a setitimer() call fails as expected
- *		      with incorrect values.
- *
- * ALGORITHM
- *	loop if that option was specified
- *	allocate needed space and set up needed values
- *	issue the system call
- *	check the errno value
- *	  issue a PASS message if we get EINVAL
- *	otherwise, the tests fails
- *	  issue a FAIL message
- *	  break any remaining tests
- *	  call cleanup
- *
- * USAGE:  <for command-line>
- *  setitimer03 [-c n] [-e] [-i n] [-I x] [-p x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -e   : Turn on errno logging.
- *	       -i n : Execute test n times.
- *	       -I x : Execute test for x seconds.
- *	       -P x : Pause for x seconds between iterations.
- *	       -t   : Turn on syscall timing.
- *
- * HISTORY
- *	03/2001 - Written by Wayne Boyer
- *
- * RESTRICTIONS
- *	none
- */
-
-#include "test.h"
-
-#include <errno.h>
-#include <sys/time.h>
-
-void cleanup(void);
-void setup(void);
-
-char *TCID = "setitimer03";
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
-{
-	int lc;
-	struct itimerval *value, *ovalue;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();		/* global setup */
-
-	/* The following loop checks looping state if -i option given */
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-
-		/* allocate some space for timer structures */
-
-		if ((value = malloc((size_t)sizeof(struct itimerval))) ==
-		    NULL) {
-			tst_brkm(TBROK, cleanup, "value malloc failed");
-		}
-
-		if ((ovalue = malloc((size_t)sizeof(struct itimerval))) ==
-		    NULL) {
-			tst_brkm(TBROK, cleanup, "value malloc failed");
-		}
-
-		/* set up some reasonable values */
-
-		value->it_value.tv_sec = 30;
-		value->it_value.tv_usec = 0;
-		value->it_interval.tv_sec = 0;
-		value->it_interval.tv_usec = 0;
-
-		/*
-		 * issue the system call with the TEST() macro
-		 * ITIMER_REAL = 0, ITIMER_VIRTUAL = 1 and ITIMER_PROF = 2
-		 */
-
-		/* make the first value negative to get a failure */
-		TEST(setitimer(-ITIMER_PROF, value, ovalue));
-
-		if (TEST_RETURN == 0) {
-			tst_resm(TFAIL, "call failed to produce expected error "
-				 "- errno = %d - %s", TEST_ERRNO,
-				 strerror(TEST_ERRNO));
-			continue;
-		}
-
-		switch (TEST_ERRNO) {
-		case EINVAL:
-			tst_resm(TPASS, "expected failure - errno = %d - %s",
-				 TEST_ERRNO, strerror(TEST_ERRNO));
-			break;
-		default:
-			tst_resm(TFAIL, "call failed to produce expected error "
-				 "- errno = %d - %s", TEST_ERRNO,
-				 strerror(TEST_ERRNO));
-		}
-
-		/*
-		 * clean up things in case we are looping
-		 */
-		free(value);
-		free(ovalue);
-		value = NULL;
-		ovalue = NULL;
-	}
-
-	cleanup();
-	tst_exit();
-
-}
-
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
-void setup(void)
-{
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-}
-
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
-void cleanup(void)
-{
-
-}
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check
  2022-10-09  6:55 [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check Li Wang
  2022-10-09  6:55 ` [LTP] [PATCH 2/2] setitimer03: convert to new API Li Wang
@ 2022-10-20  8:11 ` Richard Palethorpe
  1 sibling, 0 replies; 8+ messages in thread
From: Richard Palethorpe @ 2022-10-20  8:11 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hello,

Merged, thanks!

Li Wang <liwang@redhat.com> writes:

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  testcases/kernel/syscalls/getitimer/getitimer02.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/getitimer/getitimer02.c b/testcases/kernel/syscalls/getitimer/getitimer02.c
> index 863e8e0a8..cd98dcce8 100644
> --- a/testcases/kernel/syscalls/getitimer/getitimer02.c
> +++ b/testcases/kernel/syscalls/getitimer/getitimer02.c
> @@ -27,8 +27,9 @@ static struct tcase {
>         struct itimerval **val;
>         int exp_errno;
>  } tcases[] = {
> -       {ITIMER_REAL, &invalid, EFAULT},
> -       {-ITIMER_PROF, &value, EINVAL},
> +       {ITIMER_REAL,    &invalid, EFAULT},
> +       {ITIMER_VIRTUAL, &invalid, EFAULT},
> +       {-ITIMER_PROF,   &value,   EINVAL},
>  };
>  
>  static int sys_getitimer(int which, void *curr_value)
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] setitimer03: convert to new API
  2022-10-09  6:55 ` [LTP] [PATCH 2/2] setitimer03: convert to new API Li Wang
@ 2022-10-20  8:17   ` Richard Palethorpe
  2022-10-20  9:30     ` Li Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Palethorpe @ 2022-10-20  8:17 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hello,

Li Wang <liwang@redhat.com> writes:

> Combine this EINVAL test into setitimer02 and add one additional
> ITIMER_VIRTUAL verification.

EINVAL is not mentioned in the test description.

>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  runtest/syscalls                              |   1 -
>  .../kernel/syscalls/setitimer/.gitignore      |   1 -
>  .../kernel/syscalls/setitimer/setitimer02.c   |  30 +++-
>  .../kernel/syscalls/setitimer/setitimer03.c   | 158 ------------------
>  4 files changed, 24 insertions(+), 166 deletions(-)
>  delete mode 100644 testcases/kernel/syscalls/setitimer/setitimer03.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 61a7b7677..2d673836d 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1325,7 +1325,6 @@ sethostname03 sethostname03
>  
>  setitimer01 setitimer01
>  setitimer02 setitimer02
> -setitimer03 setitimer03
>  
>  setns01 setns01
>  setns02 setns02
> diff --git a/testcases/kernel/syscalls/setitimer/.gitignore b/testcases/kernel/syscalls/setitimer/.gitignore
> index 048db9b31..35779a32c 100644
> --- a/testcases/kernel/syscalls/setitimer/.gitignore
> +++ b/testcases/kernel/syscalls/setitimer/.gitignore
> @@ -1,3 +1,2 @@
>  /setitimer01
>  /setitimer02
> -/setitimer03
> diff --git a/testcases/kernel/syscalls/setitimer/setitimer02.c b/testcases/kernel/syscalls/setitimer/setitimer02.c
> index 9ac9ce1fa..ccba231c9 100644
> --- a/testcases/kernel/syscalls/setitimer/setitimer02.c
> +++ b/testcases/kernel/syscalls/setitimer/setitimer02.c
> @@ -18,17 +18,33 @@
>  #include "tst_test.h"
>  #include "lapi/syscalls.h"
>  
> -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)

> +} 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?

>  
>  static int sys_setitimer(int which, void *new_value, void *old_value)
>  {
>  	return tst_syscall(__NR_setitimer, which, new_value, old_value);
>  }
>  
> -static void verify_setitimer(void)
> +static void verify_setitimer(unsigned int i)
>  {
> -	TST_EXP_FAIL(sys_setitimer(ITIMER_REAL, value, (struct itimerval *)-1),
> -	             EFAULT);
> +        struct tcase *tc = &tcases[i];
> +
> +	if (tc->exp_errno == EFAULT)
> +		*(tc->oval) = (struct itimerval *)-1;

Or, why do we use an if statement here instead of defining it in the struct?

-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] setitimer03: convert to new API
  2022-10-20  8:17   ` Richard Palethorpe
@ 2022-10-20  9:30     ` Li Wang
  2022-10-20  9:45       ` Li Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2022-10-20  9:30 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp


[-- Attachment #1.1: Type: text/plain, Size: 1659 bytes --]

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.

And for the last test item, I don't want the invalid pointer to have
a side effect disturbs the first invalid argument.



>
> >
> >  static int sys_setitimer(int which, void *new_value, void *old_value)
> >  {
> >       return tst_syscall(__NR_setitimer, which, new_value, old_value);
> >  }
> >
> > -static void verify_setitimer(void)
> > +static void verify_setitimer(unsigned int i)
> >  {
> > -     TST_EXP_FAIL(sys_setitimer(ITIMER_REAL, value, (struct itimerval
> *)-1),
> > -                  EFAULT);
> > +        struct tcase *tc = &tcases[i];
> > +
> > +     if (tc->exp_errno == EFAULT)
> > +             *(tc->oval) = (struct itimerval *)-1;
>
> Or, why do we use an if statement here instead of defining it in the
> struct?
>

Same above, that's why here explicitly reassign an invalid address.


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 3283 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] setitimer03: convert to new API
  2022-10-20  9:30     ` Li Wang
@ 2022-10-20  9:45       ` Li Wang
  2022-10-20 10:31         ` Richard Palethorpe
  0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2022-10-20  9:45 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp


[-- Attachment #1.1: Type: text/plain, Size: 1132 bytes --]

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.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2641 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] setitimer03: convert to new API
  2022-10-20  9:45       ` Li Wang
@ 2022-10-20 10:31         ` Richard Palethorpe
  2022-10-20 11:28           ` Li Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Palethorpe @ 2022-10-20 10:31 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] setitimer03: convert to new API
  2022-10-20 10:31         ` Richard Palethorpe
@ 2022-10-20 11:28           ` Li Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Li Wang @ 2022-10-20 11:28 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp


[-- Attachment #1.1: Type: text/plain, Size: 4060 bytes --]

On Thu, Oct 20, 2022 at 6:40 PM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> 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:
>

Ok, I got your point.  Previously I was stuck in thinking about how to keep
the unified tcase struct for error testing.

Seems I should give up the fixed mindset.

static struct tcase {
     ...
} tcases[] = {
     ...
};



>
> @@ -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.
>
>

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 6153 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-10-20 11:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-09  6:55 [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check Li Wang
2022-10-09  6:55 ` [LTP] [PATCH 2/2] setitimer03: convert to new API Li Wang
2022-10-20  8:17   ` Richard Palethorpe
2022-10-20  9:30     ` Li Wang
2022-10-20  9:45       ` Li Wang
2022-10-20 10:31         ` Richard Palethorpe
2022-10-20 11:28           ` Li Wang
2022-10-20  8:11 ` [LTP] [PATCH 1/2] getitimer02: add ITIMER_VIRTUAL timer error check Richard Palethorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.