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

[Joel Brobecker <brobecker@adacore.com>]

> 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