[LTP] [PATCH V3] tst_run_cmd: added support for stdout and stderr redirection

[LTP] [PATCH V3] tst_run_cmd: added support for stdout and stderr redirection

[Stanislav Kholmanskikh]

Now there are two functions:
tst_run_cmd() - redirects cmd's stdout/stderr to file descriptors
tst_run_cmd_rdr_file() - a wrapper around tst_run_cmd(), redirects its output
to files.

------------------------------------------------------------------------------
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=48897031&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 V3] tst_run_cmd: added support for stdout and stderr redirection

[Stanislav Kholmanskikh]

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

For tst_run_cmd added support for stdout/stderr redirection to
a file descriptor.

Added function tst_run_cmd_rdr_file which allows to redirect
tst_run_cmd output to a file.

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

diff --git a/include/test.h b/include/test.h
index a76fafc..ab0cdcc 100644
--- a/include/test.h
+++ b/include/test.h
@@ -219,8 +219,26 @@ 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_fd: file descriptor where to redirect stdout. Set -1 if
+ * redirection is not needed.
+ * @stderr_fd: file descriptor where to redirect stderr. Set -1 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[],
+		int stdout_fd,
+		int stderr_fd);
+
+/* Executes tst_run_cmd() and redirects its output to a file
+ * @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_rdr_file(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..1829b4c 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -19,12 +19,21 @@
  *
  */
 
+#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[],
+		int stdout_fd,
+		int stderr_fd)
 {
 	if (argv == NULL || argv[0] == NULL) {
 		tst_brkm(TBROK, cleanup_fn,
@@ -36,8 +45,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_fd != -1) {
+			close(STDOUT_FILENO);
+			dup2(stdout_fd, STDOUT_FILENO);
+		}
+
+		if (stderr_fd != -1) {
+			close(STDERR_FILENO);
+			dup2(stderr_fd, STDERR_FILENO);
+		}
+
 		_exit(execvp(argv[0], argv));
+	}
 
 	int ret = -1;
 	if (waitpid(pid, &ret, 0) != pid) {
@@ -50,3 +71,44 @@ void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
 			argv[0], __FILE__, __LINE__);
 	}
 }
+
+void tst_run_cmd_rdr_file(void (cleanup_fn)(void),
+			char *const argv[],
+			const char *stdout_path,
+			const char *stderr_path)
+{
+	int stdout_fd = -1;
+	int stderr_fd = -1;
+
+	if (stdout_path != NULL) {
+		stdout_fd = open(stdout_path,
+				OPEN_FLAGS, OPEN_MODE);
+
+		if (stdout_fd == -1)
+			tst_resm(TWARN | TERRNO,
+				"open() on %s failed at %s:%d",
+				stdout_path, __FILE__, __LINE__);
+	}
+
+	if (stderr_path != NULL) {
+		stderr_fd = open(stderr_path,
+				OPEN_FLAGS, OPEN_MODE);
+
+		if (stderr_fd == -1)
+			tst_resm(TWARN | TERRNO,
+				"open() on %s failed at %s:%d",
+				stderr_path, __FILE__, __LINE__);
+	}
+
+	tst_run_cmd(cleanup_fn, argv, stdout_fd, stderr_fd);
+
+	if ((stdout_fd != -1) && (close(stdout_fd) == -1))
+		tst_resm(TWARN | TERRNO,
+			"close() on %s failed at %s:%d",
+			stdout_path, __FILE__, __LINE__);
+
+	if ((stderr_fd != -1) && (close(stderr_fd) == -1))
+		tst_resm(TWARN | TERRNO,
+			"close() on %s failed at %s:%d",
+			stderr_path, __FILE__, __LINE__);
+}
-- 
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=48897031&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 V3] tst_run_cmd: added support for stdout and stderr redirection

[chrubis]

Hi!
> For tst_run_cmd added support for stdout/stderr redirection to
> a file descriptor.
> 
> Added function tst_run_cmd_rdr_file which allows to redirect
> tst_run_cmd output to a file.
> 
> Signed-off-by: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  include/test.h    |   20 +++++++++++++++-
>  lib/tst_run_cmd.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/include/test.h b/include/test.h
> index a76fafc..ab0cdcc 100644
> --- a/include/test.h
> +++ b/include/test.h
> @@ -219,8 +219,26 @@ 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_fd: file descriptor where to redirect stdout. Set -1 if
> + * redirection is not needed.
> + * @stderr_fd: file descriptor where to redirect stderr. Set -1 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[],
> +		int stdout_fd,
> +		int stderr_fd);
> +
> +/* Executes tst_run_cmd() and redirects its output to a file
> + * @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_rdr_file(void (cleanup_fn)(void),
> +			char *const argv[],
> +			const char *stdout_path,
> +			const char *stderr_path);

This is relatively minor but the function names are not best choosen as
both of them does the redirections but only one of them has it in the
name.

What about

tst_run_cmd_fds() and tst_run_cmd_files()

or

tst_run_cmd_fds() and just tst_run_cmd()

Naming things right is hard.

> --- a/lib/tst_run_cmd.c
> +++ b/lib/tst_run_cmd.c
> @@ -19,12 +19,21 @@
>   *
>   */
>  
> +#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[],
> +		int stdout_fd,
> +		int stderr_fd)
>  {
>  	if (argv == NULL || argv[0] == NULL) {
>  		tst_brkm(TBROK, cleanup_fn,
> @@ -36,8 +45,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_fd != -1) {
> +			close(STDOUT_FILENO);
> +			dup2(stdout_fd, STDOUT_FILENO);
> +		}
> +
> +		if (stderr_fd != -1) {
> +			close(STDERR_FILENO);
> +			dup2(stderr_fd, STDERR_FILENO);
> +		}
> +
>  		_exit(execvp(argv[0], argv));
> +	}
>  
>  	int ret = -1;
>  	if (waitpid(pid, &ret, 0) != pid) {
> @@ -50,3 +71,44 @@ void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
>  			argv[0], __FILE__, __LINE__);
>  	}
>  }
> +
> +void tst_run_cmd_rdr_file(void (cleanup_fn)(void),
> +			char *const argv[],
> +			const char *stdout_path,
> +			const char *stderr_path)
> +{
> +	int stdout_fd = -1;
> +	int stderr_fd = -1;
> +
> +	if (stdout_path != NULL) {
> +		stdout_fd = open(stdout_path,
> +				OPEN_FLAGS, OPEN_MODE);
> +
> +		if (stdout_fd == -1)
> +			tst_resm(TWARN | TERRNO,
> +				"open() on %s failed at %s:%d",
> +				stdout_path, __FILE__, __LINE__);
> +	}
> +
> +	if (stderr_path != NULL) {
> +		stderr_fd = open(stderr_path,
> +				OPEN_FLAGS, OPEN_MODE);
> +
> +		if (stderr_fd == -1)
> +			tst_resm(TWARN | TERRNO,
> +				"open() on %s failed at %s:%d",
> +				stderr_path, __FILE__, __LINE__);
> +	}
> +
> +	tst_run_cmd(cleanup_fn, argv, stdout_fd, stderr_fd);
> +
> +	if ((stdout_fd != -1) && (close(stdout_fd) == -1))
> +		tst_resm(TWARN | TERRNO,
> +			"close() on %s failed at %s:%d",
> +			stdout_path, __FILE__, __LINE__);
> +
> +	if ((stderr_fd != -1) && (close(stderr_fd) == -1))
> +		tst_resm(TWARN | TERRNO,
> +			"close() on %s failed at %s:%d",
> +			stderr_path, __FILE__, __LINE__);
> +}

The code looks ok.

-- 
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=48897031&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 V4] tst_run_cmd: added support for stdout and stderr redirection

[Stanislav Kholmanskikh]

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

Added function tst_run_cmd_fds which allows redirection of
stdout/stderr to file descriptors.

Now function tst_run_cmd is a wrapper to tst_run_cmd_fds.

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

diff --git a/include/test.h b/include/test.h
index a76fafc..1957148 100644
--- a/include/test.h
+++ b/include/test.h
@@ -219,8 +219,26 @@ 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_fd: file descriptor where to redirect stdout. Set -1 if
+ * redirection is not needed.
+ * @stderr_fd: file descriptor where to redirect stderr. Set -1 if
+ * redirection is not needed.
  */
-void tst_run_cmd(void (cleanup_fn)(void), char *const argv[]);
+void tst_run_cmd_fds(void (cleanup_fn)(void),
+			char *const argv[],
+			int stdout_fd,
+			int stderr_fd);
+
+/* Executes tst_run_cmd_fds() and redirects its output to a file
+ * @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[],
+		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..714b508 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -19,12 +19,21 @@
  *
  */
 
+#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_fds(void (cleanup_fn)(void),
+		char *const argv[],
+		int stdout_fd,
+		int stderr_fd)
 {
 	if (argv == NULL || argv[0] == NULL) {
 		tst_brkm(TBROK, cleanup_fn,
@@ -36,8 +45,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_fd != -1) {
+			close(STDOUT_FILENO);
+			dup2(stdout_fd, STDOUT_FILENO);
+		}
+
+		if (stderr_fd != -1) {
+			close(STDERR_FILENO);
+			dup2(stderr_fd, STDERR_FILENO);
+		}
+
 		_exit(execvp(argv[0], argv));
+	}
 
 	int ret = -1;
 	if (waitpid(pid, &ret, 0) != pid) {
@@ -50,3 +71,44 @@ void tst_run_cmd(void (cleanup_fn)(void), char *const argv[])
 			argv[0], __FILE__, __LINE__);
 	}
 }
+
+void tst_run_cmd(void (cleanup_fn)(void),
+		char *const argv[],
+		const char *stdout_path,
+		const char *stderr_path)
+{
+	int stdout_fd = -1;
+	int stderr_fd = -1;
+
+	if (stdout_path != NULL) {
+		stdout_fd = open(stdout_path,
+				OPEN_FLAGS, OPEN_MODE);
+
+		if (stdout_fd == -1)
+			tst_resm(TWARN | TERRNO,
+				"open() on %s failed at %s:%d",
+				stdout_path, __FILE__, __LINE__);
+	}
+
+	if (stderr_path != NULL) {
+		stderr_fd = open(stderr_path,
+				OPEN_FLAGS, OPEN_MODE);
+
+		if (stderr_fd == -1)
+			tst_resm(TWARN | TERRNO,
+				"open() on %s failed at %s:%d",
+				stderr_path, __FILE__, __LINE__);
+	}
+
+	tst_run_cmd_fds(cleanup_fn, argv, stdout_fd, stderr_fd);
+
+	if ((stdout_fd != -1) && (close(stdout_fd) == -1))
+		tst_resm(TWARN | TERRNO,
+			"close() on %s failed at %s:%d",
+			stdout_path, __FILE__, __LINE__);
+
+	if ((stderr_fd != -1) && (close(stderr_fd) == -1))
+		tst_resm(TWARN | TERRNO,
+			"close() on %s failed at %s:%d",
+			stderr_path, __FILE__, __LINE__);
+}
-- 
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=48897031&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 V4] tst_run_cmd: added support for stdout and stderr redirection

[chrubis]

Hi!
> Added function tst_run_cmd_fds which allows redirection of
> stdout/stderr to file descriptors.
> 
> Now function tst_run_cmd is a wrapper to tst_run_cmd_fds.

Both patches pushed.

-- 
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=48897031&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 V4] lib/tst_module.c: modification for updated tst_run_cmd specification

[Stanislav Kholmanskikh]

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

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

diff --git a/lib/tst_module.c b/lib/tst_module.c
index 832da27..c0f2c8f 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);
+	tst_run_cmd(cleanup_fn, argv, NULL, NULL);
 }
-- 
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=48897031&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 V3] 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..0b94a26 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, -1, -1);
 	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, -1, -1);
 }
-- 
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=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

[chrubis]

Hi!
> * to hide gcc warning "initialization discards qualifiers from pointer target type"

I think that correct solution is to declare the argv as:

const char *const argv[]

So that both the array and the stored strings are constant.

(Feel free to do this in separate patch)

-- 
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=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

[chrubis]

Hi!
> 2) or to change argv in tst_run_cmd function declaration to
> const char *const argv[]

This is what I had in mind. You have to fix the function API first and
then the type of the array that is passed to it.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH] tst_run_cmd: const correctness

[Stanislav Kholmanskikh]

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

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

diff --git a/include/test.h b/include/test.h
index 1957148..75a34ab 100644
--- a/include/test.h
+++ b/include/test.h
@@ -225,7 +225,7 @@ long tst_ncpus_max(void);
  * redirection is not needed.
  */
 void tst_run_cmd_fds(void (cleanup_fn)(void),
-			char *const argv[],
+			const char *const argv[],
 			int stdout_fd,
 			int stderr_fd);
 
@@ -236,7 +236,7 @@ void tst_run_cmd_fds(void (cleanup_fn)(void),
  * not needed.
  */
 void tst_run_cmd(void (cleanup_fn)(void),
-		char *const argv[],
+		const char *const argv[],
 		const char *stdout_path,
 		const char *stderr_path);
 
diff --git a/lib/tst_module.c b/lib/tst_module.c
index c0f2c8f..028af0e 100644
--- a/lib/tst_module.c
+++ b/lib/tst_module.c
@@ -85,7 +85,7 @@ void tst_module_load(void (cleanup_fn)(void),
 	while (argv && argv[size])
 		++size;
 	size += offset;
-	char *mod_argv[size + 1]; /* + NULL in the end */
+	const char *mod_argv[size + 1]; /* + NULL in the end */
 	mod_argv[size] = NULL;
 	mod_argv[0] = "insmod";
 	mod_argv[1] = mod_path;
@@ -100,6 +100,6 @@ void tst_module_load(void (cleanup_fn)(void),
 
 void tst_module_unload(void (cleanup_fn)(void), const char *mod_name)
 {
-	char *const argv[] = { "rmmod", mod_name, NULL };
+	const char *const argv[] = { "rmmod", mod_name, NULL };
 	tst_run_cmd(cleanup_fn, argv, NULL, NULL);
 }
diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c
index 714b508..2c84cfd 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -31,7 +31,7 @@
 #define OPEN_FLAGS	(O_WRONLY | O_APPEND | O_CREAT)
 
 void tst_run_cmd_fds(void (cleanup_fn)(void),
-		char *const argv[],
+		const char *const argv[],
 		int stdout_fd,
 		int stderr_fd)
 {
@@ -73,7 +73,7 @@ void tst_run_cmd_fds(void (cleanup_fn)(void),
 }
 
 void tst_run_cmd(void (cleanup_fn)(void),
-		char *const argv[],
+		const char *const argv[],
 		const char *stdout_path,
 		const char *stderr_path)
 {
-- 
1.7.1


------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] tst_run_cmd: const correctness

[chrubis]

Hi!
>  void tst_run_cmd_fds(void (cleanup_fn)(void),
> -			char *const argv[],
> +			const char *const argv[],
>  			int stdout_fd,
>  			int stderr_fd);
>  
> @@ -236,7 +236,7 @@ void tst_run_cmd_fds(void (cleanup_fn)(void),
>   * not needed.
>   */
>  void tst_run_cmd(void (cleanup_fn)(void),
> -		char *const argv[],
> +		const char *const argv[],
>  		const char *stdout_path,
>  		const char *stderr_path);

Gosh, that fixed the warning in the test but I've overlooked that
execvp() takes char *const argv[] argument so it introduced warning in
the lib. Now I wonder why execvp() has such strange parameter type...

I'm trying to figure out how to fix it 'right' but it gets unnecessary
complicated. I guess that we can fix this patch by casting the array in
the lib before it gets to execvp().

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] tst_run_cmd: const correctness

[chrubis]

Hi!
> > I'm trying to figure out how to fix it 'right' but it gets unnecessary
> > complicated. I guess that we can fix this patch by casting the array in
> > the lib before it gets to execvp().
> I think that casting mod_name to (char *) is easier than casting array 
> before it gets to execvp(). And it doesn't require
> any changes of tst_run_cmd interfaces (ie they accepts argv the same way 
> as execvp does)...
> 
> Maybe leave everything as is and just cast mod_name to (char *)?

That is easier for one testcase, but will require the cast in each
testcase that will use the interface. From this point of view cast in
the library is better. Not elegant solution though. :(

But in the end any of the solutions will work...

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] tst_run_cmd: const correctness

[chrubis]

Hi!
> >> I think that casting mod_name to (char *) is easier than casting array
> >> before it gets to execvp(). And it doesn't require
> >> any changes of tst_run_cmd interfaces (ie they accepts argv the same way
> >> as execvp does)...
> >>
> >> Maybe leave everything as is and just cast mod_name to (char *)?
> > That is easier for one testcase, but will require the cast in each
> > testcase that will use the interface. From this point of view cast in
> > the library is better. Not elegant solution though. :(
> I google-ed the following:
> http://pubs.opengroup.org/onlinepubs/009604499/functions/exec.html 
> section RATIONALE
> 
> So as I understood from this document execvp expects 'char *const 
> argv[]'  to prevent 'non-expert' users from
> casting argv, envp from "char *[]" to "const char *[]".

Not exactly, it looks like this was choosen to be backward compatible
with older code that declared the array as char *argv[] which couldn't
be automatically casted to const char *const argv[]. So they simple
choose variant that has at least one const and allows automatic cast
from char *argv[].

...
Specifying two levels of const- qualification for the argv[] and envp[]
parameters for the exec functions may seem to be the natural choice,
given that these functions do not modify either the array of pointers or
the characters to which the function points, but this would disallow
existing correct code. Instead, only the array of pointers is noted as
constant. The table of assignment compatibility for dst= src derived
from the ISO C standard summarizes the compatibility:

<snip>

Since all existing code has a source type matching the first row, the
column that gives the most valid combinations is the third column. The
only other possibility is the fourth column, but using it would require
a cast on the argv or envp arguments. It is unfortunate that the fourth
column cannot be used, because the declaration a non-expert would
naturally use would be that in the second row.
...

They are actually complaining that as the type exec functions use is
char *const[] it couldn't be automatically casted from const char *[]
as the array would loose the const attribute. Too bad C cannot gain two
const attributes and cast from char *[] to const char *const[] is not
possible that way exec fuction could have take any string array...

Looks like I've learned something new today ;).

> Given this information and:
> > But in the end any of the solutions will work...
> I vote for a "standard" way i.e. casting array elements to (char *) 
> before calling tst_run_cmd or execvp.

Given that we do not have any existing code passing char *[] array to
the tst_run_cmd() I think that we can declare the array as const char
*const[] and cast it in the library.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2] tst_run_cmd: const correctness

[Stanislav Kholmanskikh]

Signed-off-by: Stanislav kholmanskikh <stanislav.kholmanskikh@oracle.com>
Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/test.h    |    4 ++--
 lib/tst_module.c  |    4 ++--
 lib/tst_run_cmd.c |    6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/test.h b/include/test.h
index 1957148..75a34ab 100644
--- a/include/test.h
+++ b/include/test.h
@@ -225,7 +225,7 @@ long tst_ncpus_max(void);
  * redirection is not needed.
  */
 void tst_run_cmd_fds(void (cleanup_fn)(void),
-			char *const argv[],
+			const char *const argv[],
 			int stdout_fd,
 			int stderr_fd);
 
@@ -236,7 +236,7 @@ void tst_run_cmd_fds(void (cleanup_fn)(void),
  * not needed.
  */
 void tst_run_cmd(void (cleanup_fn)(void),
-		char *const argv[],
+		const char *const argv[],
 		const char *stdout_path,
 		const char *stderr_path);
 
diff --git a/lib/tst_module.c b/lib/tst_module.c
index c0f2c8f..028af0e 100644
--- a/lib/tst_module.c
+++ b/lib/tst_module.c
@@ -85,7 +85,7 @@ void tst_module_load(void (cleanup_fn)(void),
 	while (argv && argv[size])
 		++size;
 	size += offset;
-	char *mod_argv[size + 1]; /* + NULL in the end */
+	const char *mod_argv[size + 1]; /* + NULL in the end */
 	mod_argv[size] = NULL;
 	mod_argv[0] = "insmod";
 	mod_argv[1] = mod_path;
@@ -100,6 +100,6 @@ void tst_module_load(void (cleanup_fn)(void),
 
 void tst_module_unload(void (cleanup_fn)(void), const char *mod_name)
 {
-	char *const argv[] = { "rmmod", mod_name, NULL };
+	const char *const argv[] = { "rmmod", mod_name, NULL };
 	tst_run_cmd(cleanup_fn, argv, NULL, NULL);
 }
diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c
index 714b508..804103c 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -31,7 +31,7 @@
 #define OPEN_FLAGS	(O_WRONLY | O_APPEND | O_CREAT)
 
 void tst_run_cmd_fds(void (cleanup_fn)(void),
-		char *const argv[],
+		const char *const argv[],
 		int stdout_fd,
 		int stderr_fd)
 {
@@ -57,7 +57,7 @@ void tst_run_cmd_fds(void (cleanup_fn)(void),
 			dup2(stderr_fd, STDERR_FILENO);
 		}
 
-		_exit(execvp(argv[0], argv));
+		_exit(execvp(argv[0], (char *const *)argv));
 	}
 
 	int ret = -1;
@@ -73,7 +73,7 @@ void tst_run_cmd_fds(void (cleanup_fn)(void),
 }
 
 void tst_run_cmd(void (cleanup_fn)(void),
-		char *const argv[],
+		const char *const argv[],
 		const char *stdout_path,
 		const char *stderr_path)
 {
-- 
1.7.1


------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2] tst_run_cmd: const correctness

[chrubis]

Hi!
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  include/test.h    |    4 ++--
>  lib/tst_module.c  |    4 ++--
>  lib/tst_run_cmd.c |    6 +++---
>  3 files changed, 7 insertions(+), 7 deletions(-)

Applied, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list