[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Han Pingtian]

If continue after the fork failure, a lot of processes may be killed by
the kill() on line 125.

Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
---
 testcases/kernel/syscalls/waitpid/waitpid03.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid03.c b/testcases/kernel/syscalls/waitpid/waitpid03.c
index 2599a92..648870c 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid03.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid03.c
@@ -106,7 +106,8 @@ int main(int argc, char **argv)
 				if (DEBUG)
 					tst_resm(TINFO, "child # %d", ikids);
 			} else if (pid[ikids] == -1) {
-				tst_resm(TFAIL, "cannot open fork #%d", ikids);
+				tst_brkm(TBROK|TERRNO, cleanup,
+					 "cannot open fork #%d", ikids);
 			} else {
 #ifdef UCLINUX
 				if (self_exec(argv[0], "d", ikids) < 0) {
-- 
1.9.3

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Cyril Hrubis]

Hi!
> -				tst_resm(TFAIL, "cannot open fork #%d", ikids);
> +				tst_brkm(TBROK|TERRNO, cleanup,
> +					 "cannot open fork #%d", ikids);

I guess that we should kill any of the allready sleeping children at this point.

If we move the pid[] array to be global we can create a kill_children()
function to send a signal to all pids in pid[] that are > 0 and call it
if fork fails with SIGKILL as well as from the main loop with the
SIGTERM as well.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Han Pingtian]

On Tue, Apr 12, 2016 at 05:52:41PM +0200, Cyril Hrubis wrote:
> Hi!
> > -				tst_resm(TFAIL, "cannot open fork #%d", ikids);
> > +				tst_brkm(TBROK|TERRNO, cleanup,
> > +					 "cannot open fork #%d", ikids);
> 
> I guess that we should kill any of the allready sleeping children at this point.
> 
> If we move the pid[] array to be global we can create a kill_children()
> function to send a signal to all pids in pid[] that are > 0 and call it
> if fork fails with SIGKILL as well as from the main loop with the
> SIGTERM as well.
> 
Thanks. How about this one? I think it is simple and enough for the
purpose.


From 5ec101da361dae875fc3f9293cf4ec65be8bbb93 Mon Sep 17 00:00:00 2001
From: Han Pingtian <hanpt@linux.vnet.ibm.com>
Date: Tue, 12 Apr 2016 11:12:28 +0800
Subject: [PATCH] waitpid/waitpid03: break test if fork failed

If continue after the fork failure, a lot of processes may be killed by
the kill() on line 125.

Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
---
 testcases/kernel/syscalls/waitpid/waitpid03.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid03.c b/testcases/kernel/syscalls/waitpid/waitpid03.c
index 2599a92..ba4be50 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid03.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid03.c
@@ -106,7 +106,13 @@ int main(int argc, char **argv)
 				if (DEBUG)
 					tst_resm(TINFO, "child # %d", ikids);
 			} else if (pid[ikids] == -1) {
-				tst_resm(TFAIL, "cannot open fork #%d", ikids);
+				int i;
+
+				for (i = 1; i < ikids; i++)
+					kill(pid[i], 9);
+
+				tst_brkm(TBROK|TERRNO, cleanup,
+					 "cannot open fork #%d", ikids);
 			} else {
 #ifdef UCLINUX
 				if (self_exec(argv[0], "d", ikids) < 0) {
-- 
1.9.3

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Han Pingtian]

On Tue, Apr 12, 2016 at 05:52:41PM +0200, Cyril Hrubis wrote:
> Hi!
> > -				tst_resm(TFAIL, "cannot open fork #%d", ikids);
> > +				tst_brkm(TBROK|TERRNO, cleanup,
> > +					 "cannot open fork #%d", ikids);
> 
> I guess that we should kill any of the allready sleeping children at this point.
> 
> If we move the pid[] array to be global we can create a kill_children()
> function to send a signal to all pids in pid[] that are > 0 and call it
> if fork fails with SIGKILL as well as from the main loop with the
> SIGTERM as well.
> 
I think this one is better, please have a look. Thanks.


From aff8c1decee88bae37a8460ddce8d10a7a8f3d1a Mon Sep 17 00:00:00 2001
From: Han Pingtian <hanpt@linux.vnet.ibm.com>
Date: Tue, 12 Apr 2016 11:12:28 +0800
Subject: [PATCH] waitpid/waitpid03: break test if fork failed

If continue after the fork failure, a lot of processes may be killed by
the kill() on line 125.

Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
---
 testcases/kernel/syscalls/waitpid/waitpid03.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid03.c b/testcases/kernel/syscalls/waitpid/waitpid03.c
index 2599a92..7e9d45c 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid03.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid03.c
@@ -106,7 +106,16 @@ int main(int argc, char **argv)
 				if (DEBUG)
 					tst_resm(TINFO, "child # %d", ikids);
 			} else if (pid[ikids] == -1) {
-				tst_resm(TFAIL, "cannot open fork #%d", ikids);
+				int i;
+
+				tst_resm(TBROK|TERRNO, "cannot open fork #%d",
+					 ikids);
+
+				for (i = 1; i < ikids; i++)
+					kill(pid[i], 9);
+
+				cleanup();
+				tst_exit();
 			} else {
 #ifdef UCLINUX
 				if (self_exec(argv[0], "d", ikids) < 0) {
-- 
1.9.3

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Cyril Hrubis]

Hi!
> -				tst_resm(TFAIL, "cannot open fork #%d", ikids);
> +				int i;
> +
> +				for (i = 1; i < ikids; i++)
> +					kill(pid[i], 9);
> +
> +				tst_brkm(TBROK|TERRNO, cleanup,
> +					 "cannot open fork #%d", ikids);

The problem is that the errno may get clobbered by the call to the
kill().

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Cyril Hrubis]

Hi!
> Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
> ---
>  testcases/kernel/syscalls/waitpid/waitpid03.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/waitpid/waitpid03.c b/testcases/kernel/syscalls/waitpid/waitpid03.c
> index 2599a92..7e9d45c 100644
> --- a/testcases/kernel/syscalls/waitpid/waitpid03.c
> +++ b/testcases/kernel/syscalls/waitpid/waitpid03.c
> @@ -106,7 +106,16 @@ int main(int argc, char **argv)
>  				if (DEBUG)
>  					tst_resm(TINFO, "child # %d", ikids);
>  			} else if (pid[ikids] == -1) {
> -				tst_resm(TFAIL, "cannot open fork #%d", ikids);
> +				int i;
> +
> +				tst_resm(TBROK|TERRNO, "cannot open fork #%d",
> +					 ikids);
> +
> +				for (i = 1; i < ikids; i++)
> +					kill(pid[i], 9);
> +
> +				cleanup();
> +				tst_exit();

This one is ugly. Why don't we kill the children in the test cleanup()
here as well. This should be easy, just store the number of allready
forked kids in a global variable and reset it to zero right after they
were killed in the main loop.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Han Pingtian]

On Wed, Apr 13, 2016 at 02:55:35PM +0200, Cyril Hrubis wrote:
> Hi!
> > Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
> > ---
> >  testcases/kernel/syscalls/waitpid/waitpid03.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/testcases/kernel/syscalls/waitpid/waitpid03.c b/testcases/kernel/syscalls/waitpid/waitpid03.c
> > index 2599a92..7e9d45c 100644
> > --- a/testcases/kernel/syscalls/waitpid/waitpid03.c
> > +++ b/testcases/kernel/syscalls/waitpid/waitpid03.c
> > @@ -106,7 +106,16 @@ int main(int argc, char **argv)
> >  				if (DEBUG)
> >  					tst_resm(TINFO, "child # %d", ikids);
> >  			} else if (pid[ikids] == -1) {
> > -				tst_resm(TFAIL, "cannot open fork #%d", ikids);
> > +				int i;
> > +
> > +				tst_resm(TBROK|TERRNO, "cannot open fork #%d",
> > +					 ikids);
> > +
> > +				for (i = 1; i < ikids; i++)
> > +					kill(pid[i], 9);
> > +
> > +				cleanup();
> > +				tst_exit();
> 
> This one is ugly. Why don't we kill the children in the test cleanup()
> here as well. This should be easy, just store the number of allready
> forked kids in a global variable and reset it to zero right after they
> were killed in the main loop.
> 
Thanks. Please reveiw this new patch.


From 8b7c289ac51ec2e169bbe0147d4c1c3d1f08aa0b Mon Sep 17 00:00:00 2001
From: Han Pingtian <hanpt@linux.vnet.ibm.com>
Date: Tue, 12 Apr 2016 11:12:28 +0800
Subject: [PATCH] waitpid/waitpid03: break test if fork failed

If continue after the fork failure, a lot of processes may be killed by
the kill() on line 125.

Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
---
 testcases/kernel/syscalls/waitpid/waitpid03.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid03.c b/testcases/kernel/syscalls/waitpid/waitpid03.c
index 2599a92..4b3f7a7 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid03.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid03.c
@@ -64,6 +64,8 @@ int TST_TOTAL = 1;
 
 #define	MAXUPRC	25
 
+static int ikids;
+static int pid[MAXUPRC];
 static int condition_number;
 
 #ifdef UCLINUX
@@ -75,7 +77,7 @@ int main(int argc, char **argv)
 {
 	int lc;
 
-	int status, pid[25], ret;
+	int status, ret;
 
 	tst_parse_opts(argc, argv, NULL, NULL);
 #ifdef UCLINUX
@@ -87,7 +89,6 @@ int main(int argc, char **argv)
 	/* check for looping state if -i option is given */
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 		/* reset tst_count in case we are looping */
-		int ikids = 0;
 		tst_count = 0;
 
 		/*
@@ -106,7 +107,9 @@ int main(int argc, char **argv)
 				if (DEBUG)
 					tst_resm(TINFO, "child # %d", ikids);
 			} else if (pid[ikids] == -1) {
-				tst_resm(TFAIL, "cannot open fork #%d", ikids);
+				tst_brkm(TBROK|TERRNO, cleanup, "cannot open "
+					"fork #%d", ikids);
+
 			} else {
 #ifdef UCLINUX
 				if (self_exec(argv[0], "d", ikids) < 0) {
@@ -125,6 +128,7 @@ int main(int argc, char **argv)
 			kill(pid[ikids], 15);
 		}
 
+		ikids = 0;
 		condition_number = 1;
 
 		/* Wait on one specific child */
@@ -185,4 +189,6 @@ static void setup(void)
 
 static void cleanup(void)
 {
+	while (ikids-- > 1)
+		kill(pid[ikids], 9);
 }
-- 
1.9.3

[LTP] [PATCH] waitpid/waitpid03: break test if fork failed

[Cyril Hrubis]

Hi!
> +	while (ikids-- > 1)
> +		kill(pid[ikids], 9);

I've changed the number to a signal constat i.e. 9 -> SIGKILL and
pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz