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