Re: [PATCH v3] Add a 'starti' command.

[John Baldwin <jhb@freebsd.org>]

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 <http://www.gnu.org/licenses/>.  */
> > +
> > +#include <stdio.h>
> 
> 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