Re: [RFA 1/2] C++-ify break-catch-sig

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-06-05 00:53, Tom Tromey wrote:
> This changes signal_catchpoint to be more of a C++ class, using
> std::vector and updating the users.
> 
> gdb/ChangeLog
> 2017-06-04  Tom Tromey  <tom@tromey.com>
> 
> 	* break-catch-sig.c (gdb_signal_type): Remove typedef.
> 	(struct signal_catchpoint) <signals_to_be_caught>: Now a
> 	std::vector.
> 	(~signal_catchpoint, signal_catchpoint_insert_location)
> 	(signal_catchpoint_remove_location)
> 	(signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one)
> 	(signal_catchpoint_print_mention)
> 	(signal_catchpoint_print_recreate)
> 	(signal_catchpoint_explains_signal): Update.
> 	(create_signal_catchpoint): Change type of "filter".
> 	(catch_signal_split_args): Return a std::vector.
> 	(catch_signal_command): Update.
> ---

Hi Tom,

Thanks for doing this.  The patch looks good in general.  I'd suggest 
doing these in the same patch:

- make the catch_all field/parameter/variable a bool,
- remove the destructor, since it's now empty.

> @@ -99,20 +94,17 @@ static int
>  signal_catchpoint_insert_location (struct bp_location *bl)
>  {
>    struct signal_catchpoint *c = (struct signal_catchpoint *) 
> bl->owner;
> -  int i;
> 
> -  if (c->signals_to_be_caught != NULL)
> +  if (!c->signals_to_be_caught.empty ())
>      {
> -      gdb_signal_type iter;
> +      gdb_signal iter;

You don't need this declaration...

> -      for (i = 0;
> -	   VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -	   i++)
> +      for (auto iter : c->signals_to_be_caught)

...because it's overriden by this one.  Although I'd prefer if you used 
"gdb_signal" instead of "auto" here.  It's not much longer, is more 
expressive.  There are a few instances of this throughout the patch.

> @@ -246,26 +231,25 @@ signal_catchpoint_print_one (struct breakpoint 
> *b,
>      uiout->field_skip ("addr");
>    annotate_field (5);
> 
> -  if (c->signals_to_be_caught
> -      && VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1)
> +  if (!c->signals_to_be_caught.empty ())

I think you changed the behavior of this condition.  It should probably 
be:

   c->signals_to_be_caught.size () > 1


With those comments addressed, the patch is good to me.

Thanks!

Simon