----- Original Message -----
> From: [email protected]
> To: [email protected]
> 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 <[email protected]>

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 <[email protected]>
> ---
>  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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to