From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4655 invoked by alias); 28 May 2005 23:09:03 -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 4641 invoked by uid 22791); 28 May 2005 23:08:57 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sat, 28 May 2005 23:08:57 +0000 Received: from drow by nevyn.them.org with local (Exim 4.50) id 1DcAQJ-0000dD-IU for gdb-patches@sources.redhat.com; Sat, 28 May 2005 19:08:55 -0400 Date: Sat, 28 May 2005 23:49:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: Re: mi tty commands Message-ID: <20050528230855.GE22435@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com 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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050522210040.GB9231@white> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00594.txt.bz2 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 :-) > +2005-05-22 Bob Rossi > + > + * fork-child.c (fork-inferior): Use accessor function for > + inferior_io_terminal > + * infcmd.c (inferior_io_terminal): make static All ChangeLog entries should start with a capital letter, end with a period, and be generally sentencelike. You've got the third bit down. > + (set_inferior_io_terminal): Add accessor definition > + (get_inferior_io_terminal): Add accessor definition > + (tty_command): Use accessor function > + (_initialize_infcmd): Add inferior_tty setshow variable > + * inferior.h (set_inferior_io_terminal): Add accessor declaration > + (get_inferior_io_terminal): Add accessor declaration > + * nto-procfs (procfs_create_inferior): Use accessor function > + * win32-nat.c (child_create_inferior): Use accessor function > + * mi/mi-cmd-env.c (mi_cmd_inferior_tty_set): Add MI definition > + (mi_cmd_inferior_tty_set): Add MI definition Just "New function" is usually fine. > + * mi/mi-cmds.c (mi_cmds): Add inferior-tty-set/inferior-tty-show Please use "and" instead of /. > + const char *inferior_io_terminal = get_inferior_io_terminal(); That pesky space! :-) > /* If no exec file handed to us, get it from the exec-file command > -- with a good, common error message if none is specified. */ > @@ -261,7 +262,7 @@ > > /* Tell the terminal handling subsystem what tty we plan to run on; > it will just record the information for later. */ > - new_tty_prefork (inferior_io_terminal); > + new_tty_prefork ((char*)inferior_io_terminal); Another pesky space; please use "char *". > + if (inferior_io_terminal) > + xfree ( inferior_io_terminal ); Some pesky anti-spaces. > + /* 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); > + Ditto on the pesky spaces. The Usage: bit is unnecessary. There are no other set commands with underscores; inferior-tty would be better, but would just "tty" be OK? That's even simpler. > -/* File name for default use for standard in/out in the inferior. */ > +/* Set/Get name for default use for standard in/out in the inferior. */ Get doesn't need to be capitalized here. Also, it's still a file name; could you leave that? > +Set terminal for future runs of program being debugged. I think there's either one or two missing uses of "the" here; at least one before program. > +/* Set the inferior terminal device name */ Full sentence with a full stop at the end, please. > + # test that the commands, > + # -inferior-tty-set > + # -inferior-tth-show Test that they what? Or just "Test the commands:". I tried to point out one example of each of the formatting problems; that wasn't an exhaustive list, but you can probably see all the others now. -- Daniel Jacobowitz CodeSourcery, LLC