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

[Pedro Alves <palves@redhat.com>]

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.

> +  /* 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.

> +  RUN_NORMAL,
> +
> +  /* Stop at the beginning of the program's main function.  */
> +  RUN_STOP_AT_MAIN,
> +
> +  /* Stop at the first instruction of the program.  */
> +  RUN_STOP_AT_FIRST_INSN
> +};
> +

> +/* 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\ .

> +With no arguments, uses arguments last specified (with \"run\" \
> +or \"set args\").\n\
> +To cancel previous arguments and run with no arguments,\n\
> +use \"set args\" without arguments.\n\
> +To start the inferior without using a shell, use \"set \
> +startup-with-shell off\"."
> +


> 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.

> +
> +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.

Thanks,
Pedro Alves