[PATCH 2/3] Error out immediatly when using if command without args in command list
Simon Marchi
simon.marchi@polymtl.ca
Mon Sep 4 17:15:00 GMT 2017
On 2017-09-04 14:35, Pedro Alves wrote:
> On 09/02/2017 10:36 PM, Simon Marchi wrote:
>> When using "if" (or while) without args directly on gdb's command
>> line,
>> you get this:
>>
>> (gdb) if
>> if/while commands require arguments
>>
>> When doing the same when entering a command list, you only get an
>> error
>> when the command is executed, when parse_exp_in_context_1 fails to
>> evaluate the expression.
>>
>> (gdb) define foo
>> Type commands for definition of "foo".
>> End with a line saying just "end".
>> >if
>> >end
>> >end
>> (gdb) foo
>> Argument required (expression to compute).
>>
>> I think it would make more sense to error out when inputting the
>> command
>> list directly:
>>
>> (gdb) define foo
>> Type commands for definition of "foo".
>> End with a line saying just "end".
>> >if
>> if/while commands require arguments.
>>
>> The only required change is to check whether args is an empty string
>> in
>> build_command_line.
>>
>
> LGTM. Tiny nit further below.
>
> BTW, as a potential improvement, we could consider also not
> canceling the whole command definition, but instead go back to
> expecting another line. It's a bit annoying to have to type
> everything from scratch. I've run into that occasionally with
> tracepoints, like:
>
> (gdb) trace foo
> (gdb) actions
> Enter actions for tracepoint 1, one per line.
> End with a line saying just "end".
> >collect ...
> >collect ...
> > #... several lines later:
> >endd # whoops, a typo.
> `endd' is not a tracepoint action, or is ambiguous.
> (gdb) # bah, have to start over.
>
> Instead of:
>
> (gdb) trace foo
> (gdb) actions
> Enter actions for tracepoint 1, one per line.
> End with a line saying just "end".
> >collect ...
> >collect ...
> > #... several lines later:
> >endd
> `endd' is not a tracepoint action, or is ambiguous.
> >end
> (gdb)
>
> The same safety net applied to if/while typos might be useful.
> Just an idea.
I thought about the same thing. We just need to make it clear that the
erroneous command didn't make it in the command list.
>> --- a/gdb/cli/cli-script.c
>> +++ b/gdb/cli/cli-script.c
>> @@ -147,7 +147,8 @@ build_command_line (enum command_control_type
>> type, const char *args)
>> {
>> struct command_line *cmd;
>>
>> - if (args == NULL && (type == if_control || type == while_control))
>> + if ((args == NULL || strlen (args) == 0)
>> + && (type == if_control || type == while_control))
>> error (_("if/while commands require arguments."));
>> gdb_assert (args != NULL);
>
> Nit: might not make a difference with modern compilers, though
> the canonical way to check for entry string would be:
>
> *args == '\0'
Ok, I'm pushing now with that changed.
Thanks,
Simon
More information about the Gdb-patches
mailing list