From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18160 invoked by alias); 3 Sep 2017 16:27:28 -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 17952 invoked by uid 89); 3 Sep 2017 16:27:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=shame, hugely, offer X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 03 Sep 2017 16:27:23 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v83GRG1H003470 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 3 Sep 2017 12:27:21 -0400 Received: by simark.ca (Postfix, from userid 112) id 58A3C1EA26; Sun, 3 Sep 2017 12:27:16 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 471E21E974; Sun, 3 Sep 2017 12:27:14 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 03 Sep 2017 16:27:00 -0000 From: Simon Marchi To: Jon Beniston Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] PR21985: set inferior-tty doesn't work for remote or sim In-Reply-To: <00f001d31a99$7080bd00$51823700$@beniston.com> References: <00f001d31a99$7080bd00$51823700$@beniston.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 3 Sep 2017 16:27:16 +0000 X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00024.txt.bz2 Hi Jon, Thanks for the patch and sorry for the delay. When I try to apply it locally, git complains that it is corrupted. Most likely your email client wrapped some lines. To avoid this, I suggest using the "git send-email" command. I was able to apply the patch you attached to the bug, however (I suppose it's the same). Some notes about formatting: - Please verify the usage of tabs/spaces in the indentation. 8 consecutive spaces become a tab. There's at least one instance, in remote_fileio_func_write, that should be fixed. - There are some trailing spaces here and there, please remove them. I have never used the simulator nor remote fileio, so this is a good pretext for me to learn about it. Would it be possible for you to provide some small programs that I could toy with to test the patch (and how to run them, if you feel really nice :)) ? I have some small comments inline: On 2017-08-21 18:20, Jon Beniston wrote: > The set inferior-tty command doesn't appear to work for remote or sim > targets. (That is, remote targets using remote-fileio to have the IO > appear > on the debug host). > > This is a shame as Eclipse (and potentially other front-ends) use this > command to separate the windows used for target stdio and the gdb > console. > Currently, therefore Eclipse isn't supporting remote and sim targets > fully. > > The attached patch adds support for this command in remote-fileio.c and > remote-sim.c. Reads/writes from/to stdin/stdout are redirected to the > tty. > > This may not be the ideal way to implement this within GDB, but it does > get > it working with Eclipse. I'm not hugely familiar with what ttys should > offer, other than redirect stdio. I looks right, Eclipse (or the user) should send the "set inferior-tty" command, but after it shouldn't have to do anything. GDB should take care of it. > diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c > index 252b423bfa..d36d2e7c43 100644 > --- a/gdb/remote-fileio.c > +++ b/gdb/remote-fileio.c > @@ -29,6 +29,7 @@ > #include "target.h" > #include "filenames.h" > #include "filestuff.h" > +#include "inferior.h" > > #include > #include "gdb_sys_time.h" > @@ -40,6 +41,8 @@ > static struct { > int *fd_map; > int fd_map_size; > + int tty; /* File descriptor for tty to use if > set > + inferior-tty called. */ > } remote_fio_data; > > #define FIO_FD_INVALID -1 > @@ -52,7 +55,8 @@ static int > remote_fileio_init_fd_map (void) > { > int i; > - > + const char *term; You can declare the variables where they are used. > + > if (!remote_fio_data.fd_map) > { > remote_fio_data.fd_map = XNEWVEC (int, 10); > @@ -62,6 +66,15 @@ remote_fileio_init_fd_map (void) > remote_fio_data.fd_map[2] = FIO_FD_CONSOLE_OUT; > for (i = 3; i < 10; ++i) > remote_fio_data.fd_map[i] = FIO_FD_INVALID; > + term = get_inferior_io_terminal (); > + if (term != NULL) > + { > + remote_fio_data.tty = open (term, O_RDWR | O_NOCTTY); This should probably use gdb_open_cloexec. > @@ -357,15 +385,22 @@ gdb_os_flush_stdout (host_callback *p) > static int > gdb_os_write_stderr (host_callback *p, const char *buf, int len) > { > + struct sim_inferior_data *sim_data > + = get_sim_inferior_data (current_inferior (), > SIM_INSTANCE_NEEDED); > int i; > char b[2]; > > - for (i = 0; i < len; i++) > + if (sim_data->tty < 0) > { > - b[0] = buf[i]; > - b[1] = 0; > - fputs_unfiltered (b, gdb_stdtargerr); > + for (i = 0; i < len; i++) > + { > + b[0] = buf[i]; > + b[1] = 0; > + fputs_unfiltered (b, gdb_stdtargerr); > + } The code in gdb_os_write_stdout has been changed long ago to use ui_file_write (and as you found, some unused variables were left) instead of this kind of loop. Did you try to replace this loop here with a similar ui_file_write call? > } > + else > + len = write (sim_data->tty, buf, len); > return len; > } > > @@ -609,6 +644,7 @@ gdbsim_create_inferior (struct target_ops *target, > const > char *exec_file, > int len; > char *arg_buf; > const char *args = allargs.c_str (); > + const char *term; > > if (exec_file == 0 || exec_bfd == 0) > warning (_("No executable file specified.")); > @@ -652,6 +688,16 @@ gdbsim_create_inferior (struct target_ops *target, > const char *exec_file, > > insert_breakpoints (); /* Needed to get correct instruction > in cache. */ > + > + /* We don't do this in open as Eclispe calls set-inferior-tty after > + target-select, but before run. */ Eclispe -> Eclipse. Although I wouldn't refer to a particular front-end. You could say something more generic like: "We do this here and not in open, so that it's possible to call "set inferior-tty" after connecting to the target but before run." > + term = get_inferior_io_terminal (); > + if (term != NULL) > + { > + sim_data->tty = open (term, O_RDWR | O_NOCTTY); This should also probably use gdb_open_cloexec. > + if (sim_data->tty < 0) > + error (_("Unable to open %s as inferior-tty."), term); > + } > > clear_proceed_status (0); > } Thanks, Simon