All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] setitimer01: rewrite using new API
@ 2022-10-09  8:57 Li Wang
  2022-10-20  8:42 ` Richard Palethorpe
  2022-10-24  2:40 ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Li Wang
  0 siblings, 2 replies; 9+ messages in thread
From: Li Wang @ 2022-10-09  8:57 UTC (permalink / raw)
  To: ltp

Also add signal checking when the timer take effection.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 .../kernel/syscalls/setitimer/setitimer01.c   | 230 ++++++++----------
 1 file changed, 99 insertions(+), 131 deletions(-)

diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c
index 6874b94ad..def559db8 100644
--- a/testcases/kernel/syscalls/setitimer/setitimer01.c
+++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
@@ -1,157 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
+ * 03/2001 - Written by Wayne Boyer
  *
- *   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
- *	setitimer01.c
- *
- * DESCRIPTION
- *	setitimer01 - check that a resonable setitimer() call succeeds.
+/*\
+ * [Description]
  *
- * 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 zero
- *	otherwise, the tests fails
- *	  issue a FAIL message
- *	  break any remaining tests
- *	  call cleanup
- *
- * USAGE:  <for command-line>
- *  setitimer01 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *	       -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
+ * Check that a setitimer() call pass with timer seting.
+ * Check if signal is generated correctly when when timer expiration.
  */
 
-#include "test.h"
-
 #include <errno.h>
 #include <sys/time.h>
-
-void cleanup(void);
-void setup(void);
-
-char *TCID = "setitimer01";
-int TST_TOTAL = 1;
+#include <stdlib.h>
+#include "tst_test.h"
+#include "lapi/syscalls.h"
 
 #define SEC0	0
-#define SEC1	20
-#define SEC2	40
-
-int main(int ac, char **av)
+#define SEC1	3
+#define SEC2	5
+
+static volatile int si_flag;
+static struct itimerval *value, *ovalue;
+
+static struct tcase {
+       int which;
+       struct itimerval **val;
+       struct itimerval **oval;
+       int signo;
+} tcases[] = {
+       {ITIMER_REAL,    &value, &ovalue, SIGALRM},
+       {ITIMER_VIRTUAL, &value, &ovalue, SIGVTALRM},
+       {ITIMER_PROF,    &value, &ovalue, SIGPROF},
+};
+
+static int sys_setitimer(int which, void *new_value, void *old_value)
 {
-	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 the 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, "ovalue malloc failed");
-		}
-
-		/* set up some reasonable values */
-
-		value->it_value.tv_sec = SEC1;
-		value->it_value.tv_usec = SEC0;
-		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
-		 */
+	return tst_syscall(__NR_setitimer, which, new_value, old_value);
+}
 
-		TEST(setitimer(ITIMER_REAL, value, ovalue));
+static void sig_routine(int signo)
+{
+	switch(signo){
+	case SIGALRM:
+		si_flag = 1;
+		break;
+	case SIGVTALRM:
+		si_flag = 2;
+		break;
+	case SIGPROF:
+		si_flag = 3;
+		break;
+	default:
+		break;
+	}
+}
 
-		if (TEST_RETURN != 0) {
-			tst_resm(TFAIL, "call failed - errno = %d - %s",
-				 TEST_ERRNO, strerror(TEST_ERRNO));
-			continue;
+static void verify_setitimer(unsigned int i)
+{
+	struct tcase *tc = &tcases[i];
+
+	si_flag = 0;
+	value->it_value.tv_sec = SEC1;
+	value->it_value.tv_usec = SEC0;
+	value->it_interval.tv_sec = SEC0;
+	value->it_interval.tv_usec = SEC0;
+
+	TST_EXP_PASS(sys_setitimer(tc->which, *(tc->val), *(tc->oval)));
+
+	/*
+	 * call setitimer again with new values.
+	 * the old values should be stored in ovalue
+	 */
+	value->it_value.tv_sec = SEC2;
+	value->it_value.tv_usec = SEC0;
+
+	TST_EXP_PASS(sys_setitimer(tc->which, *(tc->val), *(tc->oval)));
+
+	if (ovalue->it_value.tv_sec <= SEC1)
+		tst_res(TPASS, "setitimer functionality is correct");
+	else
+		tst_brk(TFAIL, "old timer value is not equal to expected value");
+
+	for (;;) {
+		if (tc->which == ITIMER_REAL && si_flag == 1) {
+			tst_res(TPASS, "SIGALRM signal is generated");
+			break;
 		}
 
-		/*
-		 * call setitimer again with new values.
-		 * the old values should be stored in ovalue
-		 */
-		value->it_value.tv_sec = SEC2;
-		value->it_value.tv_usec = SEC0;
-
-		if ((setitimer(ITIMER_REAL, value, ovalue)) == -1) {
-			tst_brkm(TBROK, cleanup, "second setitimer "
-				 "call failed");
+		if (tc->which == ITIMER_VIRTUAL && si_flag == 2) {
+			tst_res(TPASS, "SIGVTALRM signal is generated");
+			break;
 		}
 
-		if (ovalue->it_value.tv_sec <= SEC1) {
-			tst_resm(TPASS, "functionality is correct");
-		} else {
-			tst_brkm(TFAIL, cleanup, "old timer value is "
-				 "not equal to expected value");
+		if (tc->which == ITIMER_PROF && si_flag == 3) {
+			tst_res(TPASS, "SIGPROF signal is generated");
+			break;
 		}
 	}
 
-	cleanup();
-	tst_exit();
+	if (si_flag == 0)
+		tst_res(TFAIL, "signal is incorrect");
 }
 
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
-void setup(void)
+static void setup(void)
 {
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	SAFE_SIGNAL(SIGALRM, sig_routine);
+	SAFE_SIGNAL(SIGVTALRM, sig_routine);
+	SAFE_SIGNAL(SIGPROF, sig_routine);
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
-void cleanup(void)
-{
-
-}
+static struct tst_test test = {
+        .tcnt = ARRAY_SIZE(tcases),
+	.test = verify_setitimer,
+	.setup = setup,
+	.max_runtime = 60,
+	.bufs = (struct tst_buffers[]) {
+		{&value,  .size = sizeof(struct itimerval)},
+		{&ovalue, .size = sizeof(struct itimerval)},
+		{}
+	}
+};
-- 
2.35.3


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

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

* Re: [LTP] [PATCH] setitimer01: rewrite using new API
  2022-10-09  8:57 [LTP] [PATCH] setitimer01: rewrite using new API Li Wang
@ 2022-10-20  8:42 ` Richard Palethorpe
  2022-10-20 10:00   ` Li Wang
  2022-10-24  2:23   ` Li Wang
  2022-10-24  2:40 ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Li Wang
  1 sibling, 2 replies; 9+ messages in thread
From: Richard Palethorpe @ 2022-10-20  8:42 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hello,

Li Wang <liwang@redhat.com> writes:

> Also add signal checking when the timer take effection.
>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  .../kernel/syscalls/setitimer/setitimer01.c   | 230 ++++++++----------
>  1 file changed, 99 insertions(+), 131 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c
> index 6874b94ad..def559db8 100644
> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c
> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
> @@ -1,157 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> + * Copyright (c) International Business Machines  Corp., 2001
> + * 03/2001 - Written by Wayne Boyer
>   *
> - *   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
> - *	setitimer01.c
> - *
> - * DESCRIPTION
> - *	setitimer01 - check that a resonable setitimer() call succeeds.
> +/*\
> + * [Description]
>   *
> - * 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 zero
> - *	otherwise, the tests fails
> - *	  issue a FAIL message
> - *	  break any remaining tests
> - *	  call cleanup
> - *
> - * USAGE:  <for command-line>
> - *  setitimer01 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
> - *     where,  -c n : Run n copies concurrently.
> - *             -f   : Turn off functionality Testing.
> - *	       -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
> + * Check that a setitimer() call pass with timer seting.
> + * Check if signal is generated correctly when when timer expiration.
>   */
>  
> -#include "test.h"
> -
>  #include <errno.h>
>  #include <sys/time.h>
> -
> -void cleanup(void);
> -void setup(void);
> -
> -char *TCID = "setitimer01";
> -int TST_TOTAL = 1;
> +#include <stdlib.h>
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
>  
>  #define SEC0	0
> -#define SEC1	20
> -#define SEC2	40
> -
> -int main(int ac, char **av)
> +#define SEC1	3
> +#define SEC2	5
> +
> +static volatile int si_flag;
> +static struct itimerval *value, *ovalue;
> +
> +static struct tcase {
> +       int which;
> +       struct itimerval **val;
> +       struct itimerval **oval;
> +       int signo;
> +} tcases[] = {
> +       {ITIMER_REAL,    &value, &ovalue, SIGALRM},
> +       {ITIMER_VIRTUAL, &value, &ovalue, SIGVTALRM},
> +       {ITIMER_PROF,    &value, &ovalue, SIGPROF},

Same issue here as in other itimer test.

> +};
> +
> +static int sys_setitimer(int which, void *new_value, void *old_value)
>  {
> -	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 the 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, "ovalue malloc failed");
> -		}
> -
> -		/* set up some reasonable values */
> -
> -		value->it_value.tv_sec = SEC1;
> -		value->it_value.tv_usec = SEC0;
> -		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
> -		 */
> +	return tst_syscall(__NR_setitimer, which, new_value, old_value);
> +}
>  
> -		TEST(setitimer(ITIMER_REAL, value, ovalue));
> +static void sig_routine(int signo)
> +{
> +	switch(signo){
> +	case SIGALRM:
> +		si_flag = 1;
> +		break;
> +	case SIGVTALRM:
> +		si_flag = 2;
> +		break;
> +	case SIGPROF:
> +		si_flag = 3;
> +		break;
> +	default:
> +		break;
> +	}
> +}
>  
> -		if (TEST_RETURN != 0) {
> -			tst_resm(TFAIL, "call failed - errno = %d - %s",
> -				 TEST_ERRNO, strerror(TEST_ERRNO));
> -			continue;
> +static void verify_setitimer(unsigned int i)
> +{
> +	struct tcase *tc = &tcases[i];
> +
> +	si_flag = 0;
> +	value->it_value.tv_sec = SEC1;
> +	value->it_value.tv_usec = SEC0;

Why not test usecs instead?

> +	value->it_interval.tv_sec = SEC0;
> +	value->it_interval.tv_usec = SEC0;
> +
> +	TST_EXP_PASS(sys_setitimer(tc->which, *(tc->val), *(tc->oval)));
> +
> +	/*
> +	 * call setitimer again with new values.
> +	 * the old values should be stored in ovalue
> +	 */

We should probably set ovalue to some value > SEC1 here. Otherwise it
could just be zero or some previous value that is still valid.

> +	value->it_value.tv_sec = SEC2;
> +	value->it_value.tv_usec = SEC0;
> +
> +	TST_EXP_PASS(sys_setitimer(tc->which, *(tc->val), *(tc->oval)));
> +
> +	if (ovalue->it_value.tv_sec <= SEC1)
> +		tst_res(TPASS, "setitimer functionality is correct");
> +	else
> +		tst_brk(TFAIL, "old timer value is not equal to expected value");
> +
> +	for (;;) {

Could we use sigwait here instead?

We seem to be burning CPU cycles for no reason and if we get a spurious
signal the test will still pass if we get the correct one afterwards.

-- 
Thank you,
Richard.

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

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

* Re: [LTP] [PATCH] setitimer01: rewrite using new API
  2022-10-20  8:42 ` Richard Palethorpe
@ 2022-10-20 10:00   ` Li Wang
  2022-10-24  2:23   ` Li Wang
  1 sibling, 0 replies; 9+ messages in thread
From: Li Wang @ 2022-10-20 10:00 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp


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

Richard Palethorpe <rpalethorpe@suse.de> wrote:



> > +      * call setitimer again with new values.
> > +      * the old values should be stored in ovalue
> > +      */
>
> We should probably set ovalue to some value > SEC1 here. Otherwise it
> could just be zero or some previous value that is still valid.
>

Make sense!

And the rest comments all sound good to me. Will send V2 tomorrow.


-- 
Regards,
Li Wang

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

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


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

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

* Re: [LTP] [PATCH] setitimer01: rewrite using new API
  2022-10-20  8:42 ` Richard Palethorpe
  2022-10-20 10:00   ` Li Wang
@ 2022-10-24  2:23   ` Li Wang
  1 sibling, 0 replies; 9+ messages in thread
From: Li Wang @ 2022-10-24  2:23 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp


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

Hi Richard,

After trying that there were new problems with the below two comments.


> +static void verify_setitimer(unsigned int i)
> > +{
> > +     struct tcase *tc = &tcases[i];
> > +
> > +     si_flag = 0;
> > +     value->it_value.tv_sec = SEC1;
> > +     value->it_value.tv_usec = SEC0;
>
> Why not test usecs instead?
>

Yes, But the point we need attention here is to avoid defining
a very tiny value for USEC. Otherwise, test will sometimes get
first-timer expiration too early so that it has not had enough time
for the rest code get an execution.


> +     if (ovalue->it_value.tv_sec <= SEC1)
> > +             tst_res(TPASS, "setitimer functionality is correct");
> > +     else
> > +             tst_brk(TFAIL, "old timer value is not equal to expected
> value");
> > +
> > +     for (;;) {
>
> Could we use sigwait here instead?
>

sigwait() function suspends the calling progress which will
disturb the target process counts down the user-mode CPU time.
This led to the test hanging and never expiring the timer.


> We seem to be burning CPU cycles for no reason and if we get a spurious
> signal the test will still pass if we get the correct one afterwards.
>

And, these three signals all will terminate the process, so maybe
another resolution is to test in children and check if they exit by
the expected signal.

-- 
Regards,
Li Wang

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

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


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

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

* [LTP] [PATCH v2 1/2] setitimer03: convert to new API
  2022-10-09  8:57 [LTP] [PATCH] setitimer01: rewrite using new API Li Wang
  2022-10-20  8:42 ` Richard Palethorpe
@ 2022-10-24  2:40 ` Li Wang
  2022-10-24  2:40   ` [LTP] [PATCH v2 2/2] setitimer01: rewrite using " Li Wang
  2022-10-24  6:52   ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Richard Palethorpe
  1 sibling, 2 replies; 9+ messages in thread
From: Li Wang @ 2022-10-24  2:40 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   |  29 +++-
 .../kernel/syscalls/setitimer/setitimer03.c   | 158 ------------------
 4 files changed, 21 insertions(+), 168 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/setitimer/setitimer03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index a52b93c92..3dc6fa397 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1327,7 +1327,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..b012d71fa 100644
--- a/testcases/kernel/syscalls/setitimer/setitimer02.c
+++ b/testcases/kernel/syscalls/setitimer/setitimer02.c
@@ -8,8 +8,10 @@
 /*\
  * [Description]
  *
- * Check that a setitimer() call fails with EFAULT with invalid itimerval
- * pointer.
+ * Check that setitimer() call fails:
+ *
+ * 1. EFAULT with invalid itimerval pointer
+ * 2. EINVAL when called with an invalid first argument
  */
 
 #include <errno.h>
@@ -18,17 +20,26 @@
 #include "tst_test.h"
 #include "lapi/syscalls.h"
 
-static struct itimerval *value;
+static struct itimerval *value, *ovalue;
 
 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);
+	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)
@@ -40,10 +51,12 @@ static void setup(void)
 }
 
 static struct tst_test test = {
-	.test_all = verify_setitimer,
+	.tcnt = 3,
+	.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] 9+ messages in thread

* [LTP] [PATCH v2 2/2] setitimer01: rewrite using new API
  2022-10-24  2:40 ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Li Wang
@ 2022-10-24  2:40   ` Li Wang
  2022-10-24  6:58     ` Richard Palethorpe
  2022-10-24  6:52   ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Richard Palethorpe
  1 sibling, 1 reply; 9+ messages in thread
From: Li Wang @ 2022-10-24  2:40 UTC (permalink / raw)
  To: ltp

Also add signal checking when the timer take effection.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 .../kernel/syscalls/setitimer/setitimer01.c   | 214 +++++++-----------
 1 file changed, 77 insertions(+), 137 deletions(-)

diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c
index 6874b94ad..f04cb5a69 100644
--- a/testcases/kernel/syscalls/setitimer/setitimer01.c
+++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
@@ -1,157 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
+ * 03/2001 - Written by Wayne Boyer
  *
- *   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
- *	setitimer01.c
- *
- * DESCRIPTION
- *	setitimer01 - check that a resonable setitimer() call succeeds.
- *
- * 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 zero
- *	otherwise, the tests fails
- *	  issue a FAIL message
- *	  break any remaining tests
- *	  call cleanup
- *
- * USAGE:  <for command-line>
- *  setitimer01 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *	       -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.
+/*\
+ * [Description]
  *
- * HISTORY
- *	03/2001 - Written by Wayne Boyer
- *
- * RESTRICTIONS
- *	none
+ * Check that a setitimer() call pass with timer seting.
+ * Check if signal is generated correctly when timer expiration.
  */
 
-#include "test.h"
-
 #include <errno.h>
 #include <sys/time.h>
+#include <stdlib.h>
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+#define USEC1	10000
+#define USEC2	20000
+
+static struct itimerval *value, *ovalue;
+
+static struct tcase {
+	int which;
+	char *des;
+	int signo;
+} tcases[] = {
+	{ITIMER_REAL,    "ITIMER_REAL",    SIGALRM},
+	{ITIMER_VIRTUAL, "ITIMER_VIRTUAL", SIGVTALRM},
+	{ITIMER_PROF,    "ITIMER_PROF",    SIGPROF},
+};
+
+static int sys_setitimer(int which, void *new_value, void *old_value)
+{
+	return tst_syscall(__NR_setitimer, which, new_value, old_value);
+}
+
+static void set_setitimer_value(int usec, int o_usec)
+{
+	value->it_value.tv_sec = 0;
+	value->it_value.tv_usec = usec;
+	value->it_interval.tv_sec = 0;
+	value->it_interval.tv_usec = 0;
+
+	ovalue->it_value.tv_sec = o_usec;
+	ovalue->it_value.tv_usec = o_usec;
+	ovalue->it_interval.tv_sec = 0;
+	ovalue->it_interval.tv_usec = 0;
+}
 
-void cleanup(void);
-void setup(void);
+static void verify_setitimer(unsigned int i)
+{
+	pid_t pid;
+	int status;
+	struct tcase *tc = &tcases[i];
 
-char *TCID = "setitimer01";
-int TST_TOTAL = 1;
+	pid = SAFE_FORK();
 
-#define SEC0	0
-#define SEC1	20
-#define SEC2	40
+	if (pid == 0) {
+		tst_res(TINFO, "tc->which = %s", tc->des);
 
-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 the 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, "ovalue malloc failed");
-		}
-
-		/* set up some reasonable values */
-
-		value->it_value.tv_sec = SEC1;
-		value->it_value.tv_usec = SEC0;
-		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
-		 */
-
-		TEST(setitimer(ITIMER_REAL, value, ovalue));
-
-		if (TEST_RETURN != 0) {
-			tst_resm(TFAIL, "call failed - errno = %d - %s",
-				 TEST_ERRNO, strerror(TEST_ERRNO));
-			continue;
-		}
-
-		/*
-		 * call setitimer again with new values.
-		 * the old values should be stored in ovalue
-		 */
-		value->it_value.tv_sec = SEC2;
-		value->it_value.tv_usec = SEC0;
-
-		if ((setitimer(ITIMER_REAL, value, ovalue)) == -1) {
-			tst_brkm(TBROK, cleanup, "second setitimer "
-				 "call failed");
-		}
-
-		if (ovalue->it_value.tv_sec <= SEC1) {
-			tst_resm(TPASS, "functionality is correct");
-		} else {
-			tst_brkm(TFAIL, cleanup, "old timer value is "
-				 "not equal to expected value");
-		}
-	}
+		tst_no_corefile(0);
 
-	cleanup();
-	tst_exit();
-}
+		set_setitimer_value(USEC1, 0);
+		TST_EXP_PASS(sys_setitimer(tc->which, value, NULL));
 
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
-void setup(void)
-{
+		set_setitimer_value(USEC2, USEC2);
+		TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue));
 
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+		if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec >= USEC2)
+			tst_brk(TFAIL, "old timer value is not within the expected range");
 
-	TEST_PAUSE;
-}
+		for (;;)
+			;
+	}
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
-void cleanup(void)
-{
+	SAFE_WAITPID(pid, &status, 0);
 
+	if (WIFSIGNALED(status) && WTERMSIG(status) == tc->signo)
+		tst_res(TPASS, "Child received signal: %s", tst_strsig(tc->signo));
+	else
+		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.forks_child = 1,
+	.test = verify_setitimer,
+	.bufs = (struct tst_buffers[]) {
+		{&value,  .size = sizeof(struct itimerval)},
+		{&ovalue, .size = sizeof(struct itimerval)},
+		{}
+	}
+};
-- 
2.35.3


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

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

* Re: [LTP] [PATCH v2 1/2] setitimer03: convert to new API
  2022-10-24  2:40 ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Li Wang
  2022-10-24  2:40   ` [LTP] [PATCH v2 2/2] setitimer01: rewrite using " Li Wang
@ 2022-10-24  6:52   ` Richard Palethorpe
  1 sibling, 0 replies; 9+ messages in thread
From: Richard Palethorpe @ 2022-10-24  6:52 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.
>
> Signed-off-by: Li Wang <liwang@redhat.com>

Looks, good!

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

> ---
>  runtest/syscalls                              |   1 -
>  .../kernel/syscalls/setitimer/.gitignore      |   1 -
>  .../kernel/syscalls/setitimer/setitimer02.c   |  29 +++-
>  .../kernel/syscalls/setitimer/setitimer03.c   | 158 ------------------
>  4 files changed, 21 insertions(+), 168 deletions(-)
>  delete mode 100644 testcases/kernel/syscalls/setitimer/setitimer03.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index a52b93c92..3dc6fa397 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1327,7 +1327,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..b012d71fa 100644
> --- a/testcases/kernel/syscalls/setitimer/setitimer02.c
> +++ b/testcases/kernel/syscalls/setitimer/setitimer02.c
> @@ -8,8 +8,10 @@
>  /*\
>   * [Description]
>   *
> - * Check that a setitimer() call fails with EFAULT with invalid itimerval
> - * pointer.
> + * Check that setitimer() call fails:
> + *
> + * 1. EFAULT with invalid itimerval pointer
> + * 2. EINVAL when called with an invalid first argument
>   */
>  
>  #include <errno.h>
> @@ -18,17 +20,26 @@
>  #include "tst_test.h"
>  #include "lapi/syscalls.h"
>  
> -static struct itimerval *value;
> +static struct itimerval *value, *ovalue;
>  
>  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);
> +	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)
> @@ -40,10 +51,12 @@ static void setup(void)
>  }
>  
>  static struct tst_test test = {
> -	.test_all = verify_setitimer,
> +	.tcnt = 3,
> +	.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


-- 
Thank you,
Richard.

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

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

* Re: [LTP] [PATCH v2 2/2] setitimer01: rewrite using new API
  2022-10-24  2:40   ` [LTP] [PATCH v2 2/2] setitimer01: rewrite using " Li Wang
@ 2022-10-24  6:58     ` Richard Palethorpe
  2022-10-24  8:41       ` Li Wang
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Palethorpe @ 2022-10-24  6:58 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hello,

Li Wang <liwang@redhat.com> writes:

>  
> -#include "test.h"
> -
>  #include <errno.h>
>  #include <sys/time.h>
> +#include <stdlib.h>
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +#define USEC1	10000
> +#define USEC2	20000
> +
> +static struct itimerval *value, *ovalue;
> +
> +static struct tcase {
> +	int which;
> +	char *des;
> +	int signo;
> +} tcases[] = {
> +	{ITIMER_REAL,    "ITIMER_REAL",    SIGALRM},
> +	{ITIMER_VIRTUAL, "ITIMER_VIRTUAL", SIGVTALRM},
> +	{ITIMER_PROF,    "ITIMER_PROF",    SIGPROF},
> +};
> +
> +static int sys_setitimer(int which, void *new_value, void *old_value)
> +{
> +	return tst_syscall(__NR_setitimer, which, new_value, old_value);
> +}
> +
> +static void set_setitimer_value(int usec, int o_usec)
> +{
> +	value->it_value.tv_sec = 0;
> +	value->it_value.tv_usec = usec;
> +	value->it_interval.tv_sec = 0;
> +	value->it_interval.tv_usec = 0;
> +
> +	ovalue->it_value.tv_sec = o_usec;
> +	ovalue->it_value.tv_usec = o_usec;
> +	ovalue->it_interval.tv_sec = 0;
> +	ovalue->it_interval.tv_usec = 0;
> +}
>  
> -void cleanup(void);
> -void setup(void);
> +static void verify_setitimer(unsigned int i)
> +{
> +	pid_t pid;
> +	int status;
> +	struct tcase *tc = &tcases[i];
>  
> -char *TCID = "setitimer01";
> -int TST_TOTAL = 1;
> +	pid = SAFE_FORK();
>  
> -#define SEC0	0
> -#define SEC1	20
> -#define SEC2	40
> +	if (pid == 0) {
> +		tst_res(TINFO, "tc->which = %s", tc->des);
>  
> -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 the 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, "ovalue malloc failed");
> -		}
> -
> -		/* set up some reasonable values */
> -
> -		value->it_value.tv_sec = SEC1;
> -		value->it_value.tv_usec = SEC0;
> -		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
> -		 */
> -
> -		TEST(setitimer(ITIMER_REAL, value, ovalue));
> -
> -		if (TEST_RETURN != 0) {
> -			tst_resm(TFAIL, "call failed - errno = %d - %s",
> -				 TEST_ERRNO, strerror(TEST_ERRNO));
> -			continue;
> -		}
> -
> -		/*
> -		 * call setitimer again with new values.
> -		 * the old values should be stored in ovalue
> -		 */
> -		value->it_value.tv_sec = SEC2;
> -		value->it_value.tv_usec = SEC0;
> -
> -		if ((setitimer(ITIMER_REAL, value, ovalue)) == -1) {
> -			tst_brkm(TBROK, cleanup, "second setitimer "
> -				 "call failed");
> -		}
> -
> -		if (ovalue->it_value.tv_sec <= SEC1) {
> -			tst_resm(TPASS, "functionality is correct");
> -		} else {
> -			tst_brkm(TFAIL, cleanup, "old timer value is "
> -				 "not equal to expected value");
> -		}
> -	}
> +		tst_no_corefile(0);
>  
> -	cleanup();
> -	tst_exit();
> -}
> +		set_setitimer_value(USEC1, 0);
> +		TST_EXP_PASS(sys_setitimer(tc->which, value, NULL));
>  
> -/*
> - * setup() - performs all the ONE TIME setup for this test.
> - */
> -void setup(void)
> -{
> +		set_setitimer_value(USEC2, USEC2);
> +		TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue));
>  
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +		if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec >= USEC2)
> +			tst_brk(TFAIL, "old timer value is not within
> the expected range");

Maybe we could split testing the return value and ovalue from testing
the signal is delivered?

When testing the return value and ovalue; a very long timeout can be
used (which is never hit). When testing the signal a very short one can
be used.

This way, the test is not racing with the signal and the loop below
won't be executed much.

In any case LGTM

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>  
> -	TEST_PAUSE;
> -}
> +		for (;;)
> +			;
> +	}
>  
> -/*
> - * cleanup() - performs all the ONE TIME cleanup for this test at completion
> - * 	       or premature exit.
> - */
> -void cleanup(void)
> -{
> +	SAFE_WAITPID(pid, &status, 0);
>  
> +	if (WIFSIGNALED(status) && WTERMSIG(status) == tc->signo)
> +		tst_res(TPASS, "Child received signal: %s", tst_strsig(tc->signo));
> +	else
> +		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
>  }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.forks_child = 1,
> +	.test = verify_setitimer,
> +	.bufs = (struct tst_buffers[]) {
> +		{&value,  .size = sizeof(struct itimerval)},
> +		{&ovalue, .size = sizeof(struct itimerval)},
> +		{}
> +	}
> +};
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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

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

* Re: [LTP] [PATCH v2 2/2] setitimer01: rewrite using new API
  2022-10-24  6:58     ` Richard Palethorpe
@ 2022-10-24  8:41       ` Li Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Li Wang @ 2022-10-24  8:41 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp


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

Richard Palethorpe <rpalethorpe@suse.de> wrote:

> -/*
> > - * setup() - performs all the ONE TIME setup for this test.
> > - */
> > -void setup(void)
> > -{
> > +             set_setitimer_value(USEC2, USEC2);
> > +             TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue));
> >
> > -     tst_sig(NOFORK, DEF_HANDLER, cleanup);
> > +             if (ovalue->it_value.tv_sec != 0 ||
> ovalue->it_value.tv_usec >= USEC2)
> > +                     tst_brk(TFAIL, "old timer value is not within
> > the expected range");
>
> Maybe we could split testing the return value and ovalue from testing
> the signal is delivered?
>
> When testing the return value and ovalue; a very long timeout can be
> used (which is never hit). When testing the signal a very short one can
> be used.
>
> This way, the test is not racing with the signal and the loop below
> won't be executed much.
>

Very good point. Also, I plan to add the 'it_interval' check to
verify the periodic timer, which will make the functionality test
more complete.

This will be achieved in a separate patch.


>
> In any case LGTM
>
> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
>

Thanks for the review.

-- 
Regards,
Li Wang

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

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


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

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

end of thread, other threads:[~2022-10-24  8:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-09  8:57 [LTP] [PATCH] setitimer01: rewrite using new API Li Wang
2022-10-20  8:42 ` Richard Palethorpe
2022-10-20 10:00   ` Li Wang
2022-10-24  2:23   ` Li Wang
2022-10-24  2:40 ` [LTP] [PATCH v2 1/2] setitimer03: convert to " Li Wang
2022-10-24  2:40   ` [LTP] [PATCH v2 2/2] setitimer01: rewrite using " Li Wang
2022-10-24  6:58     ` Richard Palethorpe
2022-10-24  8:41       ` Li Wang
2022-10-24  6:52   ` [LTP] [PATCH v2 1/2] setitimer03: convert to " 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.