This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3] Add a 'starti' command.
- From: Pedro Alves <palves at redhat dot com>
- To: John Baldwin <jhb at FreeBSD dot org>, gdb-patches at sourceware dot org
- Date: Tue, 19 Sep 2017 15:35:31 +0100
- Subject: Re: [PATCH v3] Add a 'starti' command.
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DF6B77EA91
- References: <20170911220803.73819-1-jhb@FreeBSD.org>
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