[LTP] [PATCH 1/2] tst_run_cmd: added support for stdout and stderr redirection

[LTP] [PATCH 1/2] tst_run_cmd: added support for stdout and stderr redirection

[Stanislav Kholmanskikh]

From: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>

Signed-off-by: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/test.h    |    9 ++++++++-
 lib/tst_run_cmd.c |   20 ++++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/test.h b/include/test.h
index a76fafc..f294b16 100644
--- a/include/test.h
+++ b/include/test.h
@@ -219,8 +219,15 @@ long tst_ncpus_max(void);
  * @argv: a list of two (at least program name + NULL) or more pointers that
  * represent the argument list to the new program. The array of pointers
  * must be terminated by a NULL pointer.
+ * @stdout_path: path where to redirect stdout. Set NULL if redirection is
+ * not needed.
+ * @stderr_path: path where to redirect stderr. Set NULL if redirection is
+ * not needed.
  */
-void tst_run_cmd(void (cleanup_fn)(void), char *const argv[]);
+void tst_run_cmd(void (cleanup_fn)(void),
+		char *const argv[],
+		const char *stdout_path,
+		const char *stderr_path);
 
 #ifdef TST_USE_COMPAT16_SYSCALL
 #define TCID_BIT_SUFFIX "_16"
diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c
index 93fe2d6..c54c650 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -19,12 +19,16 @@
  *
  */
 
+#include <stdio.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
 #include "test.h"
 
-void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
+void tst_run_cmd(void (cleanup_fn)(void),
+		char *const argv[],
+		const char *stdout_path,
+		const char *stderr_path)
 {
 	if (argv == NULL || argv[0] == NULL) {
 		tst_brkm(TBROK, cleanup_fn,
@@ -36,8 +40,20 @@ void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "vfork failed at %s:%d",
 			__FILE__, __LINE__);
 	}
-	if (!pid)
+	if (!pid) {
+		/* redirecting stdout and stderr if needed */
+		if ((stdout_path != NULL) &&
+				(freopen(stdout_path, "a", stdout) == NULL))
+			tst_resm(TWARN | TERRNO, "freopen failed at %s:%d",
+					__FILE__, __LINE__);
+
+		if ((stderr_path != NULL) &&
+				(freopen(stderr_path, "a", stderr) == NULL))
+			tst_resm(TWARN | TERRNO, "freopen failed at %s:%d",
+					__FILE__, __LINE__);
+
 		_exit(execvp(argv[0], argv));
+	}
 
 	int ret = -1;
 	if (waitpid(pid, &ret, 0) != pid) {
-- 
1.7.1


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 2/2] lib/tst_module.c: modification for updated tst_run_cmd specification

[Stanislav Kholmanskikh]

From: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>

Modified lib/tst_module.c:
* for updated tst_run_cmd specification
* to hide gcc warning "initialization discards qualifiers from pointer target type"

Signed-off-by: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 lib/tst_module.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/tst_module.c b/lib/tst_module.c
index 832da27..4871005 100644
--- a/lib/tst_module.c
+++ b/lib/tst_module.c
@@ -94,12 +94,12 @@ void tst_module_load(void (cleanup_fn)(void),
 	for (i = offset; i < size; ++i)
 		mod_argv[i] = argv[i - offset];
 
-	tst_run_cmd(cleanup_fn, mod_argv);
+	tst_run_cmd(cleanup_fn, mod_argv, NULL, NULL);
 	free(mod_path);
 }
 
 void tst_module_unload(void (cleanup_fn)(void), const char *mod_name)
 {
-	char *const argv[] = { "rmmod", mod_name, NULL };
-	tst_run_cmd(cleanup_fn, argv);
+	char *const argv[] = { "rmmod", (char *) mod_name, NULL };
+	tst_run_cmd(cleanup_fn, argv, NULL, NULL);
 }
-- 
1.7.1


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/2] tst_run_cmd: added support for stdout and stderr redirection

[chrubis]

Hi!
> -void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
> +void tst_run_cmd(void (cleanup_fn)(void),
> +		char *const argv[],
> +		const char *stdout_path,
> +		const char *stderr_path)
>  {
>  	if (argv == NULL || argv[0] == NULL) {
>  		tst_brkm(TBROK, cleanup_fn,
> @@ -36,8 +40,20 @@ void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
>  		tst_brkm(TBROK | TERRNO, cleanup_fn, "vfork failed at %s:%d",
>  			__FILE__, __LINE__);
>  	}
> -	if (!pid)
> +	if (!pid) {
> +		/* redirecting stdout and stderr if needed */
> +		if ((stdout_path != NULL) &&
> +				(freopen(stdout_path, "a", stdout) == NULL))
> +			tst_resm(TWARN | TERRNO, "freopen failed at %s:%d",
> +					__FILE__, __LINE__);
> +
> +		if ((stderr_path != NULL) &&
> +				(freopen(stderr_path, "a", stderr) == NULL))
> +			tst_resm(TWARN | TERRNO, "freopen failed at %s:%d",
> +					__FILE__, __LINE__);

There is a slight problem bin this part of the code. As you did vfork()
the virtual memory is not duplicated in the child and only safe
operations you can do is, exec(), _exit() and operations that affects
file description table (which is one of the things that is duplicated,
or guaranteed to be copy on write after vfork()).

So we can't just freopen the streams as freopen() is not guaranteed not
to modify memory (The FILE* object for stdout may end up in inconsistent
state in parent because it was closed by freopen() in child), but we can
close the stdout and stderr in child and redirect them via dup2().

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent 
caught up. So what steps can you take to put your SQL databases under 
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 1/2 V2] tst_run_cmd: added support for stdout and stderr redirection

[Stanislav Kholmanskikh]

From: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>

Signed-off-by: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/test.h    |    9 ++++++++-
 lib/tst_run_cmd.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/test.h b/include/test.h
index a76fafc..f294b16 100644
--- a/include/test.h
+++ b/include/test.h
@@ -219,8 +219,15 @@ long tst_ncpus_max(void);
  * @argv: a list of two (at least program name + NULL) or more pointers that
  * represent the argument list to the new program. The array of pointers
  * must be terminated by a NULL pointer.
+ * @stdout_path: path where to redirect stdout. Set NULL if redirection is
+ * not needed.
+ * @stderr_path: path where to redirect stderr. Set NULL if redirection is
+ * not needed.
  */
-void tst_run_cmd(void (cleanup_fn)(void), char *const argv[]);
+void tst_run_cmd(void (cleanup_fn)(void),
+		char *const argv[],
+		const char *stdout_path,
+		const char *stderr_path);
 
 #ifdef TST_USE_COMPAT16_SYSCALL
 #define TCID_BIT_SUFFIX "_16"
diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c
index 93fe2d6..044adc7 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -19,25 +19,71 @@
  *
  */
 
+#include <errno.h>
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <sys/wait.h>
+#include <fcntl.h>
 #include <unistd.h>
 #include "test.h"
 
-void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
+#define OPEN_MODE	(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
+#define OPEN_FLAGS	(O_WRONLY | O_APPEND | O_CREAT)
+
+void tst_run_cmd(void (cleanup_fn)(void),
+		char *const argv[],
+		const char *stdout_path,
+		const char *stderr_path)
 {
 	if (argv == NULL || argv[0] == NULL) {
 		tst_brkm(TBROK, cleanup_fn,
 			"argument list is empty at %s:%d", __FILE__, __LINE__);
 	}
 
+	/* descriptors for child (if needed) */
+	int child_stdout;
+	int child_stderr;
+
 	pid_t pid = vfork();
 	if (pid == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "vfork failed at %s:%d",
 			__FILE__, __LINE__);
 	}
-	if (!pid)
+	if (!pid) {
+		/* redirecting stdout and stderr if needed */
+		if (stdout_path != NULL) {
+			child_stdout = open(stdout_path,
+					OPEN_FLAGS, OPEN_MODE);
+
+			if (child_stdout == -1) {
+				tst_resm(TWARN | TERRNO,
+					"open() on %s failed at %s:%d: %s",
+					stdout_path, __FILE__, __LINE__,
+					strerror(errno));
+			} else {
+				close(STDOUT_FILENO);
+				dup2(child_stdout, STDOUT_FILENO);
+			}
+
+		}
+
+		if (stderr_path != NULL) {
+			child_stderr = open(stderr_path,
+					OPEN_FLAGS, OPEN_MODE);
+
+			if (child_stderr == -1) {
+				tst_resm(TWARN | TERRNO,
+					"open() on %s failed at %s:%d: %s",
+					stderr_path, __FILE__, __LINE__,
+					strerror(errno));
+			} else {
+				close(STDERR_FILENO);
+				dup2(child_stderr, STDERR_FILENO);
+			}
+		}
+
 		_exit(execvp(argv[0], argv));
+	}
 
 	int ret = -1;
 	if (waitpid(pid, &ret, 0) != pid) {
-- 
1.7.1


------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent 
caught up. So what steps can you take to put your SQL databases under 
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/2 V2] tst_run_cmd: added support for stdout and stderr redirection

[chrubis]

Hi!
> -	if (!pid)
> +	if (!pid) {
> +		/* redirecting stdout and stderr if needed */
> +		if (stdout_path != NULL) {
> +			child_stdout = open(stdout_path,
> +					OPEN_FLAGS, OPEN_MODE);
> +
> +			if (child_stdout == -1) {
> +				tst_resm(TWARN | TERRNO,
> +					"open() on %s failed at %s:%d: %s",
> +					stdout_path, __FILE__, __LINE__,
> +					strerror(errno));

You should not call the tst_resm() here for two reasons.

1. It is not designed to be called from a child of the main test process

2. It still may corrupt the parent memory although in this case it's
   not that likely (as it operates on FILE* used for the test output,
   note that ANY operation on FILE* may get it into inconsistent state
   in the parent later)

What about passing file descriptors to function (-1 means no
redirection) and create a second function on the top of it that
opens the files, does the checks and the calls the tst_run_cmd?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent 
caught up. So what steps can you take to put your SQL databases under 
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/2 V2] tst_run_cmd: added support for stdout and stderr redirection

[chrubis]

Hi!
> > What about passing file descriptors to function (-1 means no
> > redirection) and create a second function on the top of it that
> > opens the files, does the checks and the calls the tst_run_cmd?
> 
> So you propose creating two functions: one will accept (const char *) 
> arguments, the other - file descriptors. Yes?

I preffer to look at this as creating one bare function to do the work
(redirect, vfork + exec) and creating the final function on the top of
it.

Good thing about the desing is that we can keep the bare function very
simple.

> But do we really need this functionality in LTP (ie redirect cmd output 
> either to filename or to file descriptor)?

Well, it may be usefull to be able to redirect stdout of a command into
a pipe for example.

> What about modifying tst_run_cmd interface as I propose:
> tst_run_cmd_redirected(void (cleanup_fn)(void),
>                                          char *const argv[],
>                                          const char *stdout_path,
>                                          const char *stderr_path)
> but putting all the related to file descriptors' opening/closing in 
> parent's body.

That works as well.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent 
caught up. So what steps can you take to put your SQL databases under 
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list