Re: [RFA v2 3/4] Use std::vector in syscall_catchpoint

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Wednesday, July 19 2017, Tom Tromey wrote:

> This changes syscall_catchpoint to use a std::vector rather than a VEC
> for "syscalls_to_be_caught".  This simplifies the code a bit.

Thanks for the patch, Tom.  Just a few nits.

> 2017-07-17  Tom Tromey  <tom@tromey.com>
>
> 	* break-catch-syscall.c (syscall_catchpoint)
> 	<syscalls_to_be_caught>: Now a std::vector<int>
> 	(~syscall_catchpoint): Remove.
> 	(insert_catch_syscall, remove_catch_syscall)
> 	(breakpoint_hit_catch_syscall, print_one_catch_syscall)
> 	(print_mention_catch_syscall, print_recreate_catch_syscall):
> 	Update.
> 	(create_syscall_event_catchpoint): Change type of "filter"
> 	parameter.
> 	(catch_syscall_split_args): Return a std::vector.
> 	(catch_syscall_command_1, catching_syscall_number_1): Update.
> ---
>  gdb/ChangeLog             |  14 ++++++
>  gdb/break-catch-syscall.c | 112 ++++++++++++++--------------------------------
>  2 files changed, 48 insertions(+), 78 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index e178fa5..fb953ed 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,17 @@
> +2017-07-17  Tom Tromey  <tom@tromey.com>
> +
> +	* break-catch-syscall.c (syscall_catchpoint)
> +	<syscalls_to_be_caught>: Now a std::vector<int>
> +	(~syscall_catchpoint): Remove.
> +	(insert_catch_syscall, remove_catch_syscall)
> +	(breakpoint_hit_catch_syscall, print_one_catch_syscall)
> +	(print_mention_catch_syscall, print_recreate_catch_syscall):
> +	Update.
> +	(create_syscall_event_catchpoint): Change type of "filter"
> +	parameter.
> +	(catch_syscall_split_args): Return a std::vector.
> +	(catch_syscall_command_1, catching_syscall_number_1): Update.
> +
>  2017-06-04  Tom Tromey  <tom@tromey.com>
>  
>  	* break-catch-throw.c (struct exception_catchpoint)
> diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
> index eb51a61..ff0cb69 100644
> --- a/gdb/break-catch-syscall.c
> +++ b/gdb/break-catch-syscall.c
> @@ -36,22 +36,12 @@
>  
>  struct syscall_catchpoint : public breakpoint
>  {
> -  ~syscall_catchpoint () override;
> -
>    /* Syscall numbers used for the 'catch syscall' feature.  If no
> -     syscall has been specified for filtering, its value is NULL.
> -     Otherwise, it holds a list of all syscalls to be caught.  The
> -     list elements are allocated with xmalloc.  */
> -  VEC(int) *syscalls_to_be_caught;
> +     syscall has been specified for filtering, it is empty.
> +     Otherwise, it holds a list of all syscalls to be caught.  */
> +  std::vector<int> syscalls_to_be_caught;
>  };
>  
> -/* catch_syscall destructor.  */
> -
> -syscall_catchpoint::~syscall_catchpoint ()
> -{
> -  VEC_free (int, this->syscalls_to_be_caught);
> -}
> -
>  static const struct inferior_data *catch_syscall_inferior_data = NULL;
>  
>  struct catch_syscall_inferior_data
> @@ -106,15 +96,11 @@ insert_catch_syscall (struct bp_location *bl)
>      = get_catch_syscall_inferior_data (inf);
>  
>    ++inf_data->total_syscalls_count;
> -  if (!c->syscalls_to_be_caught)
> +  if (c->syscalls_to_be_caught.empty ())
>      ++inf_data->any_syscall_count;
>    else
>      {
> -      int i, iter;
> -
> -      for (i = 0;
> -           VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -           i++)
> +      for (int iter : c->syscalls_to_be_caught)
>  	{
>            int elem;
>  
> @@ -157,15 +143,11 @@ remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason)
>      = get_catch_syscall_inferior_data (inf);
>  
>    --inf_data->total_syscalls_count;
> -  if (!c->syscalls_to_be_caught)
> +  if (c->syscalls_to_be_caught.empty ())
>      --inf_data->any_syscall_count;
>    else
>      {
> -      int i, iter;
> -
> -      for (i = 0;
> -           VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -           i++)
> +      for (int iter : c->syscalls_to_be_caught)
>  	{
>            int elem;
>  	  if (iter >= VEC_length (int, inf_data->syscalls_counts))
> @@ -207,13 +189,9 @@ breakpoint_hit_catch_syscall (const struct bp_location *bl,
>    syscall_number = ws->value.syscall_number;
>  
>    /* Now, checking if the syscall is the same.  */
> -  if (c->syscalls_to_be_caught)
> +  if (!c->syscalls_to_be_caught.empty ())
>      {
> -      int i, iter;
> -
> -      for (i = 0;
> -           VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -           i++)
> +      for (int iter : c->syscalls_to_be_caught)
>  	if (syscall_number == iter)
>  	  return 1;
>  
> @@ -296,20 +274,16 @@ print_one_catch_syscall (struct breakpoint *b,
>      uiout->field_skip ("addr");
>    annotate_field (5);
>  
> -  if (c->syscalls_to_be_caught
> -      && VEC_length (int, c->syscalls_to_be_caught) > 1)
> +  if (c->syscalls_to_be_caught.size () > 1)
>      uiout->text ("syscalls \"");
>    else
>      uiout->text ("syscall \"");
>  
> -  if (c->syscalls_to_be_caught)
> +  if (!c->syscalls_to_be_caught.empty ())
>      {
> -      int i, iter;
>        char *text = xstrprintf ("%s", "");
>  
> -      for (i = 0;
> -           VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -           i++)
> +      for (int iter : c->syscalls_to_be_caught)
>          {
>            char *x = text;
>            struct syscall s;
> @@ -346,18 +320,14 @@ print_mention_catch_syscall (struct breakpoint *b)
>    struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
>    struct gdbarch *gdbarch = b->loc->gdbarch;
>  
> -  if (c->syscalls_to_be_caught)
> +  if (!c->syscalls_to_be_caught.empty ())
>      {
> -      int i, iter;
> -
> -      if (VEC_length (int, c->syscalls_to_be_caught) > 1)
> +      if (c->syscalls_to_be_caught.size () > 1)
>          printf_filtered (_("Catchpoint %d (syscalls"), b->number);
>        else
>          printf_filtered (_("Catchpoint %d (syscall"), b->number);
>  
> -      for (i = 0;
> -           VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -           i++)
> +      for (int iter : c->syscalls_to_be_caught)
>          {
>            struct syscall s;
>            get_syscall_by_number (gdbarch, iter, &s);
> @@ -385,23 +355,17 @@ print_recreate_catch_syscall (struct breakpoint *b, struct ui_file *fp)
>  
>    fprintf_unfiltered (fp, "catch syscall");
>  
> -  if (c->syscalls_to_be_caught)
> +  for (int iter : c->syscalls_to_be_caught)
>      {
> -      int i, iter;
> -
> -      for (i = 0;
> -           VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -           i++)
> -        {
> -          struct syscall s;
> +      struct syscall s;
>  
> -          get_syscall_by_number (gdbarch, iter, &s);
> -          if (s.name)
> -            fprintf_unfiltered (fp, " %s", s.name);
> -          else
> -            fprintf_unfiltered (fp, " %d", s.number);
> -        }
> +      get_syscall_by_number (gdbarch, iter, &s);
> +      if (s.name)
> +	fprintf_unfiltered (fp, " %s", s.name);
> +      else
> +	fprintf_unfiltered (fp, " %d", s.number);
>      }
> +
>    print_recreate_thread (b, fp);
>  }
>  
> @@ -418,7 +382,7 @@ syscall_catchpoint_p (struct breakpoint *b)
>  }
>  
>  static void
> -create_syscall_event_catchpoint (int tempflag, VEC(int) *filter,
> +create_syscall_event_catchpoint (int tempflag, std::vector<int> &&filter,
>                                   const struct breakpoint_ops *ops)
>  {
>    struct syscall_catchpoint *c;
> @@ -431,13 +395,11 @@ create_syscall_event_catchpoint (int tempflag, VEC(int) *filter,
>    install_breakpoint (0, c, 1);
>  }
>  
> -/* Splits the argument using space as delimiter.  Returns an xmalloc'd
> -   filter list, or NULL if no filtering is required.  */
> -static VEC(int) *
> +/* Splits the argument using space as delimiter.  */
> +static std::vector<int>

Space between comment and function declaration (I know this was wrong
before).

>  catch_syscall_split_args (char *arg)
>  {
> -  VEC(int) *result = NULL;
> -  struct cleanup *cleanup = make_cleanup (VEC_cleanup (int), &result);
> +  std::vector<int> result;
>    struct gdbarch *gdbarch = target_gdbarch ();
>  
>    while (*arg != '\0')
> @@ -460,7 +422,7 @@ catch_syscall_split_args (char *arg)
>        if (*endptr == '\0')
>  	{
>  	  get_syscall_by_number (gdbarch, syscall_number, &s);
> -	  VEC_safe_push (int, result, s.number);
> +	  result.push_back (s.number);
>  	}
>        else if (startswith (cur_name, "g:")
>  	       || startswith (cur_name, "group:"))
> @@ -482,7 +444,7 @@ catch_syscall_split_args (char *arg)
>  	    {
>  	      /* Insert each syscall that are part of the group.  No
>  		 need to check if it is valid.  */
> -	      VEC_safe_push (int, result, syscall_list[i].number);
> +	      result.push_back (syscall_list[i].number);
>  	    }
>  
>  	  xfree (syscall_list);
> @@ -500,11 +462,10 @@ catch_syscall_split_args (char *arg)
>  	    error (_("Unknown syscall name '%s'."), cur_name);
>  
>  	  /* Ok, it's valid.  */
> -	  VEC_safe_push (int, result, s.number);
> +	  result.push_back (s.number);
>  	}
>      }
>  
> -  discard_cleanups (cleanup);
>    return result;
>  }
>  
> @@ -515,7 +476,7 @@ catch_syscall_command_1 (char *arg, int from_tty,
>  			 struct cmd_list_element *command)
>  {
>    int tempflag;
> -  VEC(int) *filter;
> +  std::vector<int> filter;
>    struct syscall s;
>    struct gdbarch *gdbarch = get_current_arch ();
>  
> @@ -542,10 +503,8 @@ this architecture yet."));
>  
>    if (arg != NULL)
>      filter = catch_syscall_split_args (arg);
> -  else
> -    filter = NULL;
>  
> -  create_syscall_event_catchpoint (tempflag, filter,
> +  create_syscall_event_catchpoint (tempflag, std::move (filter),
>  				   &catch_syscall_breakpoint_ops);
>  }
>  
> @@ -586,12 +545,9 @@ catching_syscall_number_1 (struct breakpoint *b,
>      {
>        struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
>  
> -      if (c->syscalls_to_be_caught)
> +      if (!c->syscalls_to_be_caught.empty ())
>  	{
> -	  int i, iter;
> -	  for (i = 0;
> -	       VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
> -	       i++)
> +	  for (int iter : c->syscalls_to_be_caught)
>  	    if (syscall_number == iter)
>  	      return 1;
>  	}
> -- 
> 2.9.3

LGTM.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/