From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75380 invoked by alias); 19 Sep 2017 18:23:23 -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 75358 invoked by uid 89); 19 Sep 2017 18:23:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=Space, Hx-languages-length:2458, H*r:sk:c-73-23, H*RU:sk:c-73-23 X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 19 Sep 2017 18:23:20 +0000 Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 5D26310A82E; Tue, 19 Sep 2017 14:23:18 -0400 (EDT) From: John Baldwin To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3] Add a 'starti' command. Date: Tue, 19 Sep 2017 18:23:00 -0000 Message-ID: <19338806.QVLJa9ilSl@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.1-STABLE; KDE/4.14.30; amd64; ; ) In-Reply-To: <7b165874-88ec-0ef2-50c5-9eb7b19eeb53@redhat.com> References: <20170911220803.73819-1-jhb@FreeBSD.org> <7b165874-88ec-0ef2-50c5-9eb7b19eeb53@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00468.txt.bz2 On Tuesday, September 19, 2017 03:35:31 PM Pedro Alves wrote: > Hi John, > > This looks good to me, with a couple minor nits below. > > On 09/11/2017 11:08 PM, John Baldwin wrote: > > > > > -/* Implement the "run" command. If TBREAK_AT_MAIN is set, then insert > > - a temporary breakpoint at the begining of the main program before > > - running the program. */ > > +/* Determine how the new inferior will behave. */ > > + > > +enum run_how { > > { goes on next line. Ok. > > + /* Do not create any breakpoint. */ > > I wonder about tweaking this comment to avoid talking > about breakpoints, since this enum is not really strictly > about breakpoints anymore. Hmm, how about: enum run_how { /* Run program without any explicit stop during startup. */ RUN_NORMAL, ... > > +/* This help string is used for the run, start, and starti commands. > > + It is defined as a macro to prevent duplication. */ > > + > > +#define RUN_ARGS_HELP \ > > +"You may specify arguments to give it.\n\ > > +Args may include \"*\", or \"[...]\"; they are expanded using the\n\ > > +shell that will start the program (specified by the \"$SHELL\"\ > > +environment\nvariable). Input and output redirection with \">\",\ > > +\"<\", or \">>\"\nare also allowed.\n\n\ > > You have a "\n" in the middle of some lines above. Was that intended? > I'd expect to see instead lines broken at \n, ending with \n\ . Those were just copy and pasted from the run help. I think they were there to avoid overflowing 80 columns in the source. I've rewrapped the text so that newlines are at the end. > > diff --git a/gdb/testsuite/gdb.base/starti.c b/gdb/testsuite/gdb.base/starti.c > > new file mode 100644 > > index 0000000000..dc098fe8aa > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/starti.c > > @@ -0,0 +1,30 @@ > > > + You should have received a copy of the GNU General Public License > > + along with this program. If not, see . */ > > + > > +#include > > This isn't necessary, AFAICS. Oops, yes. > > + > > +int x; > > + > > +__attribute__((constructor)) void ctor() > > +{ > > + x = 1; > > +} > > + > > +int main() > > +{ > > + return 0; > > +} > > Space before parens, line break after return type. > We follow GNU convention in tests too, unless different syntax is > relevant for the test. Ok. (Ironically I copied both the spurious #include and bad style from start.c.) -- John Baldwin