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

Pedro Alves
2017-09-19T14:35:40Z

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