Skip to contentSkip to conversation

ppoll01: add signal() syscall to register the signal handler

PATCH Superseded Chandru 4 participants 7 messages Sep 24, 2009 – Oct 1, 2009

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Chandru

author
#0 lore raw

PATCH 1/1 ppoll01: add signal() syscall to register the signal handler

The ppoll01 testcase has a signal handler in it but this signal handler is not registered with the kernel through the signal() syscal. The testcase fails when it sends a SIGINT to itself as part of one of the case of the test run. The following patch
1. Adds a signal() syscall to register the signal handler
2. Corrects the expected 'revents' for the case where ppoll() is called on a file which is only opened in read/write mode, i.e case00.
3. Takes away printing the type of error ( i.e strerror(errno) ) since it only prints the last error seen and does not report the failures seen in the cases before the last failed one. The reporting of NG or OK shoudl take care of letting the user know the cases that have passed and cases that have failed.

---

ppoll01.c renamed from ppoll01.c.orig +6 -3
@@ -106,6 +106,8 @@ extern void cleanup() {
         tst_exit();
 }
 
+void sighandler(int sig);       /* signals handler function for the test */
+
 /* Local  Functions */
 /******************************************************************************/
 /*                                                                            */
@@ -125,7 +127,8 @@ extern void cleanup() {
 /*                                                                            */
 /******************************************************************************/
 void setup() {
-        /* Capture signals if any */
+        /* Capture signals*/
+	signal(SIGINT, &sighandler);
         /* Create temporary directories */
         TEST_PAUSE;
         tst_tmpdir();
@@ -191,7 +194,7 @@ struct test_case {
 static struct test_case tcase[] = {
 	{ // case00
                 .ttype          = NORMAL,
-                .expect_revents = POLLOUT,
+                .expect_revents = POLLOUT | POLLIN,
                 .ret            = 0,
                 .err            = 0,
         },
@@ -463,7 +466,7 @@ int main(int ac, char **av) {
 		                break;
 
 	        default:
-                 	   	tst_resm(TFAIL, "%s failed - errno = %d : %s", TCID, TEST_ERRNO, strerror(TEST_ERRNO));
+                 	   	tst_resm(TFAIL, "%s failed", TCID);
         		        RPRINTF("NG");
 				cleanup();
 				tst_exit();

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

#1 lore raw

I applied this patch, and it fixed ppoll01 on an Ubuntu Hardy box which had been failing. Kernel is 2.6.24.

Kelly

Quoted 78 lines · Chandru

The ppoll01 testcase has a signal handler in it but this signal handler is not registered with the kernel through the signal() syscal. The testcase fails when it sends a SIGINT to itself as part of one of the case of the test run.
The following patch
1. Adds a signal() syscall to register the signal handler
2. Corrects the expected 'revents' for the case where ppoll() is called on a file which is only opened in read/write mode, i.e case00.
3. Takes away printing the type of error ( i.e strerror(errno) ) since it only prints the last error seen and does not report the failures seen in the cases before the last failed one. The reporting of NG or OK shoudl take care of
letting the user know the cases that have passed and cases that have
failed.

---

ppoll01.c 2009-09-24 01:24:38.706487228 +0530 renamed from ppoll01.c.orig 2009-09-22 19:22:39.371235396 +0530 +2 -0
@@ -106,6 +106,8 @@ extern void cleanup() {
        tst_exit();
 }
 
+void sighandler(int sig);       /* signals handler function for the test
 */
+
 /* Local  Functions */
 

/******************************************************************************/ /*
*/

ppoll01.c 2009-09-24 01:24:38.706487228 +0530 renamed from ppoll01.c.orig 2009-09-22 19:22:39.371235396 +0530 +2 -1
@@ -125,7 +127,8 @@ extern void cleanup() {
 /*
   */
 
 /******************************************************************************/
 void setup() {
-        /* Capture signals if any */
+        /* Capture signals*/
+       signal(SIGINT, &sighandler);
        /* Create temporary directories */
        TEST_PAUSE;
        tst_tmpdir();
ppoll01.c 2009-09-24 01:24:38.706487228 +0530 renamed from ppoll01.c.orig 2009-09-22 19:22:39.371235396 +0530 +2 -2
@@ -191,7 +194,7 @@ struct test_case {
 static struct test_case tcase[] = {
       { // case00
                .ttype          = NORMAL,
-                .expect_revents = POLLOUT,
+                .expect_revents = POLLOUT | POLLIN,
                .ret            = 0,
                .err            = 0,
        },
@@ -463,7 +466,7 @@ int main(int ac, char **av) {
                               break;
 
               default:
-                               tst_resm(TFAIL, "%s failed - errno = %d :
 %s", TCID, TEST_ERRNO, strerror(TEST_ERRNO));
+                               tst_resm(TFAIL, "%s failed", TCID);
                               RPRINTF("NG");
                               cleanup();
                               tst_exit();

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Signature · 2 lines
K.D. Lucas
[email protected]

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf

_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

#2 lore raw
Quoted 23 lines · K.D. Lucas

I applied this patch, and it fixed ppoll01 on an Ubuntu Hardy box which had been failing. Kernel is 2.6.24.

Kelly

Quoted 17 lines · Chandru

The ppoll01 testcase has a signal handler in it but this signal handler is not registered with the kernel through the signal() syscal. The testcase fails when it sends a SIGINT to itself as part of one of the case of the test run.
The following patch
1. Adds a signal() syscall to register the signal handler
2. Corrects the expected 'revents' for the case where ppoll() is called on a file which is only opened in read/write mode, i.e case00.
3. Takes away printing the type of error ( i.e strerror(errno) ) since it only prints the last error seen and does not report the failures seen in the cases before the last failed one. The reporting of NG or OK shoudl take care of
   letting the user know the cases that have passed and cases that have failed.

...

Quoted 7 lines
Quoted 7 lines

               default:
-                               tst_resm(TFAIL, "%s failed - errno = %d :
%s", TCID, TEST_ERRNO, strerror(TEST_ERRNO));
+                               tst_resm(TFAIL, "%s failed", TCID); RPRINTF("NG");
                               cleanup();
                               tst_exit();

Please don't change that line.
Thanks,
-Garrett

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

#3 lore raw
In reply to Chandru
Quoted 10 lines · On Thursday 24 September 2009 02:59:32 Chandru wrote:

3. Takes away
printing the type of error ( i.e strerror(errno) ) since it only prints the last error seen and does not report the failures seen in the cases before the last failed one. The reporting of NG or OK shoudl take care of letting the user know the cases that have passed and cases that have failed.

- tst_resm(TFAIL, "%s failed - errno = %d : %s", TCID,
TEST_ERRNO, strerror(TEST_ERRNO));
+ tst_resm(TFAIL, "%s failed", TCID);

why dont you *fix* the issues you see with this output instead of making the output even more useless ? also, this should be converted to TFAIL|TTERRNO. -mike

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf

_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

author
#4 lore raw
Quoted 8 lines · On Saturday 26 September 2009 16:55:28 Mike Frysinger wrote:
Quoted 7 lines · On Thursday 24 September 2009 02:59:32 Chandru wrote:

3. Takes away
printing the type of error ( i.e strerror(errno) ) since it only prints the last error seen and does not report the failures seen in the cases before the last failed one. The reporting of NG or OK shoudl take care of letting the user know the cases that have passed and cases that have failed.

Quoted 8 lines
Quoted 3 lines

- tst_resm(TFAIL, "%s failed - errno = %d : %s", TCID,
TEST_ERRNO, strerror(TEST_ERRNO));
+ tst_resm(TFAIL, "%s failed", TCID);

why dont you *fix* the issues you see with this output instead of making the output even more useless ? also, this should be converted to TFAIL|TTERRNO. -mike

em, the change was not useless, the existing reporting of failure was. The output on failure of only the first case prints as
"ppoll01 1 TFAIL : ppoll01 failed - errno = 0 : Success"

The words 'failed' and 'Success' on the same line could confuse us. The testcase has 8 scenarios of which 5 are made to fail by passing invalid arguments. Although we could have a fix as the following

=====================================================================

ppoll01.c renamed from ppoll01.c.orig +12 -1
@@ -242,6 +242,8 @@ static struct test_case tcase[] = {
 #endif
 };
 
+int tstatus, terrno[16];
+
 #define NUM_TEST_FDS    1
 
 /*
@@ -265,6 +267,7 @@ static int do_test(struct test_case *tc)
         sigset_t *p_sigmask, sigmask;
         size_t sigsetsize = 0;
         pid_t pid = 0;
+	static int tc_no;
 
 	TEST(fd = setup_file(progdir, "test.file", fpath));
         if (fd < 0)
@@ -331,6 +334,11 @@ static int do_test(struct test_case *tc)
         errno = 0;
         TEST(sys_ret = syscall(__NR_ppoll, p_fds, nfds, p_ts, p_sigmask, sigsetsize));
         sys_errno = errno;
+	if (((tc->ret == 0) && (sys_ret == -1)) || ((tc->ret == -1) && (sys_ret == 0))) {
+		tstatus |= (1 << tc_no);
+		terrno[tc_no] = errno;
+	}
+	tc_no++;
         if (sys_ret <= 0 || tc->ret < 0)
                 goto TEST_END;
 
@@ -463,7 +471,10 @@ int main(int ac, char **av) {
 		                break;
 
 	        default:
-                 	   	tst_resm(TFAIL, "%s failed - errno = %d : %s", TCID, TEST_ERRNO, strerror(TEST_ERRNO));
+        			tst_resm(TPASS, "ppoll result = %d  ",result);
+                                for (i = 0; i < (int)(sizeof(tcase) / sizeof(tcase[0])); i++)
+                                        if (tstatus & (1 << i))
+                                                tst_resm(TFAIL|TTERRNO, "%s failed - errno = %d : %s", TCID, terrno[i], strerror(terrno[i]));
         		        RPRINTF("NG");
 				cleanup();
 				tst_exit();

======================================================================

I will retain the current existing tst_resm lines and add TFAIL|TTERRNO to it as per your suggestion. The reason being that there is not much gain in making the above changes to the test case.

Thanks,
Chandru

------------------------------------------------------------------------------
Come build with us! The BlackBerry&reg; Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9&#45;12, 2009. Register now&#33;
http://p.sf.net/sfu/devconf
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

author
#5 lore raw
Quoted 2 lines · Chandru

I will retain the current existing tst_resm lines and add TFAIL|TTERRNO to it as per your suggestion. The reason being that there is not much gain in making the above changes to the test case.

Sorry, did not change that line at all.

Thanks
Chandru

------------------------------------------------------------------------------
Come build with us! The BlackBerry&reg; Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9&#45;12, 2009. Register now&#33;
http://p.sf.net/sfu/devconf
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

#6 lore raw
In reply to Chandru
Quoted 13 lines · On Thursday 01 October 2009 03:27:46 Chandru wrote:
Quoted 12 lines · On Saturday 26 September 2009 16:55:28 Mike Frysinger wrote:
Quoted 10 lines · On Thursday 24 September 2009 02:59:32 Chandru wrote:

3. Takes away
printing the type of error ( i.e strerror(errno) ) since it only
prints the last error seen and does not report the failures seen in the cases before the last failed one. The reporting of NG or OK shoudl take care of letting the user know the cases that have passed and cases that have failed.

- tst_resm(TFAIL, "%s failed - errno = %d : %s",
TCID, TEST_ERRNO, strerror(TEST_ERRNO));
+ tst_resm(TFAIL, "%s failed", TCID);

Quoted 7 lines
Quoted 3 lines

why dont you *fix* the issues you see with this output instead of making the output even more useless ? also, this should be converted to TFAIL|TTERRNO.

em, the change was not useless, the existing reporting of failure was. The
output on failure of only the first case prints as
"ppoll01 1 TFAIL : ppoll01 failed - errno = 0 : Success"

you've shown problems in one case, not every single one. at any rate, you've taken a stab at fixing the problem properly, so arguing semantics here is a waste of time.

ppoll01.c static struct test_case tcase[] = { · line 245

Quoted 7 lines
ppoll01.c renamed from ppoll01.c.orig +1 -0
@@ -242,6 +242,8 @@ static struct test_case tcase[] = {
 #endif
 };
 
+int tstatus, terrno[16];

declare terrno static and then everything is initialized to 0. then you dont need to track things in tstatus as non-zero should indicate an error.

hard coding 16 seems like a bad idea ... isnt there already a define/variable that holds the test count that you can use ?

ppoll01.c static int do_test(struct test_case *tc) · line 342

Quoted 10 lines
ppoll01.c renamed from ppoll01.c.orig +3 -0
@@ -331,6 +334,11 @@ static int do_test(struct test_case *tc)
         errno = 0;
         TEST(sys_ret = syscall(__NR_ppoll, p_fds, nfds, p_ts, p_sigmask,
 sigsetsize));
 sys_errno = errno;
+	if (((tc->ret == 0) && (sys_ret == -1)) || ((tc->ret == -1) && (sys_ret
 == 0))) {
 +		tstatus |= (1 << tc_no);
+		terrno[tc_no] = errno;
+	}

looks like this test was written without really understanding the LTP framework. setting errno to 0 and the use of sys_errno is duplicating stuff LTP is already doing. the code should be using TEST_ERRNO to get at the errno from the TEST() call. this applies both to existing code and your new code.

Quoted 3 lines

- tst_resm(TFAIL, "%s failed - errno = %d : %s", TCID, TEST_ERRNO,

			strerror(TEST_ERRNO));

+ tst_resm(TPASS, "ppoll result = %d ",result);

how can it TPASS and then turn around and TFAIL ? this new tst_resm() should just be dropped i think.

Quoted 4 lines

+ for (i = 0; i < (int)(sizeof(tcase) / sizeof(tcase[0])); i++)
+ if (tstatus & (1 << i))
+ tst_resm(TFAIL|TTERRNO, "%s failed - errno = %d : %s", TCID,

		 terrno[i], strerror(terrno[i]));

this isnt how TTERRNO works. the code should be:

if (terrno[i]) {
	TEST_ERRNO = terrno[i];
	tst_resm(TFAIL|TTERNO, "test %i failed\n", i);
}

-mike

------------------------------------------------------------------------------
Come build with us! The BlackBerry&reg; Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9&#45;12, 2009. Register now&#33;
http://p.sf.net/sfu/devconf

_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list