This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/4] GDB: inferior standard I/O redirection
- From: Pedro Alves <palves at redhat dot com>
- To: Cleber Rosa <crosa at redhat dot com>, gdb-patches at sourceware dot org
- Cc: areis at redhat dot com
- Date: Wed, 21 Oct 2015 11:38:42 +0100
- Subject: Re: [PATCH 1/4] GDB: inferior standard I/O redirection
- Authentication-results: sourceware.org; auth=none
- References: <1444045617-14526-1-git-send-email-crosa at redhat dot com> <1444045617-14526-2-git-send-email-crosa at redhat dot com>
On 10/05/2015 12:46 PM, Cleber Rosa wrote:
> +/* Helper function that attempts to open a file and if successful
> + redirects another file descriptor to the one just opened. If
> + file_name is not given, this returns -1 as a simple way to flag
> + a skip (user has not asked for a redirection). Seems OK since
> + this is a helper function. */
> +
> +static int
> +set_std_io_helper (const char *file_name, int std_io_fd, int flags, mode_t mode)
> +{
> + int fd;
> +
> + if (file_name == NULL)
> + return -1;
> +
> + fd = open (file_name, flags, mode);
> + if (fd < 0)
> + return 0;
> +
> + if (dup2 (fd, std_io_fd) == -1) {
{ goes on the next line, and the reindent block.
> + close (fd);
> + return 0;
> + }
> +
> + close (fd);
> + return 1;
> +}
> +
> +/* Attempts to redirect stdin, stdout and stderr. If redirection was
Double-space after period.
> + not requested by the user, it will be skipped in the helper function.
> + In case of errors, each requested and failed redirection error will
> + be passed on for individual error reporting (no details such as
> + errno/perror though). */
> +
> +#define SET_STDIO_ERROR_STDIN 0x01
> +#define SET_STDIO_ERROR_STDOUT 0x02
> +#define SET_STDIO_ERROR_STDERR 0x04
> +
> +static int
> +set_std_io (void)
> +{
> + int result = 0;
> + char *file_name = NULL;
> +
> + if (set_std_io_helper (get_inferior_io_stdin(), 0,
> + O_RDONLY, S_IRUSR | S_IWUSR) == 0)
> + result |= SET_STDIO_ERROR_STDIN;
> +
> + if (set_std_io_helper (get_inferior_io_stdout(), 1,
> + O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR) == 0)
> + result |= SET_STDIO_ERROR_STDOUT;
> +
> + if (set_std_io_helper (get_inferior_io_stderr(), 2,
> + O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR) == 0)
> + result |= SET_STDIO_ERROR_STDERR;
Shouldn't we specify O_APPEND? I'm thinking of the case of starting
multiple inferiors simultaneously under gdb. Not one after the other,
but really in parallel. E.g., run&; add-inferior ...; inferior 2; run&; etc.
> +
> + return result;
> +}
> +
> /* Start an inferior Unix child process and sets inferior_ptid to its
> pid. EXEC_FILE is the file to run. ALLARGS is a string containing
> the arguments to the program. ENV is the environment vector to
> @@ -141,6 +199,7 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> struct inferior *inf;
> int i;
> int save_errno;
> + int io_redir_errors = 0;
>
> /* If no exec file handed to us, get it from the exec-file command
> -- with a good, common error message if none is specified. */
> @@ -358,6 +417,28 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> path to find $SHELL. Rich Pixley says so, and I agree. */
> environ = env;
>
> + /* Sets the inferior process standard I/O (input, output, error)
> + redirection if set with the "set inferior std{in,out,err}"
> + commands. */
> + gdb_flush (gdb_stdout);
> + gdb_flush (gdb_stderr);
> + io_redir_errors = set_std_io();
Space before parens. More instances below.
> + if (io_redir_errors & SET_STDIO_ERROR_STDIN)
> + fprintf_unfiltered (gdb_stderr,
> + "Could not set %s as stdin for %s\n",
Wrap user-visible strings in _() for i18n. More instances below.
> + get_inferior_io_stdin(),
> + exec_file);
> + if (io_redir_errors & SET_STDIO_ERROR_STDOUT)
> + fprintf_unfiltered (gdb_stderr,
> + "Could not set %s as stdout for %s\n",
> + get_inferior_io_stdout(),
> + exec_file);
> + if (io_redir_errors & SET_STDIO_ERROR_STDERR)
> + fprintf_unfiltered (gdb_stderr,
> + "Could not set %s as stderr for %s\n",
> + get_inferior_io_stderr(),
> + exec_file);
> +
> if (exec_fun != NULL)
> (*exec_fun) (argv[0], argv, env);
> else
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 4713490..9941a98 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -115,6 +115,11 @@ static char *inferior_args_scratch;
>
> static char *inferior_io_terminal_scratch;
>
> +/* Scratch area where 'set inferior-{stdin,stdout,stderr}' will store
> + user provided value. */
> +
> +static char *inferior_io_std_scratch[3] = { NULL, NULL, NULL };
> +
> /* Pid of our debugged inferior, or 0 if no inferior now.
> Since various parts of infrun.c test this to see whether there is a program
> being debugged it should be nonzero (currently 3 is used) for remote
> @@ -182,6 +187,117 @@ show_inferior_tty_command (struct ui_file *file, int from_tty,
> "is \"%s\".\n"), inferior_io_terminal);
> }
>
New functions below need intro comments. For extern functions,
use "/* See whatever.h. */".
> +static void
> +set_inferior_io_helper (const char *file_name, int stdfd)
> +{
> + gdb_assert (0 <= stdfd && stdfd <= 2);
> + xfree (current_inferior ()->standard_io[stdfd]);
> + current_inferior ()->standard_io[stdfd] =
> + file_name != NULL ? xstrdup (file_name) : NULL;
> +}
> +
> +void
> +set_inferior_io_stdin (const char *file_name)
> +{
> + set_inferior_io_helper(file_name, 0);
Space before parens.
> diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
> index 4395c98..2ae4f90 100644
> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -798,6 +798,12 @@ gdb_test "thread find" "Command requires an argument." "thread find"
> gdb_test "thread name" "No thread selected" "thread name"
> #test tty
> gdb_test "tty" "Argument required .filename to set it to\..*" "tty"
> +#test stdin
> +gdb_test "stdin" "Argument required .filename to set it to\..*" "stdin"
> +#test stdout
> +gdb_test "stdout" "Argument required .filename to set it to\..*" "stdout"
> +#test stderr
> +gdb_test "stderr" "Argument required .filename to set it to\..*" "stderr"
> #test until "u" abbreviation
> gdb_test "u" "The program is not being run." "until \"u\" abbreviation"
> #test until
>
It'd be great if we also tested that it actually works.
Thanks,
Pedro Alves