From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 93813 invoked by alias); 21 Oct 2015 10:38:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 93798 invoked by uid 89); 21 Oct 2015 10:38:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 21 Oct 2015 10:38:46 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id B6EC7AE5C4 for ; Wed, 21 Oct 2015 10:38:44 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t9LAcgue025400; Wed, 21 Oct 2015 06:38:43 -0400 Message-ID: <56276B32.6060302@redhat.com> Date: Wed, 21 Oct 2015 11:18:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Cleber Rosa , gdb-patches@sourceware.org CC: areis@redhat.com Subject: Re: [PATCH 1/4] GDB: inferior standard I/O redirection References: <1444045617-14526-1-git-send-email-crosa@redhat.com> <1444045617-14526-2-git-send-email-crosa@redhat.com> In-Reply-To: <1444045617-14526-2-git-send-email-crosa@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-10/txt/msg00393.txt.bz2 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