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

John Baldwin
2017-09-19T18:23:23Z

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