From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28855 invoked by alias); 29 May 2005 06:55:32 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 28823 invoked by uid 22791); 29 May 2005 06:55:26 -0000 Received: from romy.inter.net.il (HELO romy.inter.net.il) (192.114.186.66) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sun, 29 May 2005 06:55:26 +0000 Received: from HOME-C4E4A596F7 (IGLD-80-230-86-142.inter.net.il [80.230.86.142]) by romy.inter.net.il (MOS 3.5.8-GR) with ESMTP id BJJ20894 (AUTH halo1); Sun, 29 May 2005 09:55:13 +0300 (IDT) Date: Sun, 29 May 2005 08:37:00 -0000 Message-Id: From: Eli Zaretskii To: gdb-patches@sources.redhat.com In-reply-to: <20050528230855.GE22435@nevyn.them.org> (message from Daniel Jacobowitz on Sat, 28 May 2005 19:08:55 -0400) Subject: Re: mi tty commands Reply-to: Eli Zaretskii 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> X-SW-Source: 2005-05/txt/msg00612.txt.bz2 > Date: Sat, 28 May 2005 19:08:55 -0400 > From: Daniel Jacobowitz > > 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".