This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: mi tty commands
- From: Eli Zaretskii <eliz at gnu dot org>
- To: gdb-patches at sources dot redhat dot com
- Date: Sun, 29 May 2005 09:55:23 +0300
- Subject: Re: mi tty commands
- References: <20050224203535.GA19967@white> <01c51b79$Blat.v2.4$4089e9a0@zahav.net.il> <20050225211911.GA21363@white> <20050225212201.GA3592@nevyn.them.org> <20050228162003.GA27783@white> <20050302025219.GA29948@white> <20050311022644.GA15563@white> <20050522210040.GB9231@white> <20050528230855.GE22435@nevyn.them.org>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> Date: Sat, 28 May 2005 19:08:55 -0400
> From: Daniel Jacobowitz <drow@false.org>
>
> On Sun, May 22, 2005 at 05:00:40PM -0400, Bob Rossi wrote:
> > On Thu, Mar 10, 2005 at 09:26:44PM -0500, Bob Rossi wrote:
> > > Ping
> >
> > Ping, and O yeah, I updated the patch the best I could to get the code
> > to conform with the GNU coding standards. I hope I'm getting this right.
>
> Not quite, but I don't mind proofreading :-)
Sorry, I somehow managed to miss the original message. My comments
are below.
> + /* add the filename of the terminal connected to inferior I/O */
> + add_setshow_string_noescape_cmd ( "inferior_tty", class_run,
> + &inferior_io_terminal, _("\
> +Set terminal for future runs of program being debugged."), _("\
> +Show terminal for future runs of program being debugged."), _("\
> +Usage: set inferior_tty /dev/pts/1"), NULL, NULL, &setlist, &showlist);
> + set_cmd_completer (c, filename_completer);
You are adding a new user command, but you didn't add its
documentation to gdb.texinfo. Please add that; I don't want us to
ever have undocumented commands again.
And I agree with Daniel: let's get rid of that underscore in the
command's name.
> +Show terminal for future runs of program being debugged.
I think the ``future'' part should be removed from this sentence: what
this command outputs is the terminal used _now_ as well as in the
future.
> + (set_inferior_io_terminal): Add accessor definition
> + (get_inferior_io_terminal): Add accessor definition
You don't need to write the description of the change more than once.
Here's my suggestion for how to rewrite this:
(set_inferior_io_terminal, get_inferior_io_terminal): Add
accessor definition.
> + * inferior.h (set_inferior_io_terminal): Add accessor declaration
> + (get_inferior_io_terminal): Add accessor declaration
Same here.
> + * nto-procfs (procfs_create_inferior): Use accessor function
> + * win32-nat.c (child_create_inferior): Use accessor function
Same here:
* nto-procfs.c (procfs_create_inferior)
* win32-nat.c (child_create_inferior): Use get_inferior_io_terminal.
Note how I used the explicit name of the function used, instead of
just "accessor function".