Re: [PATCH 2/4] Error on bad count number

Joel Brobecker
2014-03-05T14:29:41Z

> GDB is quiet for these invalid input like this
>
> (gdb) enable count 1.1 1
> (gdb)
>
> This patch is to check the input number is valid, and emit error if
> input number isn't valid. With this patch, it becomes:
>
> (gdb) enable count 1.1 1
> Bad count number '1.1 1'
>
> gdb:
>
> 2014-03-05 Yao Qi <yao@codesourcery.com>
>
> * breakpoint.c (enable_count_command): Emit error if 'count'
> is zero.
>
> gdb/testsuite:
>
> 2014-03-05 Yao Qi <yao@codesourcery.com>
>
> * gdb.base/ena-dis-br.exp: Test bad count number.

I had a slight hesitation here, as we are now going to actively
reject "enable count 0 1" where it used to be accepted. But since
it makes little sense, and does not work, I think that's OK.
Example of it not working:

(gdb) b a
Breakpoint 1 at 0x401558: file a.adb, line 3.
(gdb) enable count 0 1
(gdb) run
Starting program: /[...]/a
Breakpoint 1, a () at a.adb:3
3 raise Constraint_Error;

The good news is that the breakpoint gets disabled afterwards,
so it's not completely broken:

(gdb) info break
Num Type Disp Enb Address What
1 breakpoint dis n 0x0000000000401558 in a at a.adb:3
breakpoint already hit 1 time

> + if (count == 0)
> + error (_("Bad count number '%s'"), p);

I would change it to "Invalid count number" instead.

> +
> map_breakpoint_numbers (args, do_map_enable_count_breakpoint, &count);
> }
>
> diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
> index 6f2c469..4b83e56 100644
> --- a/gdb/testsuite/gdb.base/ena-dis-br.exp
> +++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
> @@ -169,6 +169,11 @@ gdb_test "continue" \
> ".*marker1 .*:($bp_location15|$bp_location16).*" \
> "continue through enable count, now disabled"
>
> +# Test enable count with bad count number.
> +
> +gdb_test "enable count 1.1 $bp" "Bad count number '1.1 $bp'.*" \
> + "enable count with bad number"

Can you also add a test for '0' as the count number?

> # Verify that we can set a breakpoint with an ignore count N, which
> # should cause the next N triggers of the bp to be ignored. (This is
> # a flavor of enablement/disablement, after all.)

Thank you,
--
Joel