* [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.