* [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD @ 2014-11-17 16:04 xuw 2014-11-20 10:57 ` Jan Stancek 0 siblings, 1 reply; 4+ messages in thread From: xuw @ 2014-11-17 16:04 UTC (permalink / raw) To: ltp-list From: George Wang <xuw@redhat.com> system(3) will raise SIGCHLD signal to parent process, and most test cases will call tst_sig to poison all signals including the SIGCHLD. So add system(3) wrapper function to temporarily disable the poisoned handler for SIGCHLD, which will make the test cases happy. Replace directly calling system(3) with tst_systm on behalf of ignorcing SIGCHLG signal posioned handler. Signed-off-by: George Wang <xuw@redhat.com> --- include/test.h | 5 +++++ lib/tst_mkfs.c | 2 +- lib/tst_run_cmd.c | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/include/test.h b/include/test.h index 775ba39..326b686 100644 --- a/include/test.h +++ b/include/test.h @@ -300,6 +300,11 @@ void tst_run_cmd(void (cleanup_fn)(void), const char *stdout_path, const char *stderr_path); +/* Wrapper function for system(3), ignorcing SIGCLD signal. + * @command: the command to be run. + */ +int tst_system(const char *command); + /* lib/tst_mkfs.c * * @dev: path to a device diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c index 5eb3392..173347a 100644 --- a/lib/tst_mkfs.c +++ b/lib/tst_mkfs.c @@ -45,7 +45,7 @@ void tst_mkfs(void (cleanup_fn)(void), const char *dev, /* * The -f option was added to btrfs-progs v3.12 */ - if (system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { + if (tst_system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { tst_resm(TINFO, "Appending '-f' flag to mkfs.%s", fs_type); argv[pos++] = "-f"; diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c index 4eb32e6..5a02db0 100644 --- a/lib/tst_run_cmd.c +++ b/lib/tst_run_cmd.c @@ -125,3 +125,20 @@ void tst_run_cmd(void (cleanup_fn)(void), "close() on %s failed at %s:%d", stderr_path, __FILE__, __LINE__); } + +int tst_system(const char *command) +{ + int ret = 0; + + /* + *Temporarily disable SIGCHLD of user defined handler, so the + *system(3) function will not cause unexpected SIGCHLD signal + *callback function for test cases. + */ + void *old_handler = signal(SIGCHLD, SIG_DFL); + + ret = system(command); + + signal(SIGCHLD, old_handler); + return ret; +} -- 1.8.4.2 ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD 2014-11-17 16:04 [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD xuw @ 2014-11-20 10:57 ` Jan Stancek 2014-12-16 10:57 ` Stanislav Kholmanskikh 0 siblings, 1 reply; 4+ messages in thread From: Jan Stancek @ 2014-11-20 10:57 UTC (permalink / raw) To: xuw; +Cc: ltp-list ----- Original Message ----- > From: xuw@redhat.com > To: ltp-list@lists.sourceforge.net > Sent: Monday, 17 November, 2014 5:04:39 PM > Subject: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD > > From: George Wang <xuw@redhat.com> Hi, Looks good to me, I tested only on single testcase, it work as advertised. Couple nits to comments/commit message: > > system(3) will raise SIGCHLD signal to parent process, and most > test cases will call tst_sig to poison all signals including the > SIGCHLD. So add system(3) wrapper function to temporarily disable > the poisoned handler for SIGCHLD, which will make the test cases > happy. > Replace directly calling system(3) with tst_systm on behalf of ^^ tst_system > ignorcing SIGCHLG signal posioned handler. ^^ ignoring ^^ poisoned I don't get the "on behalf of" part of that sentence. Maybe: "Replace system(3) with tst_system() to ignore SIGCHLG signal handler poisoned earlier." > > Signed-off-by: George Wang <xuw@redhat.com> > --- > include/test.h | 5 +++++ > lib/tst_mkfs.c | 2 +- > lib/tst_run_cmd.c | 17 +++++++++++++++++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/include/test.h b/include/test.h > index 775ba39..326b686 100644 > --- a/include/test.h > +++ b/include/test.h > @@ -300,6 +300,11 @@ void tst_run_cmd(void (cleanup_fn)(void), > const char *stdout_path, > const char *stderr_path); > > +/* Wrapper function for system(3), ignorcing SIGCLD signal. ^^ ignoring > + * @command: the command to be run. > + */ > +int tst_system(const char *command); > + > /* lib/tst_mkfs.c > * > * @dev: path to a device > diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c > index 5eb3392..173347a 100644 > --- a/lib/tst_mkfs.c > +++ b/lib/tst_mkfs.c > @@ -45,7 +45,7 @@ void tst_mkfs(void (cleanup_fn)(void), const char *dev, > /* > * The -f option was added to btrfs-progs v3.12 > */ > - if (system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { > + if (tst_system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { This line is now too long. Regards, Jan > tst_resm(TINFO, "Appending '-f' flag to mkfs.%s", > fs_type); > argv[pos++] = "-f"; > diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c > index 4eb32e6..5a02db0 100644 > --- a/lib/tst_run_cmd.c > +++ b/lib/tst_run_cmd.c > @@ -125,3 +125,20 @@ void tst_run_cmd(void (cleanup_fn)(void), > "close() on %s failed at %s:%d", > stderr_path, __FILE__, __LINE__); > } > + > +int tst_system(const char *command) > +{ > + int ret = 0; > + > + /* > + *Temporarily disable SIGCHLD of user defined handler, so the > + *system(3) function will not cause unexpected SIGCHLD signal > + *callback function for test cases. > + */ > + void *old_handler = signal(SIGCHLD, SIG_DFL); > + > + ret = system(command); > + > + signal(SIGCHLD, old_handler); > + return ret; > +} > -- > 1.8.4.2 > > > ------------------------------------------------------------------------------ > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > with Interactivity, Sharing, Native Excel Exports, App Integration & more > Get technology previously reserved for billion-dollar corporations, FREE > http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk > _______________________________________________ > Ltp-list mailing list > Ltp-list@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/ltp-list > ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD 2014-11-20 10:57 ` Jan Stancek @ 2014-12-16 10:57 ` Stanislav Kholmanskikh 2014-12-16 14:29 ` Xu Wang 0 siblings, 1 reply; 4+ messages in thread From: Stanislav Kholmanskikh @ 2014-12-16 10:57 UTC (permalink / raw) To: xuw; +Cc: ltp-list Hi, George. I've just found that I'm also motivated to have this bug fixed :) Are you planning to send an updated version including Jan's comments? Thanks. PS: I also tested it with access06 test case: without the patch: [root@kholmanskikh access]# LTP_DEV_FS_TYPE=btrfs ./access06 access06 0 TINFO : Found free device '/dev/loop0' access06 1 TBROK : tst_sig.c:233: unexpected signal SIGCHLD/SIGCLD(17) received (pid = 10671). access06 2 TBROK : tst_sig.c:233: Remaining cases broken with: [root@kholmanskikh access]# LTP_DEV_FS_TYPE=btrfs ./access06 access06 0 TINFO : Found free device '/dev/loop0' access06 0 TINFO : Appending '-f' flag to mkfs.btrfs access06 0 TINFO : Formatting /dev/loop0 with btrfs extra opts='' access06 1 TPASS : access failed as expected: TEST_ERRNO=EROFS(30): Read-only file system On 11/20/2014 01:57 PM, Jan Stancek wrote: > > > ----- Original Message ----- >> From: xuw@redhat.com >> To: ltp-list@lists.sourceforge.net >> Sent: Monday, 17 November, 2014 5:04:39 PM >> Subject: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD >> >> From: George Wang <xuw@redhat.com> > > Hi, > > Looks good to me, I tested only on single testcase, it work > as advertised. Couple nits to comments/commit message: > >> >> system(3) will raise SIGCHLD signal to parent process, and most >> test cases will call tst_sig to poison all signals including the >> SIGCHLD. So add system(3) wrapper function to temporarily disable >> the poisoned handler for SIGCHLD, which will make the test cases >> happy. >> Replace directly calling system(3) with tst_systm on behalf of > ^^ tst_system >> ignorcing SIGCHLG signal posioned handler. > ^^ ignoring ^^ poisoned > I don't get the "on behalf of" part of that sentence. Maybe: > "Replace system(3) with tst_system() to ignore SIGCHLG > signal handler poisoned earlier." > >> >> Signed-off-by: George Wang <xuw@redhat.com> >> --- >> include/test.h | 5 +++++ >> lib/tst_mkfs.c | 2 +- >> lib/tst_run_cmd.c | 17 +++++++++++++++++ >> 3 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/include/test.h b/include/test.h >> index 775ba39..326b686 100644 >> --- a/include/test.h >> +++ b/include/test.h >> @@ -300,6 +300,11 @@ void tst_run_cmd(void (cleanup_fn)(void), >> const char *stdout_path, >> const char *stderr_path); >> >> +/* Wrapper function for system(3), ignorcing SIGCLD signal. > ^^ ignoring > >> + * @command: the command to be run. >> + */ >> +int tst_system(const char *command); >> + >> /* lib/tst_mkfs.c >> * >> * @dev: path to a device >> diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c >> index 5eb3392..173347a 100644 >> --- a/lib/tst_mkfs.c >> +++ b/lib/tst_mkfs.c >> @@ -45,7 +45,7 @@ void tst_mkfs(void (cleanup_fn)(void), const char *dev, >> /* >> * The -f option was added to btrfs-progs v3.12 >> */ >> - if (system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { >> + if (tst_system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { > > This line is now too long. > > Regards, > Jan > >> tst_resm(TINFO, "Appending '-f' flag to mkfs.%s", >> fs_type); >> argv[pos++] = "-f"; >> diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c >> index 4eb32e6..5a02db0 100644 >> --- a/lib/tst_run_cmd.c >> +++ b/lib/tst_run_cmd.c >> @@ -125,3 +125,20 @@ void tst_run_cmd(void (cleanup_fn)(void), >> "close() on %s failed at %s:%d", >> stderr_path, __FILE__, __LINE__); >> } >> + >> +int tst_system(const char *command) >> +{ >> + int ret = 0; >> + >> + /* >> + *Temporarily disable SIGCHLD of user defined handler, so the >> + *system(3) function will not cause unexpected SIGCHLD signal >> + *callback function for test cases. >> + */ >> + void *old_handler = signal(SIGCHLD, SIG_DFL); >> + >> + ret = system(command); >> + >> + signal(SIGCHLD, old_handler); >> + return ret; >> +} >> -- >> 1.8.4.2 >> >> >> ------------------------------------------------------------------------------ >> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server >> from Actuate! Instantly Supercharge Your Business Reports and Dashboards >> with Interactivity, Sharing, Native Excel Exports, App Integration & more >> Get technology previously reserved for billion-dollar corporations, FREE >> http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk >> _______________________________________________ >> Ltp-list mailing list >> Ltp-list@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/ltp-list >> > > ------------------------------------------------------------------------------ > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > with Interactivity, Sharing, Native Excel Exports, App Integration & more > Get technology previously reserved for billion-dollar corporations, FREE > http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk > _______________________________________________ > Ltp-list mailing list > Ltp-list@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/ltp-list > ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD 2014-12-16 10:57 ` Stanislav Kholmanskikh @ 2014-12-16 14:29 ` Xu Wang 0 siblings, 0 replies; 4+ messages in thread From: Xu Wang @ 2014-12-16 14:29 UTC (permalink / raw) To: Stanislav Kholmanskikh, Jan Stancek; +Cc: ltp-list Sorry for missing to read the review email from Jan. I've already post PATCHv3 about this issue to ltp-list. Thanks for Jan's review, and I appreciate it very much. Best regards, -- George Wang 王旭 Kernel Quantity Engineer Red Hat Software (Beijing) Co.,Ltd IRC:xuw Tel:+86-010-62608041 Phone:15901231579 9/F, Tower C, Raycom ----- Original Message ----- From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com> To: xuw@redhat.com Cc: "Jan Stancek" <jstancek@redhat.com>, ltp-list@lists.sourceforge.net Sent: Tuesday, December 16, 2014 6:57:43 PM Subject: Re: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD Hi, George. I've just found that I'm also motivated to have this bug fixed :) Are you planning to send an updated version including Jan's comments? Thanks. PS: I also tested it with access06 test case: without the patch: [root@kholmanskikh access]# LTP_DEV_FS_TYPE=btrfs ./access06 access06 0 TINFO : Found free device '/dev/loop0' access06 1 TBROK : tst_sig.c:233: unexpected signal SIGCHLD/SIGCLD(17) received (pid = 10671). access06 2 TBROK : tst_sig.c:233: Remaining cases broken with: [root@kholmanskikh access]# LTP_DEV_FS_TYPE=btrfs ./access06 access06 0 TINFO : Found free device '/dev/loop0' access06 0 TINFO : Appending '-f' flag to mkfs.btrfs access06 0 TINFO : Formatting /dev/loop0 with btrfs extra opts='' access06 1 TPASS : access failed as expected: TEST_ERRNO=EROFS(30): Read-only file system On 11/20/2014 01:57 PM, Jan Stancek wrote: > > > ----- Original Message ----- >> From: xuw@redhat.com >> To: ltp-list@lists.sourceforge.net >> Sent: Monday, 17 November, 2014 5:04:39 PM >> Subject: [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD >> >> From: George Wang <xuw@redhat.com> > > Hi, > > Looks good to me, I tested only on single testcase, it work > as advertised. Couple nits to comments/commit message: > >> >> system(3) will raise SIGCHLD signal to parent process, and most >> test cases will call tst_sig to poison all signals including the >> SIGCHLD. So add system(3) wrapper function to temporarily disable >> the poisoned handler for SIGCHLD, which will make the test cases >> happy. >> Replace directly calling system(3) with tst_systm on behalf of > ^^ tst_system >> ignorcing SIGCHLG signal posioned handler. > ^^ ignoring ^^ poisoned > I don't get the "on behalf of" part of that sentence. Maybe: > "Replace system(3) with tst_system() to ignore SIGCHLG > signal handler poisoned earlier." > >> >> Signed-off-by: George Wang <xuw@redhat.com> >> --- >> include/test.h | 5 +++++ >> lib/tst_mkfs.c | 2 +- >> lib/tst_run_cmd.c | 17 +++++++++++++++++ >> 3 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/include/test.h b/include/test.h >> index 775ba39..326b686 100644 >> --- a/include/test.h >> +++ b/include/test.h >> @@ -300,6 +300,11 @@ void tst_run_cmd(void (cleanup_fn)(void), >> const char *stdout_path, >> const char *stderr_path); >> >> +/* Wrapper function for system(3), ignorcing SIGCLD signal. > ^^ ignoring > >> + * @command: the command to be run. >> + */ >> +int tst_system(const char *command); >> + >> /* lib/tst_mkfs.c >> * >> * @dev: path to a device >> diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c >> index 5eb3392..173347a 100644 >> --- a/lib/tst_mkfs.c >> +++ b/lib/tst_mkfs.c >> @@ -45,7 +45,7 @@ void tst_mkfs(void (cleanup_fn)(void), const char *dev, >> /* >> * The -f option was added to btrfs-progs v3.12 >> */ >> - if (system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { >> + if (tst_system("mkfs.btrfs 2>&1 | grep '\\-f ' >/dev/null") == 0) { > > This line is now too long. > > Regards, > Jan > >> tst_resm(TINFO, "Appending '-f' flag to mkfs.%s", >> fs_type); >> argv[pos++] = "-f"; >> diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c >> index 4eb32e6..5a02db0 100644 >> --- a/lib/tst_run_cmd.c >> +++ b/lib/tst_run_cmd.c >> @@ -125,3 +125,20 @@ void tst_run_cmd(void (cleanup_fn)(void), >> "close() on %s failed at %s:%d", >> stderr_path, __FILE__, __LINE__); >> } >> + >> +int tst_system(const char *command) >> +{ >> + int ret = 0; >> + >> + /* >> + *Temporarily disable SIGCHLD of user defined handler, so the >> + *system(3) function will not cause unexpected SIGCHLD signal >> + *callback function for test cases. >> + */ >> + void *old_handler = signal(SIGCHLD, SIG_DFL); >> + >> + ret = system(command); >> + >> + signal(SIGCHLD, old_handler); >> + return ret; >> +} >> -- >> 1.8.4.2 >> >> >> ------------------------------------------------------------------------------ >> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server >> from Actuate! Instantly Supercharge Your Business Reports and Dashboards >> with Interactivity, Sharing, Native Excel Exports, App Integration & more >> Get technology previously reserved for billion-dollar corporations, FREE >> http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk >> _______________________________________________ >> Ltp-list mailing list >> Ltp-list@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/ltp-list >> > > ------------------------------------------------------------------------------ > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > with Interactivity, Sharing, Native Excel Exports, App Integration & more > Get technology previously reserved for billion-dollar corporations, FREE > http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk > _______________________________________________ > Ltp-list mailing list > Ltp-list@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/ltp-list > ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-16 14:29 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-17 16:04 [LTP] [PATCH V2] library: add tst_system for wrapper system(3) without SIGCHLD xuw 2014-11-20 10:57 ` Jan Stancek 2014-12-16 10:57 ` Stanislav Kholmanskikh 2014-12-16 14:29 ` Xu Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.