[LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep

[LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep

[Stanislav Kholmanskikh]

I came accross the following rare issue:

setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1

I suppose It may happen if the test host is under stress.
To prevent this It would be better to use SIGUSR1 for parent-child
synchronisations.

Also fixed error passing from children and performed a minor cleanup.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/setpgid/setpgid03.c |  200 ++++++++++++-------------
 1 files changed, 98 insertions(+), 102 deletions(-)

diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 4429fe4..17fd16f 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -1,34 +1,23 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   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 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.
  *
- *   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
+ * 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
- * 	setpgid03.c
- *
- * DESCRIPTION
- * 	Test to check the error and trivial conditions in setpgid system call
- *
- * USAGE
- * 	setuid03
- *
- * RESTRICTIONS
- * 	This test is not completely written in the LTP format - PLEASE FIX!
+ * Test to check the error and trivial conditions in setpgid system call
  */
 
 #define DEBUG 0
@@ -47,16 +36,16 @@
 char *TCID = "setpgid03";
 int TST_TOTAL = 1;
 
-void do_child(void);
-void setup(void);
-void cleanup(void);
+static void do_child(void);
+static void setup(void);
+static void cleanup(void);
 
 int main(int ac, char **av)
 {
 	int pid;
-	int rval, fail = 0;
-	int ret, status;
-	int exno = 0;
+	int rval;
+	int status;
+	sigset_t sigusr_set;
 
 	int lc;
 	char *msg;
@@ -68,14 +57,13 @@ int main(int ac, char **av)
 	maybe_run_child(&do_child, "");
 #endif
 
-	/*
-	 * perform global setup for the test
-	 */
 	setup();
 
+	sigemptyset(&sigusr_set);
+	sigaddset(&sigusr_set, SIGUSR1);
+
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 
-		/* reset tst_count in case we are looping */
 		tst_count = 0;
 
 //test1:
@@ -84,7 +72,7 @@ int main(int ac, char **av)
 			tst_brkm(TBROK, cleanup, "fork() failed");
 		}
 
-		if (pid == 0) {	/* child */
+		if (pid == 0) {
 #ifdef UCLINUX
 			if (self_exec(av[0], "") < 0) {
 				tst_brkm(TBROK, cleanup, "self_exec failed");
@@ -92,35 +80,33 @@ int main(int ac, char **av)
 #else
 			do_child();
 #endif
-		} else {	/* parent */
-			sleep(1);
-			rval = setpgid(pid, getppid());
-			if (errno == EPERM) {
-				tst_resm(TPASS, "setpgid SUCCESS to set "
-					 "errno to EPERM");
-			} else {
-				tst_resm(TFAIL, "setpgid FAILED, "
-					 "expect %d, return %d", EPERM, errno);
-				fail = 1;
-			}
-			sleep(1);
-			if ((ret = wait(&status)) > 0) {
-				if (DEBUG)
-					tst_resm(TINFO, "Test {%d} exited "
-						 "status 0x%0x", ret, status);
-
-				if (status != 0) {
-					fail = 1;
-				}
-			}
 		}
-		if (DEBUG) {
-			if (fail || exno) {
-				tst_resm(TINFO, "Test test 1: FAILED");
-			} else {
-				tst_resm(TINFO, "Test test 1: PASSED");
-			}
+
+		if (sigwaitinfo(&sigusr_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigwaitinfo() failed");
+
+		rval = setpgid(pid, getppid());
+		if (errno == EPERM) {
+			tst_resm(TPASS,
+				"setpgid SUCCESS to set errno to EPERM");
+		} else {
+			tst_resm(TFAIL,
+				"setpgid FAILED, expect %d, return %d",
+				EPERM, errno);
+		}
+
+		if (wait(&status) < 0)
+			tst_resm(TFAIL | TERRNO, "wait() failed");
+
+		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) {
+			if (DEBUG)
+				tst_resm(TINFO, "child exited with status %d",
+					WEXITSTATUS(status));
+
+			tst_resm(TFAIL, "child failed");
 		}
+
 //test2:
 		/*
 		 * Value of pid matches the pid of the child process and
@@ -131,39 +117,59 @@ int main(int ac, char **av)
 
 		}
 		if (pid == 0) {
-			if (execlp("sleep", "sleep", "3", NULL) < 0) {
-				perror("exec failed");
+			char buf[10];
+			int ret;
+
+			ret = snprintf(buf, 10, "%d", getppid());
+			if ((ret >= 10) || (ret <= 0)) {
+				printf("snprintf() failed\n");
+
+				exit(126);
 			}
+
+			const char *args[4 + 1];
+			args[0] = "kill";
+			args[1] = "-s";
+			args[2] = "SIGUSR1";
+			args[3] = buf;
+			args[4] = NULL;
+
+			if (execvp(args[0], (char *const *)args) < 0)
+				perror("exec failed\n");
+
 			exit(127);
+		}
+
+		if (sigwaitinfo(&sigusr_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigwaitinfo() failed");
+
+		rval = setpgid(pid, getppid());
+		if (errno == EACCES) {
+			tst_resm(TPASS,
+				"setpgid SUCCEEDED to set errno to EACCES");
 		} else {
-			sleep(1);
-			rval = setpgid(pid, getppid());
-			if (errno == EACCES) {
-				tst_resm(TPASS, "setpgid SUCCEEDED to set "
-					 "errno to EACCES");
-			} else {
-				tst_resm(TFAIL, "setpgid FAILED, expect EACCES "
-					 "got %d", errno);
-			}
-			if ((ret = wait(&status)) > 0) {
-				if (DEBUG)
-					tst_resm(TINFO, "Test {%d} exited "
-						 "status 0x%0x", ret, status);
-				if (status != 0) {
-					fail = 1;
-				}
-			}
+			tst_resm(TFAIL,
+				"setpgid FAILED, expect EACCES got %d", errno);
+		}
+
+		if (wait(&status) < 0)
+			tst_resm(TFAIL | TERRNO, "wait() failed");
+
+		if ((!WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) {
+			if (DEBUG)
+				tst_resm(TINFO, "child exited with status %d",
+					WEXITSTATUS(status));
+
+			tst_resm(TFAIL, "child failed");
 		}
 	}
+
 	cleanup();
 	tst_exit();
-
 }
 
-/*
- * do_child()
- */
-void do_child()
+static void do_child(void)
 {
 	int exno = 0;
 
@@ -171,15 +177,15 @@ void do_child()
 		tst_resm(TFAIL, "setsid failed, errno :%d", errno);
 		exno = 1;
 	}
+
+	kill(getppid(), SIGUSR1);
+
 	sleep(2);
+
 	exit(exno);
 }
 
-/*
- * setup()
- * 	performs all ONE TIME setup for this test
- */
-void setup(void)
+static void setup(void)
 {
 
 	tst_sig(FORK, DEF_HANDLER, cleanup);
@@ -189,17 +195,7 @@ void setup(void)
 	TEST_PAUSE;
 }
 
-/*
- * cleanup()
- * 	performs all the ONE TIME cleanup for this test at completion
- * 	or premature exit
- */
-void cleanup(void)
+static void cleanup(void)
 {
-	/*
-	 * print timing status if that option was specified
-	 * print errno log if that option was specified
-	 */
 	TEST_CLEANUP;
-
 }
-- 
1.7.1


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep

[Jan Stancek]

Hi,

it looks race-y after patch too. For example, let assume child runs first:

diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 17fd16f..91f9940 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -82,6 +82,7 @@ int main(int ac, char **av)
 #endif
                }

+               sleep(1);
                if (sigwaitinfo(&sigusr_set, NULL) < 0)
                        tst_brkm(TFAIL | TERRNO, cleanup,
                                "sigwaitinfo() failed");

# ./setpgid03
setpgid03    1  TBROK  :  unexpected signal 10 received (pid = 6763).
setpgid03    2  TBROK  :  Remaining cases broken

It might be easier to use "checkpoint" system, see include/tst_checkpoint.h,
and lib/tests/tst_checkpoint_* for examples.

Regards,
Jan

----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
> To: ltp-list@lists.sourceforge.net
> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
> Sent: Tuesday, 29 October, 2013 8:55:32 AM
> Subject: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep
> 
> I came accross the following rare issue:
> 
> setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
> setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1
> 
> I suppose It may happen if the test host is under stress.
> To prevent this It would be better to use SIGUSR1 for parent-child
> synchronisations.
> 
> Also fixed error passing from children and performed a minor cleanup.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/setpgid/setpgid03.c |  200
>  ++++++++++++-------------
>  1 files changed, 98 insertions(+), 102 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c
> b/testcases/kernel/syscalls/setpgid/setpgid03.c
> index 4429fe4..17fd16f 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
> @@ -1,34 +1,23 @@
>  /*
> + * Copyright (c) International Business Machines  Corp., 2001
>   *
> - *   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 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.
>   *
> - *   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
> + * 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
> - * 	setpgid03.c
> - *
> - * DESCRIPTION
> - * 	Test to check the error and trivial conditions in setpgid system call
> - *
> - * USAGE
> - * 	setuid03
> - *
> - * RESTRICTIONS
> - * 	This test is not completely written in the LTP format - PLEASE FIX!
> + * Test to check the error and trivial conditions in setpgid system call
>   */
>  
>  #define DEBUG 0
> @@ -47,16 +36,16 @@
>  char *TCID = "setpgid03";
>  int TST_TOTAL = 1;
>  
> -void do_child(void);
> -void setup(void);
> -void cleanup(void);
> +static void do_child(void);
> +static void setup(void);
> +static void cleanup(void);
>  
>  int main(int ac, char **av)
>  {
>  	int pid;
> -	int rval, fail = 0;
> -	int ret, status;
> -	int exno = 0;
> +	int rval;
> +	int status;
> +	sigset_t sigusr_set;
>  
>  	int lc;
>  	char *msg;
> @@ -68,14 +57,13 @@ int main(int ac, char **av)
>  	maybe_run_child(&do_child, "");
>  #endif
>  
> -	/*
> -	 * perform global setup for the test
> -	 */
>  	setup();
>  
> +	sigemptyset(&sigusr_set);
> +	sigaddset(&sigusr_set, SIGUSR1);
> +
>  	for (lc = 0; TEST_LOOPING(lc); lc++) {
>  
> -		/* reset tst_count in case we are looping */
>  		tst_count = 0;
>  
>  //test1:
> @@ -84,7 +72,7 @@ int main(int ac, char **av)
>  			tst_brkm(TBROK, cleanup, "fork() failed");
>  		}
>  
> -		if (pid == 0) {	/* child */
> +		if (pid == 0) {
>  #ifdef UCLINUX
>  			if (self_exec(av[0], "") < 0) {
>  				tst_brkm(TBROK, cleanup, "self_exec failed");
> @@ -92,35 +80,33 @@ int main(int ac, char **av)
>  #else
>  			do_child();
>  #endif
> -		} else {	/* parent */
> -			sleep(1);
> -			rval = setpgid(pid, getppid());
> -			if (errno == EPERM) {
> -				tst_resm(TPASS, "setpgid SUCCESS to set "
> -					 "errno to EPERM");
> -			} else {
> -				tst_resm(TFAIL, "setpgid FAILED, "
> -					 "expect %d, return %d", EPERM, errno);
> -				fail = 1;
> -			}
> -			sleep(1);
> -			if ((ret = wait(&status)) > 0) {
> -				if (DEBUG)
> -					tst_resm(TINFO, "Test {%d} exited "
> -						 "status 0x%0x", ret, status);
> -
> -				if (status != 0) {
> -					fail = 1;
> -				}
> -			}
>  		}
> -		if (DEBUG) {
> -			if (fail || exno) {
> -				tst_resm(TINFO, "Test test 1: FAILED");
> -			} else {
> -				tst_resm(TINFO, "Test test 1: PASSED");
> -			}
> +
> +		if (sigwaitinfo(&sigusr_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigwaitinfo() failed");
> +
> +		rval = setpgid(pid, getppid());
> +		if (errno == EPERM) {
> +			tst_resm(TPASS,
> +				"setpgid SUCCESS to set errno to EPERM");
> +		} else {
> +			tst_resm(TFAIL,
> +				"setpgid FAILED, expect %d, return %d",
> +				EPERM, errno);
> +		}
> +
> +		if (wait(&status) < 0)
> +			tst_resm(TFAIL | TERRNO, "wait() failed");
> +
> +		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) {
> +			if (DEBUG)
> +				tst_resm(TINFO, "child exited with status %d",
> +					WEXITSTATUS(status));
> +
> +			tst_resm(TFAIL, "child failed");
>  		}
> +
>  //test2:
>  		/*
>  		 * Value of pid matches the pid of the child process and
> @@ -131,39 +117,59 @@ int main(int ac, char **av)
>  
>  		}
>  		if (pid == 0) {
> -			if (execlp("sleep", "sleep", "3", NULL) < 0) {
> -				perror("exec failed");
> +			char buf[10];
> +			int ret;
> +
> +			ret = snprintf(buf, 10, "%d", getppid());
> +			if ((ret >= 10) || (ret <= 0)) {
> +				printf("snprintf() failed\n");
> +
> +				exit(126);
>  			}
> +
> +			const char *args[4 + 1];
> +			args[0] = "kill";
> +			args[1] = "-s";
> +			args[2] = "SIGUSR1";
> +			args[3] = buf;
> +			args[4] = NULL;
> +
> +			if (execvp(args[0], (char *const *)args) < 0)
> +				perror("exec failed\n");
> +
>  			exit(127);
> +		}
> +
> +		if (sigwaitinfo(&sigusr_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigwaitinfo() failed");
> +
> +		rval = setpgid(pid, getppid());
> +		if (errno == EACCES) {
> +			tst_resm(TPASS,
> +				"setpgid SUCCEEDED to set errno to EACCES");
>  		} else {
> -			sleep(1);
> -			rval = setpgid(pid, getppid());
> -			if (errno == EACCES) {
> -				tst_resm(TPASS, "setpgid SUCCEEDED to set "
> -					 "errno to EACCES");
> -			} else {
> -				tst_resm(TFAIL, "setpgid FAILED, expect EACCES "
> -					 "got %d", errno);
> -			}
> -			if ((ret = wait(&status)) > 0) {
> -				if (DEBUG)
> -					tst_resm(TINFO, "Test {%d} exited "
> -						 "status 0x%0x", ret, status);
> -				if (status != 0) {
> -					fail = 1;
> -				}
> -			}
> +			tst_resm(TFAIL,
> +				"setpgid FAILED, expect EACCES got %d", errno);
> +		}
> +
> +		if (wait(&status) < 0)
> +			tst_resm(TFAIL | TERRNO, "wait() failed");
> +
> +		if ((!WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) {
> +			if (DEBUG)
> +				tst_resm(TINFO, "child exited with status %d",
> +					WEXITSTATUS(status));
> +
> +			tst_resm(TFAIL, "child failed");
>  		}
>  	}
> +
>  	cleanup();
>  	tst_exit();
> -
>  }
>  
> -/*
> - * do_child()
> - */
> -void do_child()
> +static void do_child(void)
>  {
>  	int exno = 0;
>  
> @@ -171,15 +177,15 @@ void do_child()
>  		tst_resm(TFAIL, "setsid failed, errno :%d", errno);
>  		exno = 1;
>  	}
> +
> +	kill(getppid(), SIGUSR1);
> +
>  	sleep(2);
> +
>  	exit(exno);
>  }
>  
> -/*
> - * setup()
> - * 	performs all ONE TIME setup for this test
> - */
> -void setup(void)
> +static void setup(void)
>  {
>  
>  	tst_sig(FORK, DEF_HANDLER, cleanup);
> @@ -189,17 +195,7 @@ void setup(void)
>  	TEST_PAUSE;
>  }
>  
> -/*
> - * cleanup()
> - * 	performs all the ONE TIME cleanup for this test at completion
> - * 	or premature exit
> - */
> -void cleanup(void)
> +static void cleanup(void)
>  {
> -	/*
> -	 * print timing status if that option was specified
> -	 * print errno log if that option was specified
> -	 */
>  	TEST_CLEANUP;
> -
>  }
> --
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> Android is increasing in popularity, but the open development platform that
> developers love is also attractive to malware creators. Download this white
> paper to learn more about secure code signing practices that can help keep
> Android apps secure.
> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep

[Stanislav Kholmanskikh]

On 10/29/2013 02:30 PM, Jan Stancek wrote:
> Hi,
Hi.

>
> it looks race-y after patch too. For example, let assume child runs first:
>
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
> index 17fd16f..91f9940 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
> @@ -82,6 +82,7 @@ int main(int ac, char **av)
>   #endif
>                  }
>
> +               sleep(1);
>                  if (sigwaitinfo(&sigusr_set, NULL) < 0)
>                          tst_brkm(TFAIL | TERRNO, cleanup,
>                                  "sigwaitinfo() failed");
>
> # ./setpgid03
> setpgid03    1  TBROK  :  unexpected signal 10 received (pid = 6763).
> setpgid03    2  TBROK  :  Remaining cases broken
Yes... I can't realize how I wanted to prevent this race if I put 
sigwaitinfo() after fork().... :(

Sorry.


>
> It might be easier to use "checkpoint" system, see include/tst_checkpoint.h,
> and lib/tests/tst_checkpoint_* for examples.
Thank you. I will take a look.

>
> Regards,
> Jan
>
> ----- Original Message -----
>> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
>> To: ltp-list@lists.sourceforge.net
>> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
>> Sent: Tuesday, 29 October, 2013 8:55:32 AM
>> Subject: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep
>>
>> I came accross the following rare issue:
>>
>> setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
>> setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1
>>
>> I suppose It may happen if the test host is under stress.
>> To prevent this It would be better to use SIGUSR1 for parent-child
>> synchronisations.
>>
>> Also fixed error passing from children and performed a minor cleanup.
>>
>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> ---
>>   testcases/kernel/syscalls/setpgid/setpgid03.c |  200
>>   ++++++++++++-------------
>>   1 files changed, 98 insertions(+), 102 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c
>> b/testcases/kernel/syscalls/setpgid/setpgid03.c
>> index 4429fe4..17fd16f 100644
>> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
>> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
>> @@ -1,34 +1,23 @@
>>   /*
>> + * Copyright (c) International Business Machines  Corp., 2001
>>    *
>> - *   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 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.
>>    *
>> - *   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
>> + * 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
>> - * 	setpgid03.c
>> - *
>> - * DESCRIPTION
>> - * 	Test to check the error and trivial conditions in setpgid system call
>> - *
>> - * USAGE
>> - * 	setuid03
>> - *
>> - * RESTRICTIONS
>> - * 	This test is not completely written in the LTP format - PLEASE FIX!
>> + * Test to check the error and trivial conditions in setpgid system call
>>    */
>>   
>>   #define DEBUG 0
>> @@ -47,16 +36,16 @@
>>   char *TCID = "setpgid03";
>>   int TST_TOTAL = 1;
>>   
>> -void do_child(void);
>> -void setup(void);
>> -void cleanup(void);
>> +static void do_child(void);
>> +static void setup(void);
>> +static void cleanup(void);
>>   
>>   int main(int ac, char **av)
>>   {
>>   	int pid;
>> -	int rval, fail = 0;
>> -	int ret, status;
>> -	int exno = 0;
>> +	int rval;
>> +	int status;
>> +	sigset_t sigusr_set;
>>   
>>   	int lc;
>>   	char *msg;
>> @@ -68,14 +57,13 @@ int main(int ac, char **av)
>>   	maybe_run_child(&do_child, "");
>>   #endif
>>   
>> -	/*
>> -	 * perform global setup for the test
>> -	 */
>>   	setup();
>>   
>> +	sigemptyset(&sigusr_set);
>> +	sigaddset(&sigusr_set, SIGUSR1);
>> +
>>   	for (lc = 0; TEST_LOOPING(lc); lc++) {
>>   
>> -		/* reset tst_count in case we are looping */
>>   		tst_count = 0;
>>   
>>   //test1:
>> @@ -84,7 +72,7 @@ int main(int ac, char **av)
>>   			tst_brkm(TBROK, cleanup, "fork() failed");
>>   		}
>>   
>> -		if (pid == 0) {	/* child */
>> +		if (pid == 0) {
>>   #ifdef UCLINUX
>>   			if (self_exec(av[0], "") < 0) {
>>   				tst_brkm(TBROK, cleanup, "self_exec failed");
>> @@ -92,35 +80,33 @@ int main(int ac, char **av)
>>   #else
>>   			do_child();
>>   #endif
>> -		} else {	/* parent */
>> -			sleep(1);
>> -			rval = setpgid(pid, getppid());
>> -			if (errno == EPERM) {
>> -				tst_resm(TPASS, "setpgid SUCCESS to set "
>> -					 "errno to EPERM");
>> -			} else {
>> -				tst_resm(TFAIL, "setpgid FAILED, "
>> -					 "expect %d, return %d", EPERM, errno);
>> -				fail = 1;
>> -			}
>> -			sleep(1);
>> -			if ((ret = wait(&status)) > 0) {
>> -				if (DEBUG)
>> -					tst_resm(TINFO, "Test {%d} exited "
>> -						 "status 0x%0x", ret, status);
>> -
>> -				if (status != 0) {
>> -					fail = 1;
>> -				}
>> -			}
>>   		}
>> -		if (DEBUG) {
>> -			if (fail || exno) {
>> -				tst_resm(TINFO, "Test test 1: FAILED");
>> -			} else {
>> -				tst_resm(TINFO, "Test test 1: PASSED");
>> -			}
>> +
>> +		if (sigwaitinfo(&sigusr_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigwaitinfo() failed");
>> +
>> +		rval = setpgid(pid, getppid());
>> +		if (errno == EPERM) {
>> +			tst_resm(TPASS,
>> +				"setpgid SUCCESS to set errno to EPERM");
>> +		} else {
>> +			tst_resm(TFAIL,
>> +				"setpgid FAILED, expect %d, return %d",
>> +				EPERM, errno);
>> +		}
>> +
>> +		if (wait(&status) < 0)
>> +			tst_resm(TFAIL | TERRNO, "wait() failed");
>> +
>> +		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) {
>> +			if (DEBUG)
>> +				tst_resm(TINFO, "child exited with status %d",
>> +					WEXITSTATUS(status));
>> +
>> +			tst_resm(TFAIL, "child failed");
>>   		}
>> +
>>   //test2:
>>   		/*
>>   		 * Value of pid matches the pid of the child process and
>> @@ -131,39 +117,59 @@ int main(int ac, char **av)
>>   
>>   		}
>>   		if (pid == 0) {
>> -			if (execlp("sleep", "sleep", "3", NULL) < 0) {
>> -				perror("exec failed");
>> +			char buf[10];
>> +			int ret;
>> +
>> +			ret = snprintf(buf, 10, "%d", getppid());
>> +			if ((ret >= 10) || (ret <= 0)) {
>> +				printf("snprintf() failed\n");
>> +
>> +				exit(126);
>>   			}
>> +
>> +			const char *args[4 + 1];
>> +			args[0] = "kill";
>> +			args[1] = "-s";
>> +			args[2] = "SIGUSR1";
>> +			args[3] = buf;
>> +			args[4] = NULL;
>> +
>> +			if (execvp(args[0], (char *const *)args) < 0)
>> +				perror("exec failed\n");
>> +
>>   			exit(127);
>> +		}
>> +
>> +		if (sigwaitinfo(&sigusr_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigwaitinfo() failed");
>> +
>> +		rval = setpgid(pid, getppid());
>> +		if (errno == EACCES) {
>> +			tst_resm(TPASS,
>> +				"setpgid SUCCEEDED to set errno to EACCES");
>>   		} else {
>> -			sleep(1);
>> -			rval = setpgid(pid, getppid());
>> -			if (errno == EACCES) {
>> -				tst_resm(TPASS, "setpgid SUCCEEDED to set "
>> -					 "errno to EACCES");
>> -			} else {
>> -				tst_resm(TFAIL, "setpgid FAILED, expect EACCES "
>> -					 "got %d", errno);
>> -			}
>> -			if ((ret = wait(&status)) > 0) {
>> -				if (DEBUG)
>> -					tst_resm(TINFO, "Test {%d} exited "
>> -						 "status 0x%0x", ret, status);
>> -				if (status != 0) {
>> -					fail = 1;
>> -				}
>> -			}
>> +			tst_resm(TFAIL,
>> +				"setpgid FAILED, expect EACCES got %d", errno);
>> +		}
>> +
>> +		if (wait(&status) < 0)
>> +			tst_resm(TFAIL | TERRNO, "wait() failed");
>> +
>> +		if ((!WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) {
>> +			if (DEBUG)
>> +				tst_resm(TINFO, "child exited with status %d",
>> +					WEXITSTATUS(status));
>> +
>> +			tst_resm(TFAIL, "child failed");
>>   		}
>>   	}
>> +
>>   	cleanup();
>>   	tst_exit();
>> -
>>   }
>>   
>> -/*
>> - * do_child()
>> - */
>> -void do_child()
>> +static void do_child(void)
>>   {
>>   	int exno = 0;
>>   
>> @@ -171,15 +177,15 @@ void do_child()
>>   		tst_resm(TFAIL, "setsid failed, errno :%d", errno);
>>   		exno = 1;
>>   	}
>> +
>> +	kill(getppid(), SIGUSR1);
>> +
>>   	sleep(2);
>> +
>>   	exit(exno);
>>   }
>>   
>> -/*
>> - * setup()
>> - * 	performs all ONE TIME setup for this test
>> - */
>> -void setup(void)
>> +static void setup(void)
>>   {
>>   
>>   	tst_sig(FORK, DEF_HANDLER, cleanup);
>> @@ -189,17 +195,7 @@ void setup(void)
>>   	TEST_PAUSE;
>>   }
>>   
>> -/*
>> - * cleanup()
>> - * 	performs all the ONE TIME cleanup for this test at completion
>> - * 	or premature exit
>> - */
>> -void cleanup(void)
>> +static void cleanup(void)
>>   {
>> -	/*
>> -	 * print timing status if that option was specified
>> -	 * print errno log if that option was specified
>> -	 */
>>   	TEST_CLEANUP;
>> -
>>   }
>> --
>> 1.7.1
>>
>>
>> ------------------------------------------------------------------------------
>> Android is increasing in popularity, but the open development platform that
>> developers love is also attractive to malware creators. Download this white
>> paper to learn more about secure code signing practices that can help keep
>> Android apps secure.
>> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Ltp-list mailing list
>> Ltp-list@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep

[Wanlong Gao]

On 10/29/2013 03:55 PM, Stanislav Kholmanskikh wrote:
> I came accross the following rare issue:
> 
> setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
> setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1
> 
> I suppose It may happen if the test host is under stress.
> To prevent this It would be better to use SIGUSR1 for parent-child
> synchronisations.
> 
> Also fixed error passing from children and performed a minor cleanup.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>


And it's better to split the cleanup part to another patch to make
your patch clear.

Thanks,
Wanlong Gao


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH SERIES V2] setpgid03: use SIGUSR1 instead of sleep

[Stanislav Kholmanskikh]

Hi!

Changes since V1:
* Divided cleanup and logic parts of my previous patch into two separate
  patches
* It seems that "checkpoint" system is not appropriate for this case, because
  in test2 we need to use exec(). Therefore I use signals.

Thanks.



------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 1/2] setpgid03: handle children errors and cleanup

[Stanislav Kholmanskikh]

If one of the children exits with non zero value we can't see
it until DEBUG is set. Now if the exit value is not 0, we
print this value with tst_resm(TFAIL).

Also performed minor cleanup.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/setpgid/setpgid03.c |  160 +++++++++----------------
 1 files changed, 58 insertions(+), 102 deletions(-)

diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 4429fe4..2c47bc4 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -1,38 +1,25 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   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 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.
  *
- *   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
+ * 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
- * 	setpgid03.c
- *
- * DESCRIPTION
- * 	Test to check the error and trivial conditions in setpgid system call
- *
- * USAGE
- * 	setuid03
- *
- * RESTRICTIONS
- * 	This test is not completely written in the LTP format - PLEASE FIX!
+ * Test to check the error and trivial conditions in setpgid system call
  */
 
-#define DEBUG 0
-
 #include <wait.h>
 #include <limits.h>
 #include <signal.h>
@@ -47,16 +34,15 @@
 char *TCID = "setpgid03";
 int TST_TOTAL = 1;
 
-void do_child(void);
-void setup(void);
-void cleanup(void);
+static void do_child(void);
+static void setup(void);
+static void cleanup(void);
 
 int main(int ac, char **av)
 {
 	int pid;
-	int rval, fail = 0;
-	int ret, status;
-	int exno = 0;
+	int rval;
+	int status;
 
 	int lc;
 	char *msg;
@@ -68,14 +54,10 @@ int main(int ac, char **av)
 	maybe_run_child(&do_child, "");
 #endif
 
-	/*
-	 * perform global setup for the test
-	 */
 	setup();
 
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 
-		/* reset tst_count in case we are looping */
 		tst_count = 0;
 
 //test1:
@@ -92,35 +74,27 @@ int main(int ac, char **av)
 #else
 			do_child();
 #endif
-		} else {	/* parent */
-			sleep(1);
-			rval = setpgid(pid, getppid());
-			if (errno == EPERM) {
-				tst_resm(TPASS, "setpgid SUCCESS to set "
-					 "errno to EPERM");
-			} else {
-				tst_resm(TFAIL, "setpgid FAILED, "
-					 "expect %d, return %d", EPERM, errno);
-				fail = 1;
-			}
-			sleep(1);
-			if ((ret = wait(&status)) > 0) {
-				if (DEBUG)
-					tst_resm(TINFO, "Test {%d} exited "
-						 "status 0x%0x", ret, status);
-
-				if (status != 0) {
-					fail = 1;
-				}
-			}
 		}
-		if (DEBUG) {
-			if (fail || exno) {
-				tst_resm(TINFO, "Test test 1: FAILED");
-			} else {
-				tst_resm(TINFO, "Test test 1: PASSED");
-			}
+
+		sleep(1);
+		rval = setpgid(pid, getppid());
+		if (errno == EPERM) {
+			tst_resm(TPASS,
+				"setpgid SUCCESS to set errno to EPERM");
+		} else {
+			tst_resm(TFAIL,
+				"setpgid FAILED, expect %d, return %d",
+				EPERM, errno);
 		}
+		sleep(1);
+
+		if (wait(&status) < 0)
+			tst_resm(TFAIL | TERRNO, "wait() for child 1 failed");
+
+		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
+			tst_resm(TFAIL, "child 1 failed with status %d",
+				WEXITSTATUS(status));
+
 //test2:
 		/*
 		 * Value of pid matches the pid of the child process and
@@ -135,51 +109,43 @@ int main(int ac, char **av)
 				perror("exec failed");
 			}
 			exit(127);
+		}
+
+		sleep(1);
+		rval = setpgid(pid, getppid());
+		if (errno == EACCES) {
+			tst_resm(TPASS,
+				"setpgid SUCCEEDED to set errno to EACCES");
 		} else {
-			sleep(1);
-			rval = setpgid(pid, getppid());
-			if (errno == EACCES) {
-				tst_resm(TPASS, "setpgid SUCCEEDED to set "
-					 "errno to EACCES");
-			} else {
-				tst_resm(TFAIL, "setpgid FAILED, expect EACCES "
-					 "got %d", errno);
-			}
-			if ((ret = wait(&status)) > 0) {
-				if (DEBUG)
-					tst_resm(TINFO, "Test {%d} exited "
-						 "status 0x%0x", ret, status);
-				if (status != 0) {
-					fail = 1;
-				}
-			}
+			tst_resm(TFAIL,
+				"setpgid FAILED, expect EACCES got %d", errno);
 		}
+
+		if (wait(&status) < 0)
+			tst_resm(TFAIL | TERRNO, "wait() for child 2 failed");
+
+		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
+			tst_resm(TFAIL, "child 2 failed with status %d",
+				WEXITSTATUS(status));
 	}
+
 	cleanup();
 	tst_exit();
-
 }
 
-/*
- * do_child()
- */
-void do_child()
+static void do_child(void)
 {
 	int exno = 0;
 
 	if (setsid() < 0) {
-		tst_resm(TFAIL, "setsid failed, errno :%d", errno);
+		printf("setsid() failed, errno: %d\n", errno);
 		exno = 1;
 	}
 	sleep(2);
 	exit(exno);
 }
 
-/*
- * setup()
- * 	performs all ONE TIME setup for this test
- */
-void setup(void)
+static void setup(void)
 {
 
 	tst_sig(FORK, DEF_HANDLER, cleanup);
@@ -189,17 +155,7 @@ void setup(void)
 	TEST_PAUSE;
 }
 
-/*
- * cleanup()
- * 	performs all the ONE TIME cleanup for this test at completion
- * 	or premature exit
- */
-void cleanup(void)
+static void cleanup(void)
 {
-	/*
-	 * print timing status if that option was specified
-	 * print errno log if that option was specified
-	 */
 	TEST_CLEANUP;
-
 }
-- 
1.7.1


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep

[Stanislav Kholmanskikh]

I came accross the following rare issue:

setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1

I suppose It may happen if the test host is under stress.

To prevent this It would be better to use SIGUSR1 for parent-child
synchronisations.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/setpgid/setpgid03.c |  123 +++++++++++++++++++++++--
 1 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 2c47bc4..34975b7 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
  *
  * 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
@@ -34,15 +35,20 @@
 char *TCID = "setpgid03";
 int TST_TOTAL = 1;
 
+static pid_t pid = -1;
+
 static void do_child(void);
+static void sigusr_handler(int signum);
+static void term_handler(int signum);
 static void setup(void);
 static void cleanup(void);
 
 int main(int ac, char **av)
 {
-	int pid;
 	int rval;
 	int status;
+	sigset_t sig_set;
+	struct sigaction sa;
 
 	int lc;
 	char *msg;
@@ -56,12 +62,42 @@ int main(int ac, char **av)
 
 	setup();
 
+	/* We need to block these signals before fork()
+	 * because our children may terminate either
+	 * normally (parent receives both SIGUSR1 and SIGCHLD) or
+	 * abnormally (parent receives SIGCHLD only)
+	 */
+	sigemptyset(&sig_set);
+	sigaddset(&sig_set, SIGUSR1);
+	sigaddset(&sig_set, SIGCHLD);
+
+	/* Since we use SIGUSR1 as a control signal It should not
+	 * be an "unexpected" one
+	 */
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_handler = sigusr_handler;
+	if (sigaction(SIGUSR1, &sa, NULL) < 0)
+		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGUSR1) failed");
+
+	/* But for SIGINT, SIGTERM we should kill our children */
+	sa.sa_handler = term_handler;
+	if (sigaction(SIGTERM, &sa, NULL) < 0)
+		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGTERM) failed");
+	if (sigaction(SIGINT, &sa, NULL) < 0)
+		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGINT) failed");
+
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 
 		tst_count = 0;
 
 //test1:
 		/* sid of the calling process is not same */
+
+		if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigprocmask(SIG_BLOCK) in test 1 failed");
+
+
 		if ((pid = FORK_OR_VFORK()) == -1) {
 			tst_brkm(TBROK, cleanup, "fork() failed");
 		}
@@ -76,7 +112,14 @@ int main(int ac, char **av)
 #endif
 		}
 
-		sleep(1);
+		if (sigwaitinfo(&sig_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigwaitinfo() failed");
+
+		if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigprocmask(SIG_UNBLOCK) in test 2 failed");
+
 		rval = setpgid(pid, getppid());
 		if (errno == EPERM) {
 			tst_resm(TPASS,
@@ -86,11 +129,14 @@ int main(int ac, char **av)
 				"setpgid FAILED, expect %d, return %d",
 				EPERM, errno);
 		}
-		sleep(1);
+
+		kill(pid, SIGUSR1);
 
 		if (wait(&status) < 0)
 			tst_resm(TFAIL | TERRNO, "wait() for child 1 failed");
 
+		pid = -1;
+
 		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
 			tst_resm(TFAIL, "child 1 failed with status %d",
 				WEXITSTATUS(status));
@@ -100,18 +146,47 @@ int main(int ac, char **av)
 		 * Value of pid matches the pid of the child process and
 		 * the child process has exec successfully. Error
 		 */
+
+		if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigprocmask(SIG_BLOCK) in test 2 failed");
+
 		if ((pid = FORK_OR_VFORK()) == -1) {
 			tst_resm(TFAIL, "Fork failed");
 
 		}
 		if (pid == 0) {
-			if (execlp("sleep", "sleep", "3", NULL) < 0) {
-				perror("exec failed");
+			char buf[10];
+			int ret;
+
+			ret = snprintf(buf, 10, "%d", getppid());
+			if ((ret >= 10) || (ret <= 0)) {
+				printf("snprintf() failed\n");
+
+				exit(126);
 			}
+
+			const char *args[4 + 1];
+			args[0] = "kill";
+			args[1] = "-s";
+			args[2] = "SIGUSR1";
+			args[3] = buf;
+			args[4] = NULL;
+
+			if (execvp(args[0], (char *const *)args) < 0)
+				perror("execvp() failed\n");
+
 			exit(127);
 		}
 
-		sleep(1);
+		if (sigwaitinfo(&sig_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigwaitinfo() failed");
+
+		if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0)
+			tst_brkm(TFAIL | TERRNO, cleanup,
+				"sigprocmask(SIG_UNBLOCK) in test 2 failed");
+
 		rval = setpgid(pid, getppid());
 		if (errno == EACCES) {
 			tst_resm(TPASS,
@@ -124,6 +199,8 @@ int main(int ac, char **av)
 		if (wait(&status) < 0)
 			tst_resm(TFAIL | TERRNO, "wait() for child 2 failed");
 
+		pid = -1;
+
 		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
 			tst_resm(TFAIL, "child 2 failed with status %d",
 				WEXITSTATUS(status));
@@ -136,18 +213,48 @@ int main(int ac, char **av)
 static void do_child(void)
 {
 	int exno = 0;
+	sigset_t set;
 
 	if (setsid() < 0) {
 		printf("setsid() failed, errno: %d\n", errno);
 		exno = 1;
 	}
-	sleep(2);
+
+	kill(getppid(), SIGUSR1);
+
+	sigemptyset(&set);
+	sigaddset(&set, SIGUSR1);
+	if (sigprocmask(SIG_UNBLOCK, &set, NULL) < 0) {
+		printf("sigprocmask() in child failed: %d\n", errno);
+
+		exit(1);
+	}
+
+	pause();
+
 	exit(exno);
 }
 
-static void setup(void)
+static void sigusr_handler(int signum)
 {
+}
+
+static void term_handler(int signum)
+{
+	if (pid == 0) {
+		printf("Child received signal %d\n", signum);
 
+		exit(1);
+	} else {
+		if (pid > 0)
+			kill(pid, signum);
+
+		tst_brkm(TFAIL, cleanup, "Signal %d received. Exiting", signum);
+	}
+}
+
+static void setup(void)
+{
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	umask(0);
-- 
1.7.1


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep

[Jan Stancek]

Hi,

I think I still see a possible race, try running it with following change:
diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 34975b7..1d478cb 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -221,6 +221,7 @@ static void do_child(void)
        }

        kill(getppid(), SIGUSR1);
+       sleep(1);

        sigemptyset(&set);
        sigaddset(&set, SIGUSR1);

I also noticed that test2 can now call setpgid() on process which
can be zombie at that time. It worked fine on my system (RHEL6.4),
but it made me wonder if we can rely on this behavior. If we can,
then also test1's do_child() can be simplified.

Regards,
Jan

----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
> To: ltp-list@lists.sourceforge.net
> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
> Sent: Wednesday, 30 October, 2013 10:07:38 AM
> Subject: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep
> 
> I came accross the following rare issue:
> 
> setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
> setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1
> 
> I suppose It may happen if the test host is under stress.
> 
> To prevent this It would be better to use SIGUSR1 for parent-child
> synchronisations.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/setpgid/setpgid03.c |  123
>  +++++++++++++++++++++++--
>  1 files changed, 115 insertions(+), 8 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c
> b/testcases/kernel/syscalls/setpgid/setpgid03.c
> index 2c47bc4..34975b7 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) International Business Machines  Corp., 2001
> + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
>   *
>   * 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
> @@ -34,15 +35,20 @@
>  char *TCID = "setpgid03";
>  int TST_TOTAL = 1;
>  
> +static pid_t pid = -1;
> +
>  static void do_child(void);
> +static void sigusr_handler(int signum);
> +static void term_handler(int signum);
>  static void setup(void);
>  static void cleanup(void);
>  
>  int main(int ac, char **av)
>  {
> -	int pid;
>  	int rval;
>  	int status;
> +	sigset_t sig_set;
> +	struct sigaction sa;
>  
>  	int lc;
>  	char *msg;
> @@ -56,12 +62,42 @@ int main(int ac, char **av)
>  
>  	setup();
>  
> +	/* We need to block these signals before fork()
> +	 * because our children may terminate either
> +	 * normally (parent receives both SIGUSR1 and SIGCHLD) or
> +	 * abnormally (parent receives SIGCHLD only)
> +	 */
> +	sigemptyset(&sig_set);
> +	sigaddset(&sig_set, SIGUSR1);
> +	sigaddset(&sig_set, SIGCHLD);
> +
> +	/* Since we use SIGUSR1 as a control signal It should not
> +	 * be an "unexpected" one
> +	 */
> +	memset(&sa, 0, sizeof(sa));
> +	sa.sa_handler = sigusr_handler;
> +	if (sigaction(SIGUSR1, &sa, NULL) < 0)
> +		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGUSR1) failed");
> +
> +	/* But for SIGINT, SIGTERM we should kill our children */
> +	sa.sa_handler = term_handler;
> +	if (sigaction(SIGTERM, &sa, NULL) < 0)
> +		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGTERM) failed");
> +	if (sigaction(SIGINT, &sa, NULL) < 0)
> +		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGINT) failed");
> +
>  	for (lc = 0; TEST_LOOPING(lc); lc++) {
>  
>  		tst_count = 0;
>  
>  //test1:
>  		/* sid of the calling process is not same */
> +
> +		if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigprocmask(SIG_BLOCK) in test 1 failed");
> +
> +
>  		if ((pid = FORK_OR_VFORK()) == -1) {
>  			tst_brkm(TBROK, cleanup, "fork() failed");
>  		}
> @@ -76,7 +112,14 @@ int main(int ac, char **av)
>  #endif
>  		}
>  
> -		sleep(1);
> +		if (sigwaitinfo(&sig_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigwaitinfo() failed");
> +
> +		if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigprocmask(SIG_UNBLOCK) in test 2 failed");
> +
>  		rval = setpgid(pid, getppid());
>  		if (errno == EPERM) {
>  			tst_resm(TPASS,
> @@ -86,11 +129,14 @@ int main(int ac, char **av)
>  				"setpgid FAILED, expect %d, return %d",
>  				EPERM, errno);
>  		}
> -		sleep(1);
> +
> +		kill(pid, SIGUSR1);
>  
>  		if (wait(&status) < 0)
>  			tst_resm(TFAIL | TERRNO, "wait() for child 1 failed");
>  
> +		pid = -1;
> +
>  		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
>  			tst_resm(TFAIL, "child 1 failed with status %d",
>  				WEXITSTATUS(status));
> @@ -100,18 +146,47 @@ int main(int ac, char **av)
>  		 * Value of pid matches the pid of the child process and
>  		 * the child process has exec successfully. Error
>  		 */
> +
> +		if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigprocmask(SIG_BLOCK) in test 2 failed");
> +
>  		if ((pid = FORK_OR_VFORK()) == -1) {
>  			tst_resm(TFAIL, "Fork failed");
>  
>  		}
>  		if (pid == 0) {
> -			if (execlp("sleep", "sleep", "3", NULL) < 0) {
> -				perror("exec failed");
> +			char buf[10];
> +			int ret;
> +
> +			ret = snprintf(buf, 10, "%d", getppid());
> +			if ((ret >= 10) || (ret <= 0)) {
> +				printf("snprintf() failed\n");
> +
> +				exit(126);
>  			}
> +
> +			const char *args[4 + 1];
> +			args[0] = "kill";
> +			args[1] = "-s";
> +			args[2] = "SIGUSR1";
> +			args[3] = buf;
> +			args[4] = NULL;
> +
> +			if (execvp(args[0], (char *const *)args) < 0)
> +				perror("execvp() failed\n");
> +
>  			exit(127);
>  		}
>  
> -		sleep(1);
> +		if (sigwaitinfo(&sig_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigwaitinfo() failed");
> +
> +		if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0)
> +			tst_brkm(TFAIL | TERRNO, cleanup,
> +				"sigprocmask(SIG_UNBLOCK) in test 2 failed");
> +
>  		rval = setpgid(pid, getppid());
>  		if (errno == EACCES) {
>  			tst_resm(TPASS,
> @@ -124,6 +199,8 @@ int main(int ac, char **av)
>  		if (wait(&status) < 0)
>  			tst_resm(TFAIL | TERRNO, "wait() for child 2 failed");
>  
> +		pid = -1;
> +
>  		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
>  			tst_resm(TFAIL, "child 2 failed with status %d",
>  				WEXITSTATUS(status));
> @@ -136,18 +213,48 @@ int main(int ac, char **av)
>  static void do_child(void)
>  {
>  	int exno = 0;
> +	sigset_t set;
>  
>  	if (setsid() < 0) {
>  		printf("setsid() failed, errno: %d\n", errno);
>  		exno = 1;
>  	}
> -	sleep(2);
> +
> +	kill(getppid(), SIGUSR1);
> +
> +	sigemptyset(&set);
> +	sigaddset(&set, SIGUSR1);
> +	if (sigprocmask(SIG_UNBLOCK, &set, NULL) < 0) {
> +		printf("sigprocmask() in child failed: %d\n", errno);
> +
> +		exit(1);
> +	}
> +
> +	pause();
> +
>  	exit(exno);
>  }
>  
> -static void setup(void)
> +static void sigusr_handler(int signum)
>  {
> +}
> +
> +static void term_handler(int signum)
> +{
> +	if (pid == 0) {
> +		printf("Child received signal %d\n", signum);
>  
> +		exit(1);
> +	} else {
> +		if (pid > 0)
> +			kill(pid, signum);
> +
> +		tst_brkm(TFAIL, cleanup, "Signal %d received. Exiting", signum);
> +	}
> +}
> +
> +static void setup(void)
> +{
>  	tst_sig(FORK, DEF_HANDLER, cleanup);
>  
>  	umask(0);
> --
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> Android is increasing in popularity, but the open development platform that
> developers love is also attractive to malware creators. Download this white
> paper to learn more about secure code signing practices that can help keep
> Android apps secure.
> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep

[Stanislav Kholmanskikh]

On 10/30/2013 05:45 PM, Jan Stancek wrote:
> Hi,
>
> I think I still see a possible race, try running it with following change:
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
> index 34975b7..1d478cb 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
> @@ -221,6 +221,7 @@ static void do_child(void)
>          }
>
>          kill(getppid(), SIGUSR1);
> +       sleep(1);
>
>          sigemptyset(&set);
>          sigaddset(&set, SIGUSR1);
It seems that this helps:
diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c 
b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 34975b7..e5bf1cd 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -36,6 +36,7 @@ char *TCID = "setpgid03";
  int TST_TOTAL = 1;

  static pid_t pid = -1;
+static volatile int exit_child;

  static void do_child(void);
  static void sigusr_handler(int signum);
@@ -230,13 +231,15 @@ static void do_child(void)
                 exit(1);
         }

-       pause();
+       while(!exit_child) {
+       }

         exit(exno);
  }

  static void sigusr_handler(int signum)
  {
+       exit_child = 1;
  }


> I also noticed that test2 can now call setpgid() on process which
> can be zombie at that time. It worked fine on my system (RHEL6.4),
> but it made me wonder if we can rely on this behavior. If we can,
> then also test1's do_child() can be simplified.
I tried to do exit() in do_child() before setpgid() was executed in 
parent and
setpgid() returned EPERM. I.e. test1 succeeds in that scenario.

But I'm not sure if we can rely on that. I would prefer not to rely...

Maybe It would be better to write a standalone program (instead of 
/bin/kill)
which will send SIGUSR1 signal to its parent and then wait to get 
SIGUSR1 signal from
parent and do exit() after that.

How do you think?

Thank you.

> Regards,
> Jan
>
> ----- Original Message -----
>> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
>> To: ltp-list@lists.sourceforge.net
>> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
>> Sent: Wednesday, 30 October, 2013 10:07:38 AM
>> Subject: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep
>>
>> I came accross the following rare issue:
>>
>> setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
>> setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1
>>
>> I suppose It may happen if the test host is under stress.
>>
>> To prevent this It would be better to use SIGUSR1 for parent-child
>> synchronisations.
>>
>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> ---
>>   testcases/kernel/syscalls/setpgid/setpgid03.c |  123
>>   +++++++++++++++++++++++--
>>   1 files changed, 115 insertions(+), 8 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c
>> b/testcases/kernel/syscalls/setpgid/setpgid03.c
>> index 2c47bc4..34975b7 100644
>> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
>> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
>> @@ -1,5 +1,6 @@
>>   /*
>>    * Copyright (c) International Business Machines  Corp., 2001
>> + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
>>    *
>>    * 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
>> @@ -34,15 +35,20 @@
>>   char *TCID = "setpgid03";
>>   int TST_TOTAL = 1;
>>   
>> +static pid_t pid = -1;
>> +
>>   static void do_child(void);
>> +static void sigusr_handler(int signum);
>> +static void term_handler(int signum);
>>   static void setup(void);
>>   static void cleanup(void);
>>   
>>   int main(int ac, char **av)
>>   {
>> -	int pid;
>>   	int rval;
>>   	int status;
>> +	sigset_t sig_set;
>> +	struct sigaction sa;
>>   
>>   	int lc;
>>   	char *msg;
>> @@ -56,12 +62,42 @@ int main(int ac, char **av)
>>   
>>   	setup();
>>   
>> +	/* We need to block these signals before fork()
>> +	 * because our children may terminate either
>> +	 * normally (parent receives both SIGUSR1 and SIGCHLD) or
>> +	 * abnormally (parent receives SIGCHLD only)
>> +	 */
>> +	sigemptyset(&sig_set);
>> +	sigaddset(&sig_set, SIGUSR1);
>> +	sigaddset(&sig_set, SIGCHLD);
>> +
>> +	/* Since we use SIGUSR1 as a control signal It should not
>> +	 * be an "unexpected" one
>> +	 */
>> +	memset(&sa, 0, sizeof(sa));
>> +	sa.sa_handler = sigusr_handler;
>> +	if (sigaction(SIGUSR1, &sa, NULL) < 0)
>> +		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGUSR1) failed");
>> +
>> +	/* But for SIGINT, SIGTERM we should kill our children */
>> +	sa.sa_handler = term_handler;
>> +	if (sigaction(SIGTERM, &sa, NULL) < 0)
>> +		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGTERM) failed");
>> +	if (sigaction(SIGINT, &sa, NULL) < 0)
>> +		tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGINT) failed");
>> +
>>   	for (lc = 0; TEST_LOOPING(lc); lc++) {
>>   
>>   		tst_count = 0;
>>   
>>   //test1:
>>   		/* sid of the calling process is not same */
>> +
>> +		if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigprocmask(SIG_BLOCK) in test 1 failed");
>> +
>> +
>>   		if ((pid = FORK_OR_VFORK()) == -1) {
>>   			tst_brkm(TBROK, cleanup, "fork() failed");
>>   		}
>> @@ -76,7 +112,14 @@ int main(int ac, char **av)
>>   #endif
>>   		}
>>   
>> -		sleep(1);
>> +		if (sigwaitinfo(&sig_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigwaitinfo() failed");
>> +
>> +		if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigprocmask(SIG_UNBLOCK) in test 2 failed");
>> +
>>   		rval = setpgid(pid, getppid());
>>   		if (errno == EPERM) {
>>   			tst_resm(TPASS,
>> @@ -86,11 +129,14 @@ int main(int ac, char **av)
>>   				"setpgid FAILED, expect %d, return %d",
>>   				EPERM, errno);
>>   		}
>> -		sleep(1);
>> +
>> +		kill(pid, SIGUSR1);
>>   
>>   		if (wait(&status) < 0)
>>   			tst_resm(TFAIL | TERRNO, "wait() for child 1 failed");
>>   
>> +		pid = -1;
>> +
>>   		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
>>   			tst_resm(TFAIL, "child 1 failed with status %d",
>>   				WEXITSTATUS(status));
>> @@ -100,18 +146,47 @@ int main(int ac, char **av)
>>   		 * Value of pid matches the pid of the child process and
>>   		 * the child process has exec successfully. Error
>>   		 */
>> +
>> +		if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigprocmask(SIG_BLOCK) in test 2 failed");
>> +
>>   		if ((pid = FORK_OR_VFORK()) == -1) {
>>   			tst_resm(TFAIL, "Fork failed");
>>   
>>   		}
>>   		if (pid == 0) {
>> -			if (execlp("sleep", "sleep", "3", NULL) < 0) {
>> -				perror("exec failed");
>> +			char buf[10];
>> +			int ret;
>> +
>> +			ret = snprintf(buf, 10, "%d", getppid());
>> +			if ((ret >= 10) || (ret <= 0)) {
>> +				printf("snprintf() failed\n");
>> +
>> +				exit(126);
>>   			}
>> +
>> +			const char *args[4 + 1];
>> +			args[0] = "kill";
>> +			args[1] = "-s";
>> +			args[2] = "SIGUSR1";
>> +			args[3] = buf;
>> +			args[4] = NULL;
>> +
>> +			if (execvp(args[0], (char *const *)args) < 0)
>> +				perror("execvp() failed\n");
>> +
>>   			exit(127);
>>   		}
>>   
>> -		sleep(1);
>> +		if (sigwaitinfo(&sig_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigwaitinfo() failed");
>> +
>> +		if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0)
>> +			tst_brkm(TFAIL | TERRNO, cleanup,
>> +				"sigprocmask(SIG_UNBLOCK) in test 2 failed");
>> +
>>   		rval = setpgid(pid, getppid());
>>   		if (errno == EACCES) {
>>   			tst_resm(TPASS,
>> @@ -124,6 +199,8 @@ int main(int ac, char **av)
>>   		if (wait(&status) < 0)
>>   			tst_resm(TFAIL | TERRNO, "wait() for child 2 failed");
>>   
>> +		pid = -1;
>> +
>>   		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
>>   			tst_resm(TFAIL, "child 2 failed with status %d",
>>   				WEXITSTATUS(status));
>> @@ -136,18 +213,48 @@ int main(int ac, char **av)
>>   static void do_child(void)
>>   {
>>   	int exno = 0;
>> +	sigset_t set;
>>   
>>   	if (setsid() < 0) {
>>   		printf("setsid() failed, errno: %d\n", errno);
>>   		exno = 1;
>>   	}
>> -	sleep(2);
>> +
>> +	kill(getppid(), SIGUSR1);
>> +
>> +	sigemptyset(&set);
>> +	sigaddset(&set, SIGUSR1);
>> +	if (sigprocmask(SIG_UNBLOCK, &set, NULL) < 0) {
>> +		printf("sigprocmask() in child failed: %d\n", errno);
>> +
>> +		exit(1);
>> +	}
>> +
>> +	pause();
>> +
>>   	exit(exno);
>>   }
>>   
>> -static void setup(void)
>> +static void sigusr_handler(int signum)
>>   {
>> +}
>> +
>> +static void term_handler(int signum)
>> +{
>> +	if (pid == 0) {
>> +		printf("Child received signal %d\n", signum);
>>   
>> +		exit(1);
>> +	} else {
>> +		if (pid > 0)
>> +			kill(pid, signum);
>> +
>> +		tst_brkm(TFAIL, cleanup, "Signal %d received. Exiting", signum);
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>>   	tst_sig(FORK, DEF_HANDLER, cleanup);
>>   
>>   	umask(0);
>> --
>> 1.7.1
>>
>>
>> ------------------------------------------------------------------------------
>> Android is increasing in popularity, but the open development platform that
>> developers love is also attractive to malware creators. Download this white
>> paper to learn more about secure code signing practices that can help keep
>> Android apps secure.
>> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Ltp-list mailing list
>> Ltp-list@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep

[chrubis]

Hi!
There are child/parent synchronization primitives in the LTP library.
See include/tst_checkpoint.h and lib/tests/tst_checkpoint_child.c.

All that is needed is to initialize checkpoint, call wait in parent and
once child is ready call signal in child. That will fix the first race.

Once child signals the parent it needs to wait till the parent executes
the test, simple pause() in child and kill() from parent should fix this
part.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep

[chrubis]

Hi!
> 
> But I need an exec() in the second case.

I've overlooked the second case, however checkpoint works for this one
as well. You need to build a helper binary setpgid03_child.c that would
call the signal function and then do the wait.

The checkpoint interface is build on the top of unix sockets so it does
not matter whether the process was forked, execed or started from
termial. All that is needed is correct CWD (which is inherited both on
fork and exec) so that the socked special file is found.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep

[chrubis]

Hi!
> > I've overlooked the second case, however checkpoint works for this one
> > as well. You need to build a helper binary setpgid03_child.c that would
> > call the signal function and then do the wait.
> >
> > The checkpoint interface is build on the top of unix sockets so it does
> > not matter whether the process was forked, execed or started from
> > termial. All that is needed is correct CWD (which is inherited both on
> > fork and exec) so that the socked special file is found.
> >
> There is one issue.
> 
> Look at lib/tests/tst_checkpoint_parent.c.
> 
> If the child exits before calling TST_CHECKPOINT_CHILD_WAIT(&checkpoint) 
> (for example, if it gets a signal), the parent
> will wait at TST_CHECKPOINT_SIGNAL_CHILD(NULL, &checkpoint) (blocking at 
> open(O_WRONLY)) until we kill it manually.

Yes, it will hang. Feel free to send a patch with a timeout.

> I don't think that such situation can occur in this particular test case 
> but if we had more activity (like function calls and some logic) before
> execution of TST_CHECKPOINT_CHILD_WAIT in the child, we would have troubles.

It's unlikely situation but it may happen. Generally any of the tests
may hang (due to bug in libc or kernel for example) and ideally the test
driver (ltp-pan currently) should watch for timeouts and kill such
tests, which is not implemented currently.

I'm working on replacement testdriver when my time allows, I have most
of the major parts working and hope to have working prototype soon
(hopefully till the end of the year).

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout

[Stanislav Kholmanskikh]

If a child exits before opening TST_CHECKPOINT_FIFO
for reading, tst_checkpoint_signal_child() issued from the parent
will block forever.

To handle such situations added timeout logic to tst_checkpoint_signal_child();

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 lib/tst_checkpoint.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 56c86b5..a7936da 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -30,9 +30,54 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <poll.h>
+#include <time.h>
 
 #include "tst_checkpoint.h"
 
+/*
+ * Issue open() on 'path' fifo with O_WRONLY flag and wait for
+ * a reader up to 'timeout' ms.
+ *
+ * Returns:
+ *   0   - timeout has happened
+ *   > 0 - file descriptor
+ *   -1  - an error has occurred (errno is set accordingly)
+ *
+ */
+int open_wronly_timed(const char *path, unsigned int timeout)
+{
+	int fd;
+	int interval_ms = 1; /* how often issue open(O_NONBLOCK) */
+	long timeleft = timeout;
+	int saved_errno = errno;
+
+	struct timespec interval;
+	interval.tv_sec = 0;
+	interval.tv_nsec = interval_ms * 1000000; /* in ns */
+
+	for (;;) {
+		fd = open(path, O_WRONLY | O_NONBLOCK);
+		if (fd < 0) {
+			if ((errno == ENXIO) || (errno == EINTR)) {
+				if (timeleft <= 0) {
+					errno = saved_errno;
+
+				return 0;
+				}
+
+				timeleft -= interval_ms;
+				nanosleep(&interval, NULL);
+
+				continue;
+			}
+
+			return -1;
+		}
+
+		return fd;
+	}
+}
+
 void tst_checkpoint_init(const char *file, const int lineno,
                          struct tst_checkpoint *self)
 {
@@ -195,12 +240,19 @@ void tst_checkpoint_signal_child(const char *file, const int lineno,
 {
 	int ret, fd;
 	
-	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
+	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
 
-	if (fd < 0) {
+	switch (fd) {
+	case 0:
+		tst_brkm(TBROK, cleanup_fn,
+			"Checkpoint timeouted after %u msecs at %s:%d",
+			self->timeout, file, lineno);
+	break;
+	case -1:
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 		         "Failed to open fifo '%s' at %s:%d",
 		         TST_CHECKPOINT_FIFO, file, lineno);
+	break;
 	}
 
 	ret = write(fd, "p", 1);
-- 
1.7.1


------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout

[Jan Stancek]

----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
> To: ltp-list@lists.sourceforge.net
> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
> Sent: Wednesday, 13 November, 2013 8:52:42 AM
> Subject: [LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout
> 
> If a child exits before opening TST_CHECKPOINT_FIFO
> for reading, tst_checkpoint_signal_child() issued from the parent
> will block forever.
> 
> To handle such situations added timeout logic to
> tst_checkpoint_signal_child();
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  lib/tst_checkpoint.c |   56
>  ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 2 deletions(-)

Hi,

I would go with just 2 return values:
  -1 - for error/timeout
  fd - otherwise

fd == 0 is a valid descriptor
we can tell if it was timeout or error by errno code 

Why do we need "saved_errno"?

Regards,
Jan

> 
> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> index 56c86b5..a7936da 100644
> --- a/lib/tst_checkpoint.c
> +++ b/lib/tst_checkpoint.c
> @@ -30,9 +30,54 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <poll.h>
> +#include <time.h>
>  
>  #include "tst_checkpoint.h"
>  
> +/*
> + * Issue open() on 'path' fifo with O_WRONLY flag and wait for
> + * a reader up to 'timeout' ms.
> + *
> + * Returns:
> + *   0   - timeout has happened
> + *   > 0 - file descriptor
> + *   -1  - an error has occurred (errno is set accordingly)
> + *
> + */
> +int open_wronly_timed(const char *path, unsigned int timeout)
> +{
> +	int fd;
> +	int interval_ms = 1; /* how often issue open(O_NONBLOCK) */
> +	long timeleft = timeout;
> +	int saved_errno = errno;
> +
> +	struct timespec interval;
> +	interval.tv_sec = 0;
> +	interval.tv_nsec = interval_ms * 1000000; /* in ns */
> +
> +	for (;;) {
> +		fd = open(path, O_WRONLY | O_NONBLOCK);
> +		if (fd < 0) {
> +			if ((errno == ENXIO) || (errno == EINTR)) {
> +				if (timeleft <= 0) {
> +					errno = saved_errno;
> +
> +				return 0;
> +				}
> +
> +				timeleft -= interval_ms;
> +				nanosleep(&interval, NULL);
> +
> +				continue;
> +			}
> +
> +			return -1;
> +		}
> +
> +		return fd;
> +	}
> +}
> +
>  void tst_checkpoint_init(const char *file, const int lineno,
>                           struct tst_checkpoint *self)
>  {
> @@ -195,12 +240,19 @@ void tst_checkpoint_signal_child(const char *file,
> const int lineno,
>  {
>  	int ret, fd;
>  	
> -	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
> +	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
>  
> -	if (fd < 0) {
> +	switch (fd) {
> +	case 0:
> +		tst_brkm(TBROK, cleanup_fn,
> +			"Checkpoint timeouted after %u msecs at %s:%d",
> +			self->timeout, file, lineno);
> +	break;
> +	case -1:
>  		tst_brkm(TBROK | TERRNO, cleanup_fn,
>  		         "Failed to open fifo '%s' at %s:%d",
>  		         TST_CHECKPOINT_FIFO, file, lineno);
> +	break;
>  	}
>  
>  	ret = write(fd, "p", 1);
> --
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
> OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
> Free app hosting. Or install the open source package on any LAMP server.
> Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
> http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout

[Stanislav Kholmanskikh]

On 11/13/2013 12:46 PM, Jan Stancek wrote:
>
> ----- Original Message -----
>> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
>> To: ltp-list@lists.sourceforge.net
>> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
>> Sent: Wednesday, 13 November, 2013 8:52:42 AM
>> Subject: [LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout
>>
>> If a child exits before opening TST_CHECKPOINT_FIFO
>> for reading, tst_checkpoint_signal_child() issued from the parent
>> will block forever.
>>
>> To handle such situations added timeout logic to
>> tst_checkpoint_signal_child();
>>
>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> ---
>>   lib/tst_checkpoint.c |   56
>>   ++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 54 insertions(+), 2 deletions(-)
> Hi,
>
> I would go with just 2 return values:
>    -1 - for error/timeout
>    fd - otherwise
>
> fd == 0 is a valid descriptor
> we can tell if it was timeout or error by errno code
Hi.

Ok.

> Why do we need "saved_errno"?
Initially I though to use "saved_errno" to handle this situation:
* nanosleep() fails and updates global errno
* open_wronly_timed() returns

But I've looked again on the code and It seems that this situation will 
not happen and so on "saved_errno" is useless...

Thanks.

>
> Regards,
> Jan
>
>> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
>> index 56c86b5..a7936da 100644
>> --- a/lib/tst_checkpoint.c
>> +++ b/lib/tst_checkpoint.c
>> @@ -30,9 +30,54 @@
>>   #include <sys/stat.h>
>>   #include <fcntl.h>
>>   #include <poll.h>
>> +#include <time.h>
>>   
>>   #include "tst_checkpoint.h"
>>   
>> +/*
>> + * Issue open() on 'path' fifo with O_WRONLY flag and wait for
>> + * a reader up to 'timeout' ms.
>> + *
>> + * Returns:
>> + *   0   - timeout has happened
>> + *   > 0 - file descriptor
>> + *   -1  - an error has occurred (errno is set accordingly)
>> + *
>> + */
>> +int open_wronly_timed(const char *path, unsigned int timeout)
>> +{
>> +	int fd;
>> +	int interval_ms = 1; /* how often issue open(O_NONBLOCK) */
>> +	long timeleft = timeout;
>> +	int saved_errno = errno;
>> +
>> +	struct timespec interval;
>> +	interval.tv_sec = 0;
>> +	interval.tv_nsec = interval_ms * 1000000; /* in ns */
>> +
>> +	for (;;) {
>> +		fd = open(path, O_WRONLY | O_NONBLOCK);
>> +		if (fd < 0) {
>> +			if ((errno == ENXIO) || (errno == EINTR)) {
>> +				if (timeleft <= 0) {
>> +					errno = saved_errno;
>> +
>> +				return 0;
>> +				}
>> +
>> +				timeleft -= interval_ms;
>> +				nanosleep(&interval, NULL);
>> +
>> +				continue;
>> +			}
>> +
>> +			return -1;
>> +		}
>> +
>> +		return fd;
>> +	}
>> +}
>> +
>>   void tst_checkpoint_init(const char *file, const int lineno,
>>                            struct tst_checkpoint *self)
>>   {
>> @@ -195,12 +240,19 @@ void tst_checkpoint_signal_child(const char *file,
>> const int lineno,
>>   {
>>   	int ret, fd;
>>   	
>> -	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
>> +	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
>>   
>> -	if (fd < 0) {
>> +	switch (fd) {
>> +	case 0:
>> +		tst_brkm(TBROK, cleanup_fn,
>> +			"Checkpoint timeouted after %u msecs at %s:%d",
>> +			self->timeout, file, lineno);
>> +	break;
>> +	case -1:
>>   		tst_brkm(TBROK | TERRNO, cleanup_fn,
>>   		         "Failed to open fifo '%s' at %s:%d",
>>   		         TST_CHECKPOINT_FIFO, file, lineno);
>> +	break;
>>   	}
>>   
>>   	ret = write(fd, "p", 1);
>> --
>> 1.7.1
>>
>>
>> ------------------------------------------------------------------------------
>> DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
>> OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
>> Free app hosting. Or install the open source package on any LAMP server.
>> Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
>> http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Ltp-list mailing list
>> Ltp-list@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>


------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [RFC PATCH V2] tst_checkpoint_signal_child: implemented timeout

[Stanislav Kholmanskikh]

If a child exits before opening TST_CHECKPOINT_FIFO
for reading, tst_checkpoint_signal_child() issued from the parent
will block forever.

To handle such situations added timeout logic to tst_checkpoint_signal_child();

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 lib/tst_checkpoint.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 56c86b5..842ac74 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -30,9 +30,52 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <poll.h>
+#include <time.h>
 
 #include "tst_checkpoint.h"
 
+/*
+ * Issue open() on 'path' fifo with O_WRONLY flag and wait for
+ * a reader up to 'timeout' ms.
+ *
+ * Returns:
+ *   > 0 - file descriptor
+ *   -1  - an error has occurred (errno is set accordingly)
+ *
+ */
+int open_wronly_timed(const char *path, unsigned int timeout)
+{
+	int fd;
+	int interval_ms = 1; /* how often issue open(O_NONBLOCK) */
+	long timeleft = timeout;
+
+	struct timespec interval;
+	interval.tv_sec = 0;
+	interval.tv_nsec = interval_ms * 1000000; /* in ns */
+
+	for (;;) {
+		fd = open(path, O_WRONLY | O_NONBLOCK);
+		if (fd < 0) {
+			if ((errno == ENXIO) || (errno == EINTR)) {
+				if (timeleft <= 0) {
+					errno = ETIMEDOUT;
+
+					return -1;
+				}
+
+				timeleft -= interval_ms;
+				nanosleep(&interval, NULL);
+
+				continue;
+			}
+
+			return -1;
+		}
+
+		return fd;
+	}
+}
+
 void tst_checkpoint_init(const char *file, const int lineno,
                          struct tst_checkpoint *self)
 {
@@ -195,12 +238,18 @@ void tst_checkpoint_signal_child(const char *file, const int lineno,
 {
 	int ret, fd;
 	
-	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
+	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
 
 	if (fd < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "Failed to open fifo '%s' at %s:%d",
-		         TST_CHECKPOINT_FIFO, file, lineno);
+		if (errno == ETIMEDOUT) {
+			tst_brkm(TBROK, cleanup_fn,
+				"Checkpoint timeouted after %u msecs at %s:%d",
+				self->timeout, file, lineno);
+		} else {
+			tst_brkm(TBROK | TERRNO, cleanup_fn,
+				"Failed to open fifo '%s' at %s:%d",
+				TST_CHECKPOINT_FIFO, file, lineno);
+		}
 	}
 
 	ret = write(fd, "p", 1);
-- 
1.7.1


------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH V2] tst_checkpoint_signal_child: implemented timeout

[chrubis]

Hi!
> If a child exits before opening TST_CHECKPOINT_FIFO
> for reading, tst_checkpoint_signal_child() issued from the parent
> will block forever.
> 
> To handle such situations added timeout logic to tst_checkpoint_signal_child();
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  lib/tst_checkpoint.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> index 56c86b5..842ac74 100644
> --- a/lib/tst_checkpoint.c
> +++ b/lib/tst_checkpoint.c
> @@ -30,9 +30,52 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <poll.h>
> +#include <time.h>
>  
>  #include "tst_checkpoint.h"
>  
> +/*
> + * Issue open() on 'path' fifo with O_WRONLY flag and wait for
> + * a reader up to 'timeout' ms.
> + *
> + * Returns:
> + *   > 0 - file descriptor
> + *   -1  - an error has occurred (errno is set accordingly)
> + *
> + */
> +int open_wronly_timed(const char *path, unsigned int timeout)
> +{
> +	int fd;
> +	int interval_ms = 1; /* how often issue open(O_NONBLOCK) */
> +	long timeleft = timeout;
> +
> +	struct timespec interval;
> +	interval.tv_sec = 0;
> +	interval.tv_nsec = interval_ms * 1000000; /* in ns */
> +
> +	for (;;) {

What about using

	for (i = 0; i < timeout; i += interval) {

> +		fd = open(path, O_WRONLY | O_NONBLOCK);
> +		if (fd < 0) {
> +			if ((errno == ENXIO) || (errno == EINTR)) {
> +				if (timeleft <= 0) {
> +					errno = ETIMEDOUT;
> +
> +					return -1;
> +				}
> +
> +				timeleft -= interval_ms;
> +				nanosleep(&interval, NULL);

What about using usleep() which has simplier interface instead?

> +				continue;
> +			}
> +
> +			return -1;
> +		}
> +
> +		return fd;
> +	}
> +}
> +
>  void tst_checkpoint_init(const char *file, const int lineno,
>                           struct tst_checkpoint *self)
>  {
> @@ -195,12 +238,18 @@ void tst_checkpoint_signal_child(const char *file, const int lineno,
>  {
>  	int ret, fd;
>  	
> -	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
> +	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
>  
>  	if (fd < 0) {
> -		tst_brkm(TBROK | TERRNO, cleanup_fn,
> -		         "Failed to open fifo '%s' at %s:%d",
> -		         TST_CHECKPOINT_FIFO, file, lineno);
> +		if (errno == ETIMEDOUT) {
> +			tst_brkm(TBROK, cleanup_fn,
> +				"Checkpoint timeouted after %u msecs at %s:%d",
> +				self->timeout, file, lineno);
> +		} else {
> +			tst_brkm(TBROK | TERRNO, cleanup_fn,
> +				"Failed to open fifo '%s' at %s:%d",
> +				TST_CHECKPOINT_FIFO, file, lineno);
> +		}

Hmm, there is no need to complicate this part, if you have set the errno
to ETIMEOUTED the tst_brkm() will include the errno and will look like:

foo01	1  TBROK  :  Failed to open fifo 'tst_checkpoint_fifo' at tst_checkopoint.c:211: ERRNO=ETIMEDOUT(110): Connection timed out

Which is in my opinion descriptive enough.


But we need to add EDTIMEDOUT into the tst_res.c fist, actually there are still
some errno values missing, so I will add them them there beforehand.

diff --git a/lib/tst_res.c b/lib/tst_res.c
index f73022b..7d87492 100644
--- a/lib/tst_res.c
+++ b/lib/tst_res.c
@@ -255,6 +255,7 @@ static const char *strerrnodef(int err)
                    PAIR(ENOSYS)
                    PAIR(ENOTEMPTY)
                    PAIR(ELOOP)
+                   PAIR(ETIMEDOUT)
        };
        return pair_lookup(errno_pairs, err);
 }


-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V3 1/2] tst_res: added ETIME, ETIMEDOUT errno values

[Stanislav Kholmanskikh]

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 lib/tst_res.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/tst_res.c b/lib/tst_res.c
index f73022b..698b12c 100644
--- a/lib/tst_res.c
+++ b/lib/tst_res.c
@@ -255,6 +255,8 @@ static const char *strerrnodef(int err)
 		    PAIR(ENOSYS)
 		    PAIR(ENOTEMPTY)
 		    PAIR(ELOOP)
+		    PAIR(ETIMEDOUT)
+		    PAIR(ETIME)
 	};
 	return pair_lookup(errno_pairs, err);
 }
-- 
1.7.1


------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V3 2/2] tst_checkpoint_signal_child: implemented timeout

[Stanislav Kholmanskikh]

If a child exits before opening TST_CHECKPOINT_FIFO
for reading, tst_checkpoint_signal_child() issued from the parent
will block forever.

To handle such situations added timeout logic to tst_checkpoint_signal_child();

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 lib/tst_checkpoint.c |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 56c86b5..e53c499 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -33,6 +33,40 @@
 
 #include "tst_checkpoint.h"
 
+/*
+ * Issue open() on 'path' fifo with O_WRONLY flag and wait for
+ * a reader up to 'timeout' ms.
+ *
+ * Returns:
+ *   > 0 - file descriptor
+ *   -1  - an error has occurred (errno is set accordingly)
+ *
+ */
+int open_wronly_timed(const char *path, unsigned int timeout)
+{
+	int fd;
+	int i;
+	int interval = 1; /* how often issue open(O_NONBLOCK), in ms */
+
+	for (i = 0; i < timeout; i += interval) {
+		fd = open(path, O_WRONLY | O_NONBLOCK);
+		if (fd < 0) {
+			if ((errno == ENXIO) || (errno == EINTR)) {
+				usleep(interval * 1000);
+
+				continue;
+			}
+
+			return -1;
+		}
+
+		return fd;
+	}
+
+	errno = ETIMEDOUT;
+	return -1;
+}
+
 void tst_checkpoint_init(const char *file, const int lineno,
                          struct tst_checkpoint *self)
 {
@@ -195,7 +229,7 @@ void tst_checkpoint_signal_child(const char *file, const int lineno,
 {
 	int ret, fd;
 	
-	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
+	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
 
 	if (fd < 0) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-- 
1.7.1


------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/2] tst_checkpoint_signal_child: implemented timeout

[Jan Stancek]

----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
> To: ltp-list@lists.sourceforge.net
> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
> Sent: Thursday, 14 November, 2013 7:37:57 AM
> Subject: [LTP] [PATCH V3 2/2] tst_checkpoint_signal_child: implemented	timeout
> 
> If a child exits before opening TST_CHECKPOINT_FIFO
> for reading, tst_checkpoint_signal_child() issued from the parent
> will block forever.
> 
> To handle such situations added timeout logic to
> tst_checkpoint_signal_child();
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>

One small nit: the comment should say ">= 0", other than that, it looks good to me:
Reviewed-by: Jan Stancek <jstancek@redhat.com>

I'm sending a follow-up patch (tst_checkpoint_child_exits2), which I used to verify this one.

Regards,
Jan

> ---
>  lib/tst_checkpoint.c |   36 +++++++++++++++++++++++++++++++++++-
>  1 files changed, 35 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> index 56c86b5..e53c499 100644
> --- a/lib/tst_checkpoint.c
> +++ b/lib/tst_checkpoint.c
> @@ -33,6 +33,40 @@
>  
>  #include "tst_checkpoint.h"
>  
> +/*
> + * Issue open() on 'path' fifo with O_WRONLY flag and wait for
> + * a reader up to 'timeout' ms.
> + *
> + * Returns:
> + *   > 0 - file descriptor
> + *   -1  - an error has occurred (errno is set accordingly)
> + *
> + */
> +int open_wronly_timed(const char *path, unsigned int timeout)
> +{
> +	int fd;
> +	int i;
> +	int interval = 1; /* how often issue open(O_NONBLOCK), in ms */
> +
> +	for (i = 0; i < timeout; i += interval) {
> +		fd = open(path, O_WRONLY | O_NONBLOCK);
> +		if (fd < 0) {
> +			if ((errno == ENXIO) || (errno == EINTR)) {
> +				usleep(interval * 1000);
> +
> +				continue;
> +			}
> +
> +			return -1;
> +		}
> +
> +		return fd;
> +	}
> +
> +	errno = ETIMEDOUT;
> +	return -1;
> +}
> +
>  void tst_checkpoint_init(const char *file, const int lineno,
>                           struct tst_checkpoint *self)
>  {
> @@ -195,7 +229,7 @@ void tst_checkpoint_signal_child(const char *file, const
> int lineno,
>  {
>  	int ret, fd;
>  	
> -	fd = open(TST_CHECKPOINT_FIFO, O_WRONLY);
> +	fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout);
>  
>  	if (fd < 0) {
>  		tst_brkm(TBROK | TERRNO, cleanup_fn,
> --
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
> OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
> Free app hosting. Or install the open source package on any LAMP server.
> Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
> http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 1/2] tst_res: added ETIME, ETIMEDOUT errno values

[chrubis]

Hi!
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  lib/tst_res.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/tst_res.c b/lib/tst_res.c
> index f73022b..698b12c 100644
> --- a/lib/tst_res.c
> +++ b/lib/tst_res.c
> @@ -255,6 +255,8 @@ static const char *strerrnodef(int err)
>  		    PAIR(ENOSYS)
>  		    PAIR(ENOTEMPTY)
>  		    PAIR(ELOOP)
> +		    PAIR(ETIMEDOUT)
> +		    PAIR(ETIME)
>  	};
>  	return pair_lookup(errno_pairs, err);
>  }

I've just pushed patch that add all ERRNO values found in headers on
recent enough distribution.

Please check that it compiles at older distros you are using and if not
send a patch with ifdefs as they are around the HWPOISON.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/2] tst_checkpoint_signal_child: implemented timeout

[chrubis]

Hi!
> If a child exits before opening TST_CHECKPOINT_FIFO
> for reading, tst_checkpoint_signal_child() issued from the parent
> will block forever.
> 
> To handle such situations added timeout logic to tst_checkpoint_signal_child();
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>

I'll push this version with the comment fixed to '>=', thanks a lot.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V3 1/2] setpgid03: handle children errors and cleanup

[Stanislav Kholmanskikh]

If one of the children exits with non zero value we can't see
it until DEBUG is set. Now if the exit value is not 0, we
print this value with tst_resm(TFAIL).

Also performed minor cleanup.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/setpgid/setpgid03.c |  160 +++++++++----------------
 1 files changed, 58 insertions(+), 102 deletions(-)

diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 4429fe4..2c47bc4 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -1,38 +1,25 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   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 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.
  *
- *   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
+ * 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
- * 	setpgid03.c
- *
- * DESCRIPTION
- * 	Test to check the error and trivial conditions in setpgid system call
- *
- * USAGE
- * 	setuid03
- *
- * RESTRICTIONS
- * 	This test is not completely written in the LTP format - PLEASE FIX!
+ * Test to check the error and trivial conditions in setpgid system call
  */
 
-#define DEBUG 0
-
 #include <wait.h>
 #include <limits.h>
 #include <signal.h>
@@ -47,16 +34,15 @@
 char *TCID = "setpgid03";
 int TST_TOTAL = 1;
 
-void do_child(void);
-void setup(void);
-void cleanup(void);
+static void do_child(void);
+static void setup(void);
+static void cleanup(void);
 
 int main(int ac, char **av)
 {
 	int pid;
-	int rval, fail = 0;
-	int ret, status;
-	int exno = 0;
+	int rval;
+	int status;
 
 	int lc;
 	char *msg;
@@ -68,14 +54,10 @@ int main(int ac, char **av)
 	maybe_run_child(&do_child, "");
 #endif
 
-	/*
-	 * perform global setup for the test
-	 */
 	setup();
 
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 
-		/* reset tst_count in case we are looping */
 		tst_count = 0;
 
 //test1:
@@ -92,35 +74,27 @@ int main(int ac, char **av)
 #else
 			do_child();
 #endif
-		} else {	/* parent */
-			sleep(1);
-			rval = setpgid(pid, getppid());
-			if (errno == EPERM) {
-				tst_resm(TPASS, "setpgid SUCCESS to set "
-					 "errno to EPERM");
-			} else {
-				tst_resm(TFAIL, "setpgid FAILED, "
-					 "expect %d, return %d", EPERM, errno);
-				fail = 1;
-			}
-			sleep(1);
-			if ((ret = wait(&status)) > 0) {
-				if (DEBUG)
-					tst_resm(TINFO, "Test {%d} exited "
-						 "status 0x%0x", ret, status);
-
-				if (status != 0) {
-					fail = 1;
-				}
-			}
 		}
-		if (DEBUG) {
-			if (fail || exno) {
-				tst_resm(TINFO, "Test test 1: FAILED");
-			} else {
-				tst_resm(TINFO, "Test test 1: PASSED");
-			}
+
+		sleep(1);
+		rval = setpgid(pid, getppid());
+		if (errno == EPERM) {
+			tst_resm(TPASS,
+				"setpgid SUCCESS to set errno to EPERM");
+		} else {
+			tst_resm(TFAIL,
+				"setpgid FAILED, expect %d, return %d",
+				EPERM, errno);
 		}
+		sleep(1);
+
+		if (wait(&status) < 0)
+			tst_resm(TFAIL | TERRNO, "wait() for child 1 failed");
+
+		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
+			tst_resm(TFAIL, "child 1 failed with status %d",
+				WEXITSTATUS(status));
+
 //test2:
 		/*
 		 * Value of pid matches the pid of the child process and
@@ -135,51 +109,43 @@ int main(int ac, char **av)
 				perror("exec failed");
 			}
 			exit(127);
+		}
+
+		sleep(1);
+		rval = setpgid(pid, getppid());
+		if (errno == EACCES) {
+			tst_resm(TPASS,
+				"setpgid SUCCEEDED to set errno to EACCES");
 		} else {
-			sleep(1);
-			rval = setpgid(pid, getppid());
-			if (errno == EACCES) {
-				tst_resm(TPASS, "setpgid SUCCEEDED to set "
-					 "errno to EACCES");
-			} else {
-				tst_resm(TFAIL, "setpgid FAILED, expect EACCES "
-					 "got %d", errno);
-			}
-			if ((ret = wait(&status)) > 0) {
-				if (DEBUG)
-					tst_resm(TINFO, "Test {%d} exited "
-						 "status 0x%0x", ret, status);
-				if (status != 0) {
-					fail = 1;
-				}
-			}
+			tst_resm(TFAIL,
+				"setpgid FAILED, expect EACCES got %d", errno);
 		}
+
+		if (wait(&status) < 0)
+			tst_resm(TFAIL | TERRNO, "wait() for child 2 failed");
+
+		if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0))
+			tst_resm(TFAIL, "child 2 failed with status %d",
+				WEXITSTATUS(status));
 	}
+
 	cleanup();
 	tst_exit();
-
 }
 
-/*
- * do_child()
- */
-void do_child()
+static void do_child(void)
 {
 	int exno = 0;
 
 	if (setsid() < 0) {
-		tst_resm(TFAIL, "setsid failed, errno :%d", errno);
+		printf("setsid() failed, errno: %d\n", errno);
 		exno = 1;
 	}
 	sleep(2);
 	exit(exno);
 }
 
-/*
- * setup()
- * 	performs all ONE TIME setup for this test
- */
-void setup(void)
+static void setup(void)
 {
 
 	tst_sig(FORK, DEF_HANDLER, cleanup);
@@ -189,17 +155,7 @@ void setup(void)
 	TEST_PAUSE;
 }
 
-/*
- * cleanup()
- * 	performs all the ONE TIME cleanup for this test at completion
- * 	or premature exit
- */
-void cleanup(void)
+static void cleanup(void)
 {
-	/*
-	 * print timing status if that option was specified
-	 * print errno log if that option was specified
-	 */
 	TEST_CLEANUP;
-
 }
-- 
1.7.1


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V3 2/2] setpgid03: use tst_checkpoint interface

[Stanislav Kholmanskikh]

I came accross the following rare issue:

setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1

I suppose it may happen if the test host is under stress.

To prevent this it would be better to use tst_checkpoint interface
for parent-child synchronisations.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/.gitignore               |    1 +
 testcases/kernel/syscalls/setpgid/setpgid03.c      |   57 ++++++++++++++++----
 .../kernel/syscalls/setpgid/setpgid03_child.c      |   31 +++++++++++
 3 files changed, 78 insertions(+), 11 deletions(-)
 create mode 100644 testcases/kernel/syscalls/setpgid/setpgid03_child.c

diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index bbefb25..6ce8900 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -777,6 +777,7 @@
 /setpgid/setpgid01
 /setpgid/setpgid02
 /setpgid/setpgid03
+/setpgid/setpgid03_child
 /setpgrp/setpgrp01
 /setpgrp/setpgrp02
 /setpriority/setpriority01
diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index 2c47bc4..b9ecddd 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
  *
  * 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
@@ -31,12 +32,17 @@
 #include "test.h"
 #include "usctest.h"
 
+#define TEST_APP "setpgid03_child"
+
 char *TCID = "setpgid03";
 int TST_TOTAL = 1;
 
+static struct tst_checkpoint checkpoint;
+
 static void do_child(void);
 static void setup(void);
 static void cleanup(void);
+static void alarm_handler(int signo);
 
 int main(int ac, char **av)
 {
@@ -76,7 +82,7 @@ int main(int ac, char **av)
 #endif
 		}
 
-		sleep(1);
+		TST_CHECKPOINT_PARENT_WAIT(cleanup, &checkpoint);
 		rval = setpgid(pid, getppid());
 		if (errno == EPERM) {
 			tst_resm(TPASS,
@@ -86,7 +92,7 @@ int main(int ac, char **av)
 				"setpgid FAILED, expect %d, return %d",
 				EPERM, errno);
 		}
-		sleep(1);
+		TST_CHECKPOINT_SIGNAL_CHILD(cleanup, &checkpoint);
 
 		if (wait(&status) < 0)
 			tst_resm(TFAIL | TERRNO, "wait() for child 1 failed");
@@ -105,13 +111,13 @@ int main(int ac, char **av)
 
 		}
 		if (pid == 0) {
-			if (execlp("sleep", "sleep", "3", NULL) < 0) {
+			if (execlp(TEST_APP, TEST_APP, NULL) < 0)
 				perror("exec failed");
-			}
+
 			exit(127);
 		}
 
-		sleep(1);
+		TST_CHECKPOINT_PARENT_WAIT(cleanup, &checkpoint);
 		rval = setpgid(pid, getppid());
 		if (errno == EACCES) {
 			tst_resm(TPASS,
@@ -120,6 +126,7 @@ int main(int ac, char **av)
 			tst_resm(TFAIL,
 				"setpgid FAILED, expect EACCES got %d", errno);
 		}
+		TST_CHECKPOINT_SIGNAL_CHILD(cleanup, &checkpoint);
 
 		if (wait(&status) < 0)
 			tst_resm(TFAIL | TERRNO, "wait() for child 2 failed");
@@ -133,23 +140,49 @@ int main(int ac, char **av)
 	tst_exit();
 }
 
+static void alarm_handler(int signo)
+{
+	printf("CHILD: checkpoint timed out\n");
+
+	exit(3);
+}
+
 static void do_child(void)
 {
-	int exno = 0;
+	unsigned int timeout = checkpoint.timeout / 1000;
+	struct sigaction sa;
+
+	memset(&sa, 0, sizeof(struct sigaction));
+	sa.sa_handler = alarm_handler;
+
+	if (sigaction(SIGALRM, &sa, NULL) < 0) {
+		printf("CHILD: sigaction() failed, errno: %d\n", errno);
+		exit(1);
+	}
 
 	if (setsid() < 0) {
-		printf("setsid() failed, errno: %d\n", errno);
-		exno = 1;
+		printf("CHILD: setsid() failed, errno: %d\n", errno);
+		exit(2);
 	}
-	sleep(2);
-	exit(exno);
+
+	alarm(timeout);
+	TST_CHECKPOINT_SIGNAL_PARENT(&checkpoint);
+
+	alarm(timeout);
+	TST_CHECKPOINT_CHILD_WAIT(&checkpoint);
+
+	exit(0);
 }
 
 static void setup(void)
 {
-
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
+	tst_tmpdir();
+
+	TST_CHECKPOINT_INIT(&checkpoint);
+	checkpoint.timeout = 10000;
+
 	umask(0);
 
 	TEST_PAUSE;
@@ -157,5 +190,7 @@ static void setup(void)
 
 static void cleanup(void)
 {
+	tst_rmdir();
+
 	TEST_CLEANUP;
 }
diff --git a/testcases/kernel/syscalls/setpgid/setpgid03_child.c b/testcases/kernel/syscalls/setpgid/setpgid03_child.c
new file mode 100644
index 0000000..c4cf892
--- /dev/null
+++ b/testcases/kernel/syscalls/setpgid/setpgid03_child.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
+ *
+ * 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 would 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 the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "test.h"
+
+char *TCID = "setpgid03_child";
+
+static struct tst_checkpoint checkpoint = { .timeout = 10000, .retval = 1};
+
+int main(void)
+{
+	TST_CHECKPOINT_SIGNAL_PARENT(&checkpoint);
+	TST_CHECKPOINT_CHILD_WAIT(&checkpoint);
+
+	return 0;
+}
-- 
1.7.1


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/2] setpgid03: use tst_checkpoint interface

[chrubis]

Hi!
> I came accross the following rare issue:
> 
> setpgid03    1  TPASS  :  setpgid SUCCESS to set errno to EPERM
> setpgid03    2  TFAIL  :  setpgid FAILED, expect EACCES got 1
> 
> I suppose it may happen if the test host is under stress.
> 
> To prevent this it would be better to use tst_checkpoint interface
> for parent-child synchronisations.

Pushed.

I've added a follow up patch that removes the alarm() timeout code in
the child and does a few more fixes.

The timeout should better be handled, if handled at all, in the
checkpoint code. I would not care much about this case because when the
parent is dead does not talk to the child the test will fail anyway...

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/2] setpgid03: use tst_checkpoint interface

[chrubis]

Hi!
> > The timeout should better be handled, if handled at all, in the
> > checkpoint code. I would not care much about this case because when the
> > parent is dead does not talk to the child the test will fail anyway...
> >
> 
> I didn't care about the test failure in such case. I cared about that 
> without alarm() the child will keep running (i.e. blocking on IO) even 
> after the parent is dead.

I've noticed that as well, the child will continue to run and because it
called setsid() the test driver (pan) will not kill it because it left
it's parent process group.

> But I agree that it's not the best place to handle this inside the test 
> case.

I was thinking of adding the alarm() code into the checkpoint child
code. This should fix the problem once for all. But even then this
codepath happens only when something went wrong so this has not big
priority...

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list