[PATCH][cr-test] eclone-2 bug fix

[PATCH][cr-test] eclone-2 bug fix

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 3 May 2010 17:49:32 -0700
Subject: [PATCH 1/1] eclone-2 bug fix

When target-pid is in use, eclone() returns EBUSY, not EAGAIN.  The
test passes now because eclone() correctly fails, but the test case
can do a better job of checking the errno.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 eclone/eclone-2.c |   49 ++++++++++++++++++++++++-------------------------
 1 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/eclone/eclone-2.c b/eclone/eclone-2.c
index b825ac3..5550e43 100644
--- a/eclone/eclone-2.c
+++ b/eclone/eclone-2.c
@@ -64,14 +64,11 @@ static int do_eclone(int (*child_fn)(void *), void *child_arg,
 		fflush(stdout);
 	}
 
-	if (rc < 0 && errno == EAGAIN) {
-		printf("PASS: Unable to create a process with a pid that is "
-			"in use\n");
-		exit(0);
-	} else {
-		printf("ERROR: eclone(): rc %d, errno %d\n", rc, errno);
-		return rc;
-	}
+	if (rc < 0)
+		rc = -errno;
+
+	return rc;
+
 }
 
 int main()
@@ -90,27 +87,29 @@ int main()
 
 	pid = do_eclone(do_child, CHILD_ARG, flags, nr_pids, pids);
 
-	if (verbose) {
-		printf("[%d, %d]: Parent waiting for %d\n", getpid(),
-					gettid(), pid);
+	if (pid == -EBUSY) {
+		printf("PASS: Unable to create a process with a pid that is "
+			"in use\n");
+		return 0;
+	} else if (pid < 0) {
+		printf("ERROR: eclone(): errno %d\n", pid);
+		return 1;
 	}
 
+	printf("[%d, %d]: Parent waiting for %d\n", getpid(), gettid(), pid);
+
 	rc = waitpid(pid, &status, __WALL);
-	if (rc < 0) {
-		printf("ERROR: ");
-		verbose = 1;
-	}
 
-	if (verbose) {
-		printf("\twaitpid(): child %d, rc %d, error %d, status 0x%x\n",
-				getpid(), rc, errno, status);
-		if (rc >=0) {
-			if (WIFEXITED(status)) {
-				printf("\t EXITED, %d\n", WEXITSTATUS(status));
-			} else if (WIFSIGNALED(status)) {
-				printf("\t SIGNALED, %d\n", WTERMSIG(status));
-			}
+	printf("\twaitpid(): child %d, rc %d, error %d, status 0x%x\n",
+			getpid(), rc, errno, status);
+
+	if (rc >=0) {
+		if (WIFEXITED(status)) {
+			printf("\t EXITED, %d\n", WEXITSTATUS(status));
+		} else if (WIFSIGNALED(status)) {
+			printf("\t SIGNALED, %d\n", WTERMSIG(status));
 		}
 	}
-	return 0;
+
+	return 1;
 }
-- 
1.6.6.1

Re: [PATCH][cr-test] eclone-2 bug fix

[Serge E. Hallyn]

Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
>  	pid = do_eclone(do_child, CHILD_ARG, flags, nr_pids, pids);
> 
> -	if (verbose) {
> -		printf("[%d, %d]: Parent waiting for %d\n", getpid(),
> -					gettid(), pid);
> +	if (pid == -EBUSY) {
> +		printf("PASS: Unable to create a process with a pid that is "
> +			"in use\n");
> +		return 0;
> +	} else if (pid < 0) {
> +		printf("ERROR: eclone(): errno %d\n", pid);
> +		return 1;

Note that if pid > 0 you don't print out an error msg.  I pushed a
trivial patch on top of this one to do so.  Pls let me know if I
misunderstood and that wasn't right.

thanks,
-serge

Re: [PATCH][cr-test] eclone-2 bug fix

[Sukadev Bhattiprolu]

Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| Note that if pid > 0 you don't print out an error msg.  I pushed a
| trivial patch on top of this one to do so.  Pls let me know if I
| misunderstood and that wasn't right.

Well, if the child was created (i.e pid > 0) do_child() prints a "FAIL"
message and the test exits with 1. But the message does not hurt, although
for consistency we should mark it "FAIL" rather than "ERROR".

Thanks,

Sukadev

Re: [PATCH][cr-test] eclone-2 bug fix

[Serge E. Hallyn]

Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
> Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
> | Note that if pid > 0 you don't print out an error msg.  I pushed a
> | trivial patch on top of this one to do so.  Pls let me know if I
> | misunderstood and that wasn't right.
> 
> Well, if the child was created (i.e pid > 0) do_child() prints a "FAIL"
> message and the test exits with 1. But the message does not hurt, although

Thinking in terms of future ltp integration, I'm not sure whether the
child doing a TFAIL will result in parent test reporting failure on
exit or not, so I prefer the parent report failure when possible.

> for consistency we should mark it "FAIL" rather than "ERROR".

Uh, I guess I was following you example :)

I've changed both and pushed.

thanks,
-serge

Re: [PATCH][cr-test] eclone-2 bug fix

[Sukadev Bhattiprolu]

Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
| > Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| > | Note that if pid > 0 you don't print out an error msg.  I pushed a
| > | trivial patch on top of this one to do so.  Pls let me know if I
| > | misunderstood and that wasn't right.
| > 
| > Well, if the child was created (i.e pid > 0) do_child() prints a "FAIL"
| > message and the test exits with 1. But the message does not hurt, although
| 
| Thinking in terms of future ltp integration, I'm not sure whether the
| child doing a TFAIL will result in parent test reporting failure on
| exit or not, so I prefer the parent report failure when possible.

Ok. I guess we can remove the printf() from do_child().

| 
| > for consistency we should mark it "FAIL" rather than "ERROR".
| 
| Uh, I guess I was following you example :)

In cr-tests, I have been using ERROR for BROK and FAIL for actual failure.
If we want to use BROK, there is one other "ERROR" that could be changed
too. Here is a patch.

Thanks,

---
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 4 May 2010 18:55:32 -0700
Subject: [PATCH 1/1] Use BROK for error messages

Use BROK instead of ERROR for error messages and remove an extra
printf() in the child.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 eclone/eclone-2.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/eclone/eclone-2.c b/eclone/eclone-2.c
index eddab21..ae2e15e 100644
--- a/eclone/eclone-2.c
+++ b/eclone/eclone-2.c
@@ -22,9 +22,6 @@ pid_t pids[2];
 
 int do_child(void *arg)
 {
-	printf("FAIL: Child created with [%d, %d], but we expected child "
-			"creation to fail since pid is in use\n", gettid(),
- 			getpid());
 	exit(2);
 }
 
@@ -37,7 +34,7 @@ static int do_eclone(int (*child_fn)(void *), void *child_arg,
 
 	stack = genstack_alloc(STACKSIZE);
 	if (!stack) {
-		printf("ERROR: genstack_alloc() returns NULL for size %d\n",
+		printf("BROK: genstack_alloc() returns NULL for size %d\n",
 				STACKSIZE);
 		exit(1);
 	}
-- 
1.6.6.1

Re: [PATCH][cr-test] eclone-2 bug fix

[Serge E. Hallyn]

thanks, applied.

Note that I'm also unhappy with tests which test for too
specific an error value (see reported sysctl03 breakage
in ltp just today).  It's fragile.  But I don't feel like
tweaking this any more right now, and it's not like I'm
sure that letting any <0 errno mean PASS is the way to
go either.  It's just that it's a maintenance pain...
and I guess ltp should come to some conclusion about how
to handle it.

thanks,
-serge