[RFA v2 0/4] C++-ify some breakpoint subclasses a bit more

[RFA v2 0/4] C++-ify some breakpoint subclasses a bit more

[Tom Tromey]

This is v2 of this series:

https://sourceware.org/ml/gdb-patches/2017-06/msg00096.html

I believe this fixes all the review comments.
I also added a couple of patches to fix up "catch syscall" a bit.

Regtested on the buildbot.

Tom

[RFA v2 1/4] C++-ify break-catch-sig

[Tom Tromey]

This changes signal_catchpoint to be more of a C++ class, using
std::vector and updating the users.

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.
	<catch_all>: Now a bool.
	(~signal_catchpoint): Remove.
	(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" and
	"catch_all".
	(catch_signal_split_args): Return a std::vector.  Change type of
	"catch_all".
	(catch_signal_command): Update.
---
 gdb/ChangeLog         |  19 +++++++
 gdb/break-catch-sig.c | 143 +++++++++++++++++---------------------------------
 2 files changed, 66 insertions(+), 96 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bc6e55b..42bde4b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+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.
+	<catch_all>: Now a bool.
+	(~signal_catchpoint): Remove.
+	(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" and
+	"catch_all".
+	(catch_signal_split_args): Return a std::vector.  Change type of
+	"catch_all".
+	(catch_signal_command): Update.
+
 2017-07-18  David Blaikie  <dblaikie@gmail.com>
 
 	* dwarf2read.c (create_cus_hash_table): Re-add lost initialization
diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 3eede93..afac016 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -33,30 +33,24 @@
 
 #define INTERNAL_SIGNAL(x) ((x) == GDB_SIGNAL_TRAP || (x) == GDB_SIGNAL_INT)
 
-typedef enum gdb_signal gdb_signal_type;
-
-DEF_VEC_I (gdb_signal_type);
-
 /* An instance of this type is used to represent a signal catchpoint.
    A breakpoint is really of this type iff its ops pointer points to
    SIGNAL_CATCHPOINT_OPS.  */
 
 struct signal_catchpoint : public breakpoint
 {
-  ~signal_catchpoint () override;
-
   /* Signal numbers used for the 'catch signal' feature.  If no signal
-     has been specified for filtering, its value is NULL.  Otherwise,
+     has been specified for filtering, it is empty.  Otherwise,
      it holds a list of all signals to be caught.  */
 
-  VEC (gdb_signal_type) *signals_to_be_caught;
+  std::vector<gdb_signal> signals_to_be_caught;
 
-  /* If SIGNALS_TO_BE_CAUGHT is NULL, then all "ordinary" signals are
+  /* If SIGNALS_TO_BE_CAUGHT is empty, then all "ordinary" signals are
      caught.  If CATCH_ALL is non-zero, then internal signals are
-     caught as well.  If SIGNALS_TO_BE_CAUGHT is non-NULL, then this
+     caught as well.  If SIGNALS_TO_BE_CAUGHT is not empty, then this
      field is ignored.  */
 
-  int catch_all;
+  bool catch_all;
 };
 
 /* The breakpoint_ops structure to be used in signal catchpoints.  */
@@ -85,13 +79,6 @@ signal_to_name_or_int (enum gdb_signal sig)
 
 \f
 
-/* signal_catchpoint destructor.  */
-
-signal_catchpoint::~signal_catchpoint ()
-{
-  VEC_free (gdb_signal_type, this->signals_to_be_caught);
-}
-
 /* Implement the "insert_location" breakpoint_ops method for signal
    catchpoints.  */
 
@@ -99,20 +86,15 @@ 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;
-
-      for (i = 0;
-	   VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
-	   i++)
+      for (gdb_signal iter : c->signals_to_be_caught)
 	++signal_catch_counts[iter];
     }
   else
     {
-      for (i = 0; i < GDB_SIGNAL_LAST; ++i)
+      for (int i = 0; i < GDB_SIGNAL_LAST; ++i)
 	{
 	  if (c->catch_all || !INTERNAL_SIGNAL (i))
 	    ++signal_catch_counts[i];
@@ -132,15 +114,10 @@ signal_catchpoint_remove_location (struct bp_location *bl,
 				   enum remove_bp_reason reason)
 {
   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;
-
-      for (i = 0;
-	   VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
-	   i++)
+      for (gdb_signal iter : c->signals_to_be_caught)
 	{
 	  gdb_assert (signal_catch_counts[iter] > 0);
 	  --signal_catch_counts[iter];
@@ -148,7 +125,7 @@ signal_catchpoint_remove_location (struct bp_location *bl,
     }
   else
     {
-      for (i = 0; i < GDB_SIGNAL_LAST; ++i)
+      for (int i = 0; i < GDB_SIGNAL_LAST; ++i)
 	{
 	  if (c->catch_all || !INTERNAL_SIGNAL (i))
 	    {
@@ -174,7 +151,7 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl,
 {
   const struct signal_catchpoint *c
     = (const struct signal_catchpoint *) bl->owner;
-  gdb_signal_type signal_number;
+  gdb_signal signal_number;
 
   if (ws->kind != TARGET_WAITKIND_STOPPED)
     return 0;
@@ -184,18 +161,12 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl,
   /* If we are catching specific signals in this breakpoint, then we
      must guarantee that the called signal is the same signal we are
      catching.  */
-  if (c->signals_to_be_caught)
+  if (!c->signals_to_be_caught.empty ())
     {
-      int i;
-      gdb_signal_type iter;
-
-      for (i = 0;
-           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
-           i++)
+      for (gdb_signal iter : c->signals_to_be_caught)
 	if (signal_number == iter)
 	  return 1;
       /* Not the same.  */
-      gdb_assert (!iter);
       return 0;
     }
   else
@@ -246,26 +217,24 @@ 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 ())
     uiout->text ("signals \"");
   else
     uiout->text ("signal \"");
 
-  if (c->signals_to_be_caught)
+  if (c->signals_to_be_caught.size () > 1)
     {
-      int i;
-      gdb_signal_type iter;
       std::string text;
 
-      for (i = 0;
-           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
-           i++)
+      bool first = true;
+      for (gdb_signal iter : c->signals_to_be_caught)
         {
 	  const char *name = signal_to_name_or_int (iter);
 
-	  if (i > 0)
+	  if (!first)
 	    text += " ";
+	  first = false;
+
 	  text += name;
         }
       uiout->field_string ("what", text.c_str ());
@@ -287,19 +256,14 @@ signal_catchpoint_print_mention (struct breakpoint *b)
 {
   struct signal_catchpoint *c = (struct signal_catchpoint *) b;
 
-  if (c->signals_to_be_caught)
+  if (!c->signals_to_be_caught.empty ())
     {
-      int i;
-      gdb_signal_type iter;
-
-      if (VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1)
+      if (c->signals_to_be_caught.size () > 1)
         printf_filtered (_("Catchpoint %d (signals"), b->number);
       else
         printf_filtered (_("Catchpoint %d (signal"), b->number);
 
-      for (i = 0;
-           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
-           i++)
+      for (gdb_signal iter : c->signals_to_be_caught)
         {
 	  const char *name = signal_to_name_or_int (iter);
 
@@ -323,14 +287,9 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp)
 
   fprintf_unfiltered (fp, "catch signal");
 
-  if (c->signals_to_be_caught)
+  if (!c->signals_to_be_caught.empty ())
     {
-      int i;
-      gdb_signal_type iter;
-
-      for (i = 0;
-           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
-           i++)
+      for (gdb_signal iter : c->signals_to_be_caught)
 	fprintf_unfiltered (fp, " %s", signal_to_name_or_int (iter));
     }
   else if (c->catch_all)
@@ -355,8 +314,8 @@ signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
    then internal signals like SIGTRAP are not caught.  */
 
 static void
-create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
-			  int catch_all)
+create_signal_catchpoint (int tempflag, std::vector<gdb_signal> &&filter,
+			  bool catch_all)
 {
   struct signal_catchpoint *c;
   struct gdbarch *gdbarch = get_current_arch ();
@@ -373,57 +332,50 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
 /* Splits the argument using space as delimiter.  Returns an xmalloc'd
    filter list, or NULL if no filtering is required.  */
 
-static VEC (gdb_signal_type) *
-catch_signal_split_args (char *arg, int *catch_all)
+static std::vector<gdb_signal>
+catch_signal_split_args (char *arg, bool *catch_all)
 {
-  VEC (gdb_signal_type) *result = NULL;
-  struct cleanup *cleanup = make_cleanup (VEC_cleanup (gdb_signal_type),
-					  &result);
+  std::vector<gdb_signal> result;
   int first = 1;
 
   while (*arg != '\0')
     {
       int num;
-      gdb_signal_type signal_number;
-      char *one_arg, *endptr;
-      struct cleanup *inner_cleanup;
+      gdb_signal signal_number;
+      char *endptr;
 
-      one_arg = extract_arg (&arg);
+      gdb::unique_xmalloc_ptr<char> one_arg (extract_arg (&arg));
       if (one_arg == NULL)
 	break;
-      inner_cleanup = make_cleanup (xfree, one_arg);
 
       /* Check for the special flag "all".  */
-      if (strcmp (one_arg, "all") == 0)
+      if (strcmp (one_arg.get (), "all") == 0)
 	{
 	  arg = skip_spaces (arg);
 	  if (*arg != '\0' || !first)
 	    error (_("'all' cannot be caught with other signals"));
-	  *catch_all = 1;
-	  gdb_assert (result == NULL);
-	  do_cleanups (inner_cleanup);
-	  discard_cleanups (cleanup);
-	  return NULL;
+	  *catch_all = true;
+	  gdb_assert (result.empty ());
+	  return result;
 	}
 
       first = 0;
 
       /* Check if the user provided a signal name or a number.  */
-      num = (int) strtol (one_arg, &endptr, 0);
+      num = (int) strtol (one_arg.get (), &endptr, 0);
       if (*endptr == '\0')
 	signal_number = gdb_signal_from_command (num);
       else
 	{
-	  signal_number = gdb_signal_from_name (one_arg);
+	  signal_number = gdb_signal_from_name (one_arg.get ());
 	  if (signal_number == GDB_SIGNAL_UNKNOWN)
-	    error (_("Unknown signal name '%s'."), one_arg);
+	    error (_("Unknown signal name '%s'."), one_arg.get ());
 	}
 
-      VEC_safe_push (gdb_signal_type, result, signal_number);
-      do_cleanups (inner_cleanup);
+      result.push_back (signal_number);
     }
 
-  discard_cleanups (cleanup);
+  result.shrink_to_fit ();
   return result;
 }
 
@@ -433,8 +385,9 @@ static void
 catch_signal_command (char *arg, int from_tty,
 		      struct cmd_list_element *command)
 {
-  int tempflag, catch_all = 0;
-  VEC (gdb_signal_type) *filter;
+  int tempflag;
+  bool catch_all = false;
+  std::vector<gdb_signal> filter;
 
   tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
 
@@ -448,10 +401,8 @@ catch_signal_command (char *arg, int from_tty,
 
   if (arg != NULL)
     filter = catch_signal_split_args (arg, &catch_all);
-  else
-    filter = NULL;
 
-  create_signal_catchpoint (tempflag, filter, catch_all);
+  create_signal_catchpoint (tempflag, std::move (filter), catch_all);
 }
 
 static void
-- 
2.9.3

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

[Sergio Durigan Junior]

On Wednesday, July 19 2017, Tom Tromey wrote:

> This changes signal_catchpoint to be more of a C++ class, using
> std::vector and updating the users.

Thanks for the patch, Tom.

> 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.
> 	<catch_all>: Now a bool.
> 	(~signal_catchpoint): Remove.
> 	(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" and
> 	"catch_all".
> 	(catch_signal_split_args): Return a std::vector.  Change type of
> 	"catch_all".
> 	(catch_signal_command): Update.
> ---
>  gdb/ChangeLog         |  19 +++++++
>  gdb/break-catch-sig.c | 143 +++++++++++++++++---------------------------------
>  2 files changed, 66 insertions(+), 96 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index bc6e55b..42bde4b 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,22 @@
> +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.
> +	<catch_all>: Now a bool.
> +	(~signal_catchpoint): Remove.
> +	(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" and
> +	"catch_all".
> +	(catch_signal_split_args): Return a std::vector.  Change type of
> +	"catch_all".
> +	(catch_signal_command): Update.
> +
>  2017-07-18  David Blaikie  <dblaikie@gmail.com>
>  
>  	* dwarf2read.c (create_cus_hash_table): Re-add lost initialization
> diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
> index 3eede93..afac016 100644
> --- a/gdb/break-catch-sig.c
> +++ b/gdb/break-catch-sig.c
> @@ -33,30 +33,24 @@
>  
>  #define INTERNAL_SIGNAL(x) ((x) == GDB_SIGNAL_TRAP || (x) == GDB_SIGNAL_INT)
>  
> -typedef enum gdb_signal gdb_signal_type;
> -
> -DEF_VEC_I (gdb_signal_type);
> -
>  /* An instance of this type is used to represent a signal catchpoint.
>     A breakpoint is really of this type iff its ops pointer points to
>     SIGNAL_CATCHPOINT_OPS.  */
>  
>  struct signal_catchpoint : public breakpoint
>  {
> -  ~signal_catchpoint () override;
> -
>    /* Signal numbers used for the 'catch signal' feature.  If no signal
> -     has been specified for filtering, its value is NULL.  Otherwise,
> +     has been specified for filtering, it is empty.  Otherwise,
>       it holds a list of all signals to be caught.  */
>  
> -  VEC (gdb_signal_type) *signals_to_be_caught;
> +  std::vector<gdb_signal> signals_to_be_caught;
>  
> -  /* If SIGNALS_TO_BE_CAUGHT is NULL, then all "ordinary" signals are
> +  /* If SIGNALS_TO_BE_CAUGHT is empty, then all "ordinary" signals are
>       caught.  If CATCH_ALL is non-zero, then internal signals are

"If CATCH_ALL is true"

> -     caught as well.  If SIGNALS_TO_BE_CAUGHT is non-NULL, then this
> +     caught as well.  If SIGNALS_TO_BE_CAUGHT is not empty, then this
>       field is ignored.  */
>  
> -  int catch_all;
> +  bool catch_all;
>  };
>  
>  /* The breakpoint_ops structure to be used in signal catchpoints.  */
> @@ -85,13 +79,6 @@ signal_to_name_or_int (enum gdb_signal sig)
>  
>  \f
>  
> -/* signal_catchpoint destructor.  */
> -
> -signal_catchpoint::~signal_catchpoint ()
> -{
> -  VEC_free (gdb_signal_type, this->signals_to_be_caught);
> -}
> -
>  /* Implement the "insert_location" breakpoint_ops method for signal
>     catchpoints.  */
>  
> @@ -99,20 +86,15 @@ 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;
> -
> -      for (i = 0;
> -	   VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -	   i++)
> +      for (gdb_signal iter : c->signals_to_be_caught)
>  	++signal_catch_counts[iter];
>      }
>    else
>      {
> -      for (i = 0; i < GDB_SIGNAL_LAST; ++i)
> +      for (int i = 0; i < GDB_SIGNAL_LAST; ++i)
>  	{
>  	  if (c->catch_all || !INTERNAL_SIGNAL (i))
>  	    ++signal_catch_counts[i];
> @@ -132,15 +114,10 @@ signal_catchpoint_remove_location (struct bp_location *bl,
>  				   enum remove_bp_reason reason)
>  {
>    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;
> -
> -      for (i = 0;
> -	   VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -	   i++)
> +      for (gdb_signal iter : c->signals_to_be_caught)
>  	{
>  	  gdb_assert (signal_catch_counts[iter] > 0);
>  	  --signal_catch_counts[iter];
> @@ -148,7 +125,7 @@ signal_catchpoint_remove_location (struct bp_location *bl,
>      }
>    else
>      {
> -      for (i = 0; i < GDB_SIGNAL_LAST; ++i)
> +      for (int i = 0; i < GDB_SIGNAL_LAST; ++i)
>  	{
>  	  if (c->catch_all || !INTERNAL_SIGNAL (i))
>  	    {
> @@ -174,7 +151,7 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl,
>  {
>    const struct signal_catchpoint *c
>      = (const struct signal_catchpoint *) bl->owner;
> -  gdb_signal_type signal_number;
> +  gdb_signal signal_number;
>  
>    if (ws->kind != TARGET_WAITKIND_STOPPED)
>      return 0;
> @@ -184,18 +161,12 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl,
>    /* If we are catching specific signals in this breakpoint, then we
>       must guarantee that the called signal is the same signal we are
>       catching.  */
> -  if (c->signals_to_be_caught)
> +  if (!c->signals_to_be_caught.empty ())
>      {
> -      int i;
> -      gdb_signal_type iter;
> -
> -      for (i = 0;
> -           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -           i++)
> +      for (gdb_signal iter : c->signals_to_be_caught)
>  	if (signal_number == iter)
>  	  return 1;
>        /* Not the same.  */
> -      gdb_assert (!iter);

I don't really understand what this assert was checking.  In what
situation would iter == 0?

>        return 0;
>      }
>    else
> @@ -246,26 +217,24 @@ 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 ())

Should be:

  if (c->signals_to_be_caught.size () > 1)

>      uiout->text ("signals \"");
>    else
>      uiout->text ("signal \"");
>  
> -  if (c->signals_to_be_caught)
> +  if (c->signals_to_be_caught.size () > 1)

And here:

  if (!c->signals_to_be_caught.empty ())

>      {
> -      int i;
> -      gdb_signal_type iter;
>        std::string text;
>  
> -      for (i = 0;
> -           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -           i++)
> +      bool first = true;
> +      for (gdb_signal iter : c->signals_to_be_caught)
>          {
>  	  const char *name = signal_to_name_or_int (iter);
>  
> -	  if (i > 0)
> +	  if (!first)
>  	    text += " ";
> +	  first = false;
> +
>  	  text += name;
>          }
>        uiout->field_string ("what", text.c_str ());
> @@ -287,19 +256,14 @@ signal_catchpoint_print_mention (struct breakpoint *b)
>  {
>    struct signal_catchpoint *c = (struct signal_catchpoint *) b;
>  
> -  if (c->signals_to_be_caught)
> +  if (!c->signals_to_be_caught.empty ())
>      {
> -      int i;
> -      gdb_signal_type iter;
> -
> -      if (VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1)
> +      if (c->signals_to_be_caught.size () > 1)
>          printf_filtered (_("Catchpoint %d (signals"), b->number);
>        else
>          printf_filtered (_("Catchpoint %d (signal"), b->number);
>  
> -      for (i = 0;
> -           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -           i++)
> +      for (gdb_signal iter : c->signals_to_be_caught)
>          {
>  	  const char *name = signal_to_name_or_int (iter);
>  
> @@ -323,14 +287,9 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp)
>  
>    fprintf_unfiltered (fp, "catch signal");
>  
> -  if (c->signals_to_be_caught)
> +  if (!c->signals_to_be_caught.empty ())
>      {
> -      int i;
> -      gdb_signal_type iter;
> -
> -      for (i = 0;
> -           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> -           i++)
> +      for (gdb_signal iter : c->signals_to_be_caught)
>  	fprintf_unfiltered (fp, " %s", signal_to_name_or_int (iter));
>      }
>    else if (c->catch_all)
> @@ -355,8 +314,8 @@ signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
>     then internal signals like SIGTRAP are not caught.  */
>  
>  static void
> -create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
> -			  int catch_all)
> +create_signal_catchpoint (int tempflag, std::vector<gdb_signal> &&filter,
> +			  bool catch_all)
>  {
>    struct signal_catchpoint *c;
>    struct gdbarch *gdbarch = get_current_arch ();
> @@ -373,57 +332,50 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
>  /* Splits the argument using space as delimiter.  Returns an xmalloc'd
>     filter list, or NULL if no filtering is required.  */
>  
> -static VEC (gdb_signal_type) *
> -catch_signal_split_args (char *arg, int *catch_all)
> +static std::vector<gdb_signal>
> +catch_signal_split_args (char *arg, bool *catch_all)
>  {
> -  VEC (gdb_signal_type) *result = NULL;
> -  struct cleanup *cleanup = make_cleanup (VEC_cleanup (gdb_signal_type),
> -					  &result);
> +  std::vector<gdb_signal> result;
>    int first = 1;

Nit: "first" can be a bool.

>  
>    while (*arg != '\0')
>      {
>        int num;
> -      gdb_signal_type signal_number;
> -      char *one_arg, *endptr;
> -      struct cleanup *inner_cleanup;
> +      gdb_signal signal_number;
> +      char *endptr;
>  
> -      one_arg = extract_arg (&arg);
> +      gdb::unique_xmalloc_ptr<char> one_arg (extract_arg (&arg));
>        if (one_arg == NULL)
>  	break;
> -      inner_cleanup = make_cleanup (xfree, one_arg);
>  
>        /* Check for the special flag "all".  */
> -      if (strcmp (one_arg, "all") == 0)
> +      if (strcmp (one_arg.get (), "all") == 0)
>  	{
>  	  arg = skip_spaces (arg);
>  	  if (*arg != '\0' || !first)
>  	    error (_("'all' cannot be caught with other signals"));
> -	  *catch_all = 1;
> -	  gdb_assert (result == NULL);
> -	  do_cleanups (inner_cleanup);
> -	  discard_cleanups (cleanup);
> -	  return NULL;
> +	  *catch_all = true;
> +	  gdb_assert (result.empty ());
> +	  return result;
>  	}
>  
>        first = 0;
>  
>        /* Check if the user provided a signal name or a number.  */
> -      num = (int) strtol (one_arg, &endptr, 0);
> +      num = (int) strtol (one_arg.get (), &endptr, 0);
>        if (*endptr == '\0')
>  	signal_number = gdb_signal_from_command (num);
>        else
>  	{
> -	  signal_number = gdb_signal_from_name (one_arg);
> +	  signal_number = gdb_signal_from_name (one_arg.get ());
>  	  if (signal_number == GDB_SIGNAL_UNKNOWN)
> -	    error (_("Unknown signal name '%s'."), one_arg);
> +	    error (_("Unknown signal name '%s'."), one_arg.get ());
>  	}
>  
> -      VEC_safe_push (gdb_signal_type, result, signal_number);
> -      do_cleanups (inner_cleanup);
> +      result.push_back (signal_number);
>      }
>  
> -  discard_cleanups (cleanup);
> +  result.shrink_to_fit ();

Do you really need this call here?  You're using safe_push above, so I
don't think the vector will end up with a bigger capacity than its size.

>    return result;
>  }
>  
> @@ -433,8 +385,9 @@ static void
>  catch_signal_command (char *arg, int from_tty,
>  		      struct cmd_list_element *command)
>  {
> -  int tempflag, catch_all = 0;
> -  VEC (gdb_signal_type) *filter;
> +  int tempflag;
> +  bool catch_all = false;
> +  std::vector<gdb_signal> filter;
>  
>    tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
>  
> @@ -448,10 +401,8 @@ catch_signal_command (char *arg, int from_tty,
>  
>    if (arg != NULL)
>      filter = catch_signal_split_args (arg, &catch_all);
> -  else
> -    filter = NULL;
>  
> -  create_signal_catchpoint (tempflag, filter, catch_all);
> +  create_signal_catchpoint (tempflag, std::move (filter), catch_all);
>  }
>  
>  static void
> -- 
> 2.9.3

Otherwise, 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/

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

[Simon Marchi]

On 2017-07-20 20:13, Sergio Durigan Junior wrote:
>> 
>>    while (*arg != '\0')
>>      {
>>        int num;
>> -      gdb_signal_type signal_number;
>> -      char *one_arg, *endptr;
>> -      struct cleanup *inner_cleanup;
>> +      gdb_signal signal_number;
>> +      char *endptr;
>> 
>> -      one_arg = extract_arg (&arg);
>> +      gdb::unique_xmalloc_ptr<char> one_arg (extract_arg (&arg));
>>        if (one_arg == NULL)
>>  	break;
>> -      inner_cleanup = make_cleanup (xfree, one_arg);
>> 
>>        /* Check for the special flag "all".  */
>> -      if (strcmp (one_arg, "all") == 0)
>> +      if (strcmp (one_arg.get (), "all") == 0)
>>  	{
>>  	  arg = skip_spaces (arg);
>>  	  if (*arg != '\0' || !first)
>>  	    error (_("'all' cannot be caught with other signals"));
>> -	  *catch_all = 1;
>> -	  gdb_assert (result == NULL);
>> -	  do_cleanups (inner_cleanup);
>> -	  discard_cleanups (cleanup);
>> -	  return NULL;
>> +	  *catch_all = true;
>> +	  gdb_assert (result.empty ());
>> +	  return result;
>>  	}
>> 
>>        first = 0;
>> 
>>        /* Check if the user provided a signal name or a number.  */
>> -      num = (int) strtol (one_arg, &endptr, 0);
>> +      num = (int) strtol (one_arg.get (), &endptr, 0);
>>        if (*endptr == '\0')
>>  	signal_number = gdb_signal_from_command (num);
>>        else
>>  	{
>> -	  signal_number = gdb_signal_from_name (one_arg);
>> +	  signal_number = gdb_signal_from_name (one_arg.get ());
>>  	  if (signal_number == GDB_SIGNAL_UNKNOWN)
>> -	    error (_("Unknown signal name '%s'."), one_arg);
>> +	    error (_("Unknown signal name '%s'."), one_arg.get ());
>>  	}
>> 
>> -      VEC_safe_push (gdb_signal_type, result, signal_number);
>> -      do_cleanups (inner_cleanup);
>> +      result.push_back (signal_number);
>>      }
>> 
>> -  discard_cleanups (cleanup);
>> +  result.shrink_to_fit ();
> 
> Do you really need this call here?  You're using safe_push above, so I
> don't think the vector will end up with a bigger capacity than its 
> size.

safe_push is the old implementations, std::vector grows the capacity 
exponentially, so you're likely to be left with unused capacity.

Simon

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

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

>> -  if (c->signals_to_be_caught)
>> +  if (!c->signals_to_be_caught.empty ())
>> {
>> -      int i;
>> -      gdb_signal_type iter;
>> -
>> -      for (i = 0;
>> -           VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
>> -           i++)
>> +      for (gdb_signal iter : c->signals_to_be_caught)
>> if (signal_number == iter)
>> return 1;
>> /* Not the same.  */
>> -      gdb_assert (!iter);

Sergio> I don't really understand what this assert was checking.  In what
Sergio> situation would iter == 0?

Yeah, I don't know either.

Tom

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

[Simon Marchi]

Hi Tom,

Two nits, in addition to what Sergio already pointed out.

> @@ -355,8 +314,8 @@ signal_catchpoint_explains_signal (struct
> breakpoint *b, enum gdb_signal sig)
>     then internal signals like SIGTRAP are not caught.  */
> 
>  static void
> -create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
> -			  int catch_all)
> +create_signal_catchpoint (int tempflag, std::vector<gdb_signal> 
> &&filter,
> +			  bool catch_all)

The documentation of this function would need updating.

>  {
>    struct signal_catchpoint *c;
>    struct gdbarch *gdbarch = get_current_arch ();
> @@ -373,57 +332,50 @@ create_signal_catchpoint (int tempflag, VEC
> (gdb_signal_type) *filter,
>  /* Splits the argument using space as delimiter.  Returns an xmalloc'd
>     filter list, or NULL if no filtering is required.  */
> 
> -static VEC (gdb_signal_type) *
> -catch_signal_split_args (char *arg, int *catch_all)
> +static std::vector<gdb_signal>
> +catch_signal_split_args (char *arg, bool *catch_all)

The documentation of this function too.

Thanks,

Simon

[RFA v2 2/4] C++-ify break-catch-throw

[Tom Tromey]

This changes exception_catchpoint's "exception_rx' member to be a
std::string, and updating the users.

2017-06-04  Tom Tromey  <tom@tromey.com>

	* break-catch-throw.c (struct exception_catchpoint)
	<exception_rx>: Now a std::string.
	(~exception_catchpoint): REmove.
	(print_one_detail_exception_catchpoint): Update.
	(handle_gnu_v3_exceptions): Change type of except_rx.
	(extract_exception_regexp): Return a std::string.
	(catch_exception_command_1): Update.
---
 gdb/ChangeLog           | 10 ++++++++++
 gdb/break-catch-throw.c | 40 +++++++++++++---------------------------
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 42bde4b..e178fa5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2017-06-04  Tom Tromey  <tom@tromey.com>
 
+	* break-catch-throw.c (struct exception_catchpoint)
+	<exception_rx>: Now a std::string.
+	(~exception_catchpoint): REmove.
+	(print_one_detail_exception_catchpoint): Update.
+	(handle_gnu_v3_exceptions): Change type of except_rx.
+	(extract_exception_regexp): Return a std::string.
+	(catch_exception_command_1): Update.
+
+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.
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 0074d06..e71a885 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -76,16 +76,14 @@ static struct breakpoint_ops gnu_v3_exception_catchpoint_ops;
 
 struct exception_catchpoint : public breakpoint
 {
-  ~exception_catchpoint () override;
-
   /* The kind of exception catchpoint.  */
 
   enum exception_event_kind kind;
 
-  /* If non-NULL, an xmalloc'd string holding the source form of the
-     regular expression to match against.  */
+  /* If not empty, a string holding the source form of the regular
+     expression to match against.  */
 
-  char *exception_rx;
+  std::string exception_rx;
 
   /* If non-NULL, a compiled regular expression which is used to
      determine which exceptions to stop on.  */
@@ -140,13 +138,6 @@ classify_exception_breakpoint (struct breakpoint *b)
   return cp->kind;
 }
 
-/* exception_catchpoint destructor.  */
-
-exception_catchpoint::~exception_catchpoint ()
-{
-  xfree (this->exception_rx);
-}
-
 /* Implement the 'check_status' method.  */
 
 static void
@@ -319,10 +310,10 @@ print_one_detail_exception_catchpoint (const struct breakpoint *b,
   const struct exception_catchpoint *cp
     = (const struct exception_catchpoint *) b;
 
-  if (cp->exception_rx != NULL)
+  if (!cp->exception_rx.empty ())
     {
       uiout->text (_("\tmatching: "));
-      uiout->field_string ("regexp", cp->exception_rx);
+      uiout->field_string ("regexp", cp->exception_rx.c_str ());
       uiout->text ("\n");
     }
 }
@@ -371,15 +362,15 @@ print_recreate_exception_catchpoint (struct breakpoint *b,
 }
 
 static void
-handle_gnu_v3_exceptions (int tempflag, char *except_rx,
+handle_gnu_v3_exceptions (int tempflag, std::string &&except_rx,
 			  const char *cond_string,
 			  enum exception_event_kind ex_event, int from_tty)
 {
   std::unique_ptr<compiled_regex> pattern;
 
-  if (except_rx != NULL)
+  if (!except_rx.empty ())
     {
-      pattern.reset (new compiled_regex (except_rx, REG_NOSUB,
+      pattern.reset (new compiled_regex (except_rx.c_str (), REG_NOSUB,
 					 _("invalid type-matching regexp")));
     }
 
@@ -410,7 +401,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx,
    STRING is updated to point to the "if" token, if it exists, or to
    the end of the string.  */
 
-static char *
+static std::string
 extract_exception_regexp (const char **string)
 {
   const char *start;
@@ -435,8 +426,8 @@ extract_exception_regexp (const char **string)
 
   *string = last;
   if (last_space > start)
-    return savestring (start, last_space - start);
-  return NULL;
+    return std::string (start, last_space - start);
+  return std::string ();
 }
 
 /* Deal with "catch catch", "catch throw", and "catch rethrow"
@@ -447,17 +438,14 @@ catch_exception_command_1 (enum exception_event_kind ex_event,
 			   char *arg_entry,
 			   int tempflag, int from_tty)
 {
-  char *except_rx;
   const char *cond_string = NULL;
-  struct cleanup *cleanup;
   const char *arg = arg_entry;
 
   if (!arg)
     arg = "";
   arg = skip_spaces_const (arg);
 
-  except_rx = extract_exception_regexp (&arg);
-  cleanup = make_cleanup (xfree, except_rx);
+  std::string except_rx = extract_exception_regexp (&arg);
 
   cond_string = ep_parse_optional_if_clause (&arg);
 
@@ -469,10 +457,8 @@ catch_exception_command_1 (enum exception_event_kind ex_event,
       && ex_event != EX_EVENT_RETHROW)
     error (_("Unsupported or unknown exception event; cannot catch it"));
 
-  handle_gnu_v3_exceptions (tempflag, except_rx, cond_string,
+  handle_gnu_v3_exceptions (tempflag, std::move (except_rx), cond_string,
 			    ex_event, from_tty);
-
-  discard_cleanups (cleanup);
 }
 
 /* Implementation of "catch catch" command.  */
-- 
2.9.3

Re: [RFA v2 2/4] C++-ify break-catch-throw

[Sergio Durigan Junior]

On Wednesday, July 19 2017, Tom Tromey wrote:

> This changes exception_catchpoint's "exception_rx' member to be a
> std::string, and updating the users.
>
> 2017-06-04  Tom Tromey  <tom@tromey.com>
>
> 	* break-catch-throw.c (struct exception_catchpoint)
> 	<exception_rx>: Now a std::string.
> 	(~exception_catchpoint): REmove.
> 	(print_one_detail_exception_catchpoint): Update.
> 	(handle_gnu_v3_exceptions): Change type of except_rx.
> 	(extract_exception_regexp): Return a std::string.
> 	(catch_exception_command_1): Update.
> ---
>  gdb/ChangeLog           | 10 ++++++++++
>  gdb/break-catch-throw.c | 40 +++++++++++++---------------------------
>  2 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 42bde4b..e178fa5 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,15 @@
>  2017-06-04  Tom Tromey  <tom@tromey.com>
>  
> +	* break-catch-throw.c (struct exception_catchpoint)
> +	<exception_rx>: Now a std::string.
> +	(~exception_catchpoint): REmove.
> +	(print_one_detail_exception_catchpoint): Update.
> +	(handle_gnu_v3_exceptions): Change type of except_rx.
> +	(extract_exception_regexp): Return a std::string.
> +	(catch_exception_command_1): Update.
> +
> +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.
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index 0074d06..e71a885 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -76,16 +76,14 @@ static struct breakpoint_ops gnu_v3_exception_catchpoint_ops;
>  
>  struct exception_catchpoint : public breakpoint
>  {
> -  ~exception_catchpoint () override;
> -
>    /* The kind of exception catchpoint.  */
>  
>    enum exception_event_kind kind;
>  
> -  /* If non-NULL, an xmalloc'd string holding the source form of the
> -     regular expression to match against.  */
> +  /* If not empty, a string holding the source form of the regular
> +     expression to match against.  */
>  
> -  char *exception_rx;
> +  std::string exception_rx;
>  
>    /* If non-NULL, a compiled regular expression which is used to
>       determine which exceptions to stop on.  */
> @@ -140,13 +138,6 @@ classify_exception_breakpoint (struct breakpoint *b)
>    return cp->kind;
>  }
>  
> -/* exception_catchpoint destructor.  */
> -
> -exception_catchpoint::~exception_catchpoint ()
> -{
> -  xfree (this->exception_rx);
> -}
> -
>  /* Implement the 'check_status' method.  */
>  
>  static void
> @@ -319,10 +310,10 @@ print_one_detail_exception_catchpoint (const struct breakpoint *b,
>    const struct exception_catchpoint *cp
>      = (const struct exception_catchpoint *) b;
>  
> -  if (cp->exception_rx != NULL)
> +  if (!cp->exception_rx.empty ())
>      {
>        uiout->text (_("\tmatching: "));
> -      uiout->field_string ("regexp", cp->exception_rx);
> +      uiout->field_string ("regexp", cp->exception_rx.c_str ());
>        uiout->text ("\n");
>      }
>  }
> @@ -371,15 +362,15 @@ print_recreate_exception_catchpoint (struct breakpoint *b,
>  }
>  
>  static void
> -handle_gnu_v3_exceptions (int tempflag, char *except_rx,
> +handle_gnu_v3_exceptions (int tempflag, std::string &&except_rx,
>  			  const char *cond_string,
>  			  enum exception_event_kind ex_event, int from_tty)
>  {
>    std::unique_ptr<compiled_regex> pattern;
>  
> -  if (except_rx != NULL)
> +  if (!except_rx.empty ())
>      {
> -      pattern.reset (new compiled_regex (except_rx, REG_NOSUB,
> +      pattern.reset (new compiled_regex (except_rx.c_str (), REG_NOSUB,
>  					 _("invalid type-matching regexp")));
>      }
>  
> @@ -410,7 +401,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx,
>     STRING is updated to point to the "if" token, if it exists, or to
>     the end of the string.  */
>  
> -static char *
> +static std::string
>  extract_exception_regexp (const char **string)
>  {
>    const char *start;
> @@ -435,8 +426,8 @@ extract_exception_regexp (const char **string)
>  
>    *string = last;
>    if (last_space > start)
> -    return savestring (start, last_space - start);
> -  return NULL;
> +    return std::string (start, last_space - start);
> +  return std::string ();
>  }
>  
>  /* Deal with "catch catch", "catch throw", and "catch rethrow"
> @@ -447,17 +438,14 @@ catch_exception_command_1 (enum exception_event_kind ex_event,
>  			   char *arg_entry,
>  			   int tempflag, int from_tty)
>  {
> -  char *except_rx;
>    const char *cond_string = NULL;
> -  struct cleanup *cleanup;
>    const char *arg = arg_entry;
>  
>    if (!arg)
>      arg = "";
>    arg = skip_spaces_const (arg);
>  
> -  except_rx = extract_exception_regexp (&arg);
> -  cleanup = make_cleanup (xfree, except_rx);
> +  std::string except_rx = extract_exception_regexp (&arg);
>  
>    cond_string = ep_parse_optional_if_clause (&arg);
>  
> @@ -469,10 +457,8 @@ catch_exception_command_1 (enum exception_event_kind ex_event,
>        && ex_event != EX_EVENT_RETHROW)
>      error (_("Unsupported or unknown exception event; cannot catch it"));
>  
> -  handle_gnu_v3_exceptions (tempflag, except_rx, cond_string,
> +  handle_gnu_v3_exceptions (tempflag, std::move (except_rx), cond_string,
>  			    ex_event, from_tty);
> -
> -  discard_cleanups (cleanup);
>  }
>  
>  /* Implementation of "catch catch" command.  */
> -- 
> 2.9.3

LGTM.

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

Re: [RFA v2 2/4] C++-ify break-catch-throw

[Simon Marchi]

On 2017-07-19 22:32, Tom Tromey wrote:
> This changes exception_catchpoint's "exception_rx' member to be a
> std::string, and updating the users.
> 
> 2017-06-04  Tom Tromey  <tom@tromey.com>
> 
> 	* break-catch-throw.c (struct exception_catchpoint)
> 	<exception_rx>: Now a std::string.
> 	(~exception_catchpoint): REmove.

"REmove"

Otherwise, LGTM too.

Simon

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

[Tom Tromey]

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

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

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

[Sergio Durigan Junior]

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/

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

[Simon Marchi]

Hi Tom,

Just a nit in addition to what Sergio pointed out, otherwise LGTM too.

On 2017-07-19 22:32, Tom Tromey wrote:
> @@ -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)

Could you add a != NULL while at it?

Thanks,

Simon

[RFA v2 4/4] Use std::vector in struct catch_syscall_inferior_data

[Tom Tromey]

This changes struct catch_syscall_inferior_data to use a std::vector
rather than a VEC.  It also changes it to be allocated with new and
destroyed with delete.

2017-07-18  Tom Tromey  <tom@tromey.com>

	* break-catch-syscall.c (struct catch_syscall_inferior_data)
	<syscalls_counts>: Now a std::vector.
	(get_catch_syscall_inferior_data): Use "new".
	(catch_syscall_inferior_data_cleanup): Use "delete".
	(insert_catch_syscall, remove_catch_syscall)
	(clear_syscall_counts): Update.
---
 gdb/ChangeLog             |  9 +++++++++
 gdb/break-catch-syscall.c | 47 ++++++++++++++++-------------------------------
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fb953ed..4372853 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2017-07-18  Tom Tromey  <tom@tromey.com>
+
+	* break-catch-syscall.c (struct catch_syscall_inferior_data)
+	<syscalls_counts>: Now a std::vector.
+	(get_catch_syscall_inferior_data): Use "new".
+	(catch_syscall_inferior_data_cleanup): Use "delete".
+	(insert_catch_syscall, remove_catch_syscall)
+	(clear_syscall_counts): Update.
+
 2017-07-17  Tom Tromey  <tom@tromey.com>
 
 	* break-catch-syscall.c (syscall_catchpoint)
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index ff0cb69..3df358b 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -54,14 +54,14 @@ struct catch_syscall_inferior_data
   int any_syscall_count;
 
   /* Count of each system call.  */
-  VEC(int) *syscalls_counts;
+  std::vector<int> syscalls_counts;
 
   /* This counts all syscall catch requests, so we can readily determine
      if any catching is necessary.  */
   int total_syscalls_count;
 };
 
-static struct catch_syscall_inferior_data*
+static struct catch_syscall_inferior_data *
 get_catch_syscall_inferior_data (struct inferior *inf)
 {
   struct catch_syscall_inferior_data *inf_data;
@@ -70,7 +70,7 @@ get_catch_syscall_inferior_data (struct inferior *inf)
 	      inferior_data (inf, catch_syscall_inferior_data));
   if (inf_data == NULL)
     {
-      inf_data = XCNEW (struct catch_syscall_inferior_data);
+      inf_data = new struct catch_syscall_inferior_data ();
       set_inferior_data (inf, catch_syscall_inferior_data, inf_data);
     }
 
@@ -80,7 +80,9 @@ get_catch_syscall_inferior_data (struct inferior *inf)
 static void
 catch_syscall_inferior_data_cleanup (struct inferior *inf, void *arg)
 {
-  xfree (arg);
+  struct catch_syscall_inferior_data *inf_data
+    = (struct catch_syscall_inferior_data *) arg;
+  delete inf_data;
 }
 
 
@@ -104,31 +106,17 @@ insert_catch_syscall (struct bp_location *bl)
 	{
           int elem;
 
-	  if (iter >= VEC_length (int, inf_data->syscalls_counts))
-	    {
-              int old_size = VEC_length (int, inf_data->syscalls_counts);
-              uintptr_t vec_addr_offset
-		= old_size * ((uintptr_t) sizeof (int));
-              uintptr_t vec_addr;
-              VEC_safe_grow (int, inf_data->syscalls_counts, iter + 1);
-              vec_addr = ((uintptr_t) VEC_address (int,
-						  inf_data->syscalls_counts)
-			  + vec_addr_offset);
-              memset ((void *) vec_addr, 0,
-                      (iter + 1 - old_size) * sizeof (int));
-	    }
-          elem = VEC_index (int, inf_data->syscalls_counts, iter);
-          VEC_replace (int, inf_data->syscalls_counts, iter, ++elem);
+	  if (iter >= inf_data->syscalls_counts.size ())
+	    inf_data->syscalls_counts.resize (iter + 1);
+	  ++inf_data->syscalls_counts[iter];
 	}
     }
 
   return target_set_syscall_catchpoint (ptid_get_pid (inferior_ptid),
 					inf_data->total_syscalls_count != 0,
 					inf_data->any_syscall_count,
-					VEC_length (int,
-						    inf_data->syscalls_counts),
-					VEC_address (int,
-						     inf_data->syscalls_counts));
+					inf_data->syscalls_counts.size (),
+					inf_data->syscalls_counts.data ());
 }
 
 /* Implement the "remove" breakpoint_ops method for syscall
@@ -150,21 +138,18 @@ remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason)
       for (int iter : c->syscalls_to_be_caught)
 	{
           int elem;
-	  if (iter >= VEC_length (int, inf_data->syscalls_counts))
+	  if (iter >= inf_data->syscalls_counts.size ())
 	    /* Shouldn't happen.  */
 	    continue;
-          elem = VEC_index (int, inf_data->syscalls_counts, iter);
-          VEC_replace (int, inf_data->syscalls_counts, iter, --elem);
+          --inf_data->syscalls_counts[iter];
         }
     }
 
   return target_set_syscall_catchpoint (ptid_get_pid (inferior_ptid),
 					inf_data->total_syscalls_count != 0,
 					inf_data->any_syscall_count,
-					VEC_length (int,
-						    inf_data->syscalls_counts),
-					VEC_address (int,
-						     inf_data->syscalls_counts));
+					inf_data->syscalls_counts.size (),
+					inf_data->syscalls_counts.data ());
 }
 
 /* Implement the "breakpoint_hit" breakpoint_ops method for syscall
@@ -628,7 +613,7 @@ clear_syscall_counts (struct inferior *inf)
 
   inf_data->total_syscalls_count = 0;
   inf_data->any_syscall_count = 0;
-  VEC_free (int, inf_data->syscalls_counts);
+  inf_data->syscalls_counts.clear ();
 }
 
 static void
-- 
2.9.3

Re: [RFA v2 4/4] Use std::vector in struct catch_syscall_inferior_data

[Sergio Durigan Junior]

On Wednesday, July 19 2017, Tom Tromey wrote:

> This changes struct catch_syscall_inferior_data to use a std::vector
> rather than a VEC.  It also changes it to be allocated with new and
> destroyed with delete.

Thanks for the patch, Tom :-).

> 2017-07-18  Tom Tromey  <tom@tromey.com>
>
> 	* break-catch-syscall.c (struct catch_syscall_inferior_data)
> 	<syscalls_counts>: Now a std::vector.
> 	(get_catch_syscall_inferior_data): Use "new".
> 	(catch_syscall_inferior_data_cleanup): Use "delete".
> 	(insert_catch_syscall, remove_catch_syscall)
> 	(clear_syscall_counts): Update.
> ---
>  gdb/ChangeLog             |  9 +++++++++
>  gdb/break-catch-syscall.c | 47 ++++++++++++++++-------------------------------
>  2 files changed, 25 insertions(+), 31 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index fb953ed..4372853 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,12 @@
> +2017-07-18  Tom Tromey  <tom@tromey.com>
> +
> +	* break-catch-syscall.c (struct catch_syscall_inferior_data)
> +	<syscalls_counts>: Now a std::vector.
> +	(get_catch_syscall_inferior_data): Use "new".
> +	(catch_syscall_inferior_data_cleanup): Use "delete".
> +	(insert_catch_syscall, remove_catch_syscall)
> +	(clear_syscall_counts): Update.
> +
>  2017-07-17  Tom Tromey  <tom@tromey.com>
>  
>  	* break-catch-syscall.c (syscall_catchpoint)
> diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
> index ff0cb69..3df358b 100644
> --- a/gdb/break-catch-syscall.c
> +++ b/gdb/break-catch-syscall.c
> @@ -54,14 +54,14 @@ struct catch_syscall_inferior_data
>    int any_syscall_count;
>  
>    /* Count of each system call.  */
> -  VEC(int) *syscalls_counts;
> +  std::vector<int> syscalls_counts;
>  
>    /* This counts all syscall catch requests, so we can readily determine
>       if any catching is necessary.  */
>    int total_syscalls_count;
>  };
>  
> -static struct catch_syscall_inferior_data*
> +static struct catch_syscall_inferior_data *
>  get_catch_syscall_inferior_data (struct inferior *inf)
>  {
>    struct catch_syscall_inferior_data *inf_data;
> @@ -70,7 +70,7 @@ get_catch_syscall_inferior_data (struct inferior *inf)
>  	      inferior_data (inf, catch_syscall_inferior_data));
>    if (inf_data == NULL)
>      {
> -      inf_data = XCNEW (struct catch_syscall_inferior_data);
> +      inf_data = new struct catch_syscall_inferior_data ();
>        set_inferior_data (inf, catch_syscall_inferior_data, inf_data);
>      }
>  
> @@ -80,7 +80,9 @@ get_catch_syscall_inferior_data (struct inferior *inf)
>  static void
>  catch_syscall_inferior_data_cleanup (struct inferior *inf, void *arg)
>  {
> -  xfree (arg);
> +  struct catch_syscall_inferior_data *inf_data
> +    = (struct catch_syscall_inferior_data *) arg;
> +  delete inf_data;

Are we still using the "newline between variable declaration and code"
rule?

>  }
>  
>  
> @@ -104,31 +106,17 @@ insert_catch_syscall (struct bp_location *bl)
>  	{
>            int elem;
>  
> -	  if (iter >= VEC_length (int, inf_data->syscalls_counts))
> -	    {
> -              int old_size = VEC_length (int, inf_data->syscalls_counts);
> -              uintptr_t vec_addr_offset
> -		= old_size * ((uintptr_t) sizeof (int));
> -              uintptr_t vec_addr;
> -              VEC_safe_grow (int, inf_data->syscalls_counts, iter + 1);
> -              vec_addr = ((uintptr_t) VEC_address (int,
> -						  inf_data->syscalls_counts)
> -			  + vec_addr_offset);
> -              memset ((void *) vec_addr, 0,
> -                      (iter + 1 - old_size) * sizeof (int));
> -	    }
> -          elem = VEC_index (int, inf_data->syscalls_counts, iter);
> -          VEC_replace (int, inf_data->syscalls_counts, iter, ++elem);
> +	  if (iter >= inf_data->syscalls_counts.size ())
> +	    inf_data->syscalls_counts.resize (iter + 1);
> +	  ++inf_data->syscalls_counts[iter];

Great simplification.

>  	}
>      }
>  
>    return target_set_syscall_catchpoint (ptid_get_pid (inferior_ptid),
>  					inf_data->total_syscalls_count != 0,
>  					inf_data->any_syscall_count,
> -					VEC_length (int,
> -						    inf_data->syscalls_counts),
> -					VEC_address (int,
> -						     inf_data->syscalls_counts));
> +					inf_data->syscalls_counts.size (),
> +					inf_data->syscalls_counts.data ());
>  }
>  
>  /* Implement the "remove" breakpoint_ops method for syscall
> @@ -150,21 +138,18 @@ remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason)
>        for (int iter : c->syscalls_to_be_caught)
>  	{
>            int elem;
> -	  if (iter >= VEC_length (int, inf_data->syscalls_counts))
> +	  if (iter >= inf_data->syscalls_counts.size ())
>  	    /* Shouldn't happen.  */
>  	    continue;
> -          elem = VEC_index (int, inf_data->syscalls_counts, iter);
> -          VEC_replace (int, inf_data->syscalls_counts, iter, --elem);
> +          --inf_data->syscalls_counts[iter];
>          }
>      }
>  
>    return target_set_syscall_catchpoint (ptid_get_pid (inferior_ptid),
>  					inf_data->total_syscalls_count != 0,
>  					inf_data->any_syscall_count,
> -					VEC_length (int,
> -						    inf_data->syscalls_counts),
> -					VEC_address (int,
> -						     inf_data->syscalls_counts));
> +					inf_data->syscalls_counts.size (),
> +					inf_data->syscalls_counts.data ());
>  }
>  
>  /* Implement the "breakpoint_hit" breakpoint_ops method for syscall
> @@ -628,7 +613,7 @@ clear_syscall_counts (struct inferior *inf)
>  
>    inf_data->total_syscalls_count = 0;
>    inf_data->any_syscall_count = 0;
> -  VEC_free (int, inf_data->syscalls_counts);
> +  inf_data->syscalls_counts.clear ();
>  }
>  
>  static void
> -- 
> 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/

Re: [RFA v2 4/4] Use std::vector in struct catch_syscall_inferior_data

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> Are we still using the "newline between variable declaration and code"
Sergio> rule?

No idea.

Tom

Re: [RFA v2 4/4] Use std::vector in struct catch_syscall_inferior_data

[Simon Marchi]

On 2017-07-19 22:32, Tom Tromey wrote:
> @@ -104,31 +106,17 @@ insert_catch_syscall (struct bp_location *bl)
>  	{
>            int elem;
> 
> -	  if (iter >= VEC_length (int, inf_data->syscalls_counts))
> -	    {
> -              int old_size = VEC_length (int, 
> inf_data->syscalls_counts);
> -              uintptr_t vec_addr_offset
> -		= old_size * ((uintptr_t) sizeof (int));
> -              uintptr_t vec_addr;
> -              VEC_safe_grow (int, inf_data->syscalls_counts, iter + 
> 1);
> -              vec_addr = ((uintptr_t) VEC_address (int,
> -						  inf_data->syscalls_counts)
> -			  + vec_addr_offset);
> -              memset ((void *) vec_addr, 0,
> -                      (iter + 1 - old_size) * sizeof (int));
> -	    }
> -          elem = VEC_index (int, inf_data->syscalls_counts, iter);
> -          VEC_replace (int, inf_data->syscalls_counts, iter, ++elem);
> +	  if (iter >= inf_data->syscalls_counts.size ())
> +	    inf_data->syscalls_counts.resize (iter + 1);

I wasn't sure at first if this would insert zeros, but after some 
searching it seems like the new elements are value-initialized, so it's 
fine.

LGTM too.

Simon