[RFA 0/6] remove cleanups from Ada

[RFA 0/6] remove cleanups from Ada

[Tom Tromey]

This series removes the remaining cleanups from ada-lang.c.

Regression tested by the buildbot.

Tom

[RFA 6/6] Remove unnecessary cleanup from ada_collect_symbol_completion_matches

[Tom Tromey]

This removes an apparently unnecessary null cleanup from
ada_collect_symbol_completion_matches.  I believe the last cleanup was
removed from this function in b5ec77.

ChangeLog
2017-11-29  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (ada_collect_symbol_completion_matches): Remove
	cleanup.
---
 gdb/ChangeLog  | 5 +++++
 gdb/ada-lang.c | 3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d222918362..014d2841f0 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -6434,7 +6434,6 @@ ada_collect_symbol_completion_matches (completion_tracker &tracker,
   const struct block *b, *surrounding_static_block = 0;
   int i;
   struct block_iterator iter;
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
 
   gdb_assert (code == TYPE_CODE_UNDEF);
 
@@ -6522,8 +6521,6 @@ ada_collect_symbol_completion_matches (completion_tracker &tracker,
 				lookup_name, text, word);
     }
   }
-
-  do_cleanups (old_chain);
 }
 
                                 /* Field Access */
-- 
2.13.6

[RFA 3/6] Remove cleanup from print_mention_exception

[Tom Tromey]

This removes a cleanup from print_mention_exception by using
string_printf.

ChangeLog
2017-11-29  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (print_mention_exception): Use std::string.
---
 gdb/ChangeLog  | 4 ++++
 gdb/ada-lang.c | 8 +++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 28df250104..0248663ba8 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12537,11 +12537,9 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
       case ada_catch_exception:
         if (c->excep_string != NULL)
 	  {
-	    char *info = xstrprintf (_("`%s' Ada exception"), c->excep_string);
-	    struct cleanup *old_chain = make_cleanup (xfree, info);
-
-	    uiout->text (info);
-	    do_cleanups (old_chain);
+	    std::string info = string_printf (_("`%s' Ada exception"),
+					      c->excep_string);
+	    uiout->text (info.c_str ());
 	  }
         else
           uiout->text (_("all Ada exceptions"));
-- 
2.13.6

[RFA 1/6] Remove cleanup from old_renaming_is_invisible

[Tom Tromey]

This removes a cleanup from ada-lang.c by changing xget_renaming_scope
to return a std::string.

ChangeLog
2017-11-29  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (xget_renaming_scope): Return std::string.
	(old_renaming_is_invisible): Update.
---
 gdb/ChangeLog  |  5 +++++
 gdb/ada-lang.c | 35 +++++++----------------------------
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 3265c211fb..d5dfa07b78 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5180,9 +5180,9 @@ remove_extra_symbols (struct block_symbol *syms, int nsyms)
 /* Given a type that corresponds to a renaming entity, use the type name
    to extract the scope (package name or function name, fully qualified,
    and following the GNAT encoding convention) where this renaming has been
-   defined.  The string returned needs to be deallocated after use.  */
+   defined.  */
 
-static char *
+static std::string
 xget_renaming_scope (struct type *renaming_type)
 {
   /* The renaming types adhere to the following convention:
@@ -5193,8 +5193,6 @@ xget_renaming_scope (struct type *renaming_type)
   const char *name = type_name_no_tag (renaming_type);
   const char *suffix = strstr (name, "___XR");
   const char *last;
-  int scope_len;
-  char *scope;
 
   /* Now, backtrack a bit until we find the first "__".  Start looking
      at suffix - 3, as the <rename> part is at least one character long.  */
@@ -5204,14 +5202,7 @@ xget_renaming_scope (struct type *renaming_type)
       break;
 
   /* Make a copy of scope and return it.  */
-
-  scope_len = last - name;
-  scope = (char *) xmalloc ((scope_len + 1) * sizeof (char));
-
-  strncpy (scope, name, scope_len);
-  scope[scope_len] = '\0';
-
-  return scope;
+  return std::string (name, last);
 }
 
 /* Return nonzero if NAME corresponds to a package name.  */
@@ -5251,21 +5242,14 @@ is_package_name (const char *name)
 static int
 old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
 {
-  char *scope;
-  struct cleanup *old_chain;
-
   if (SYMBOL_CLASS (sym) != LOC_TYPEDEF)
     return 0;
 
-  scope = xget_renaming_scope (SYMBOL_TYPE (sym));
-  old_chain = make_cleanup (xfree, scope);
+  std::string scope = xget_renaming_scope (SYMBOL_TYPE (sym));
 
   /* If the rename has been defined in a package, then it is visible.  */
-  if (is_package_name (scope))
-    {
-      do_cleanups (old_chain);
-      return 0;
-    }
+  if (is_package_name (scope.c_str ()))
+    return 0;
 
   /* Check that the rename is in the current function scope by checking
      that its name starts with SCOPE.  */
@@ -5277,12 +5261,7 @@ old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
   if (startswith (function_name, "_ada_"))
     function_name += 5;
 
-  {
-    int is_invisible = !startswith (function_name, scope);
-
-    do_cleanups (old_chain);
-    return is_invisible;
-  }
+  return !startswith (function_name, scope.c_str ());
 }
 
 /* Remove entries from SYMS that corresponds to a renaming entity that
-- 
2.13.6

[RFA 4/6] Remove some more cleanups from ada-lang.c

[Tom Tromey]

This removes a few more cleanups from ada-lang.c by using
unique_xmalloc_ptr in a couple of spots.

ChangeLog
2017-11-29  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (ada_exception_message_1): Return
	unique_xmalloc_ptr.
	(ada_exception_message_1): Update.
	(ada_exception_message): Return unique_xmalloc_ptr.
	(print_it_exception): Update.
---
 gdb/ChangeLog  |  8 ++++++++
 gdb/ada-lang.c | 31 +++++++++----------------------
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 0248663ba8..8d0ac60575 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12088,8 +12088,6 @@ ada_exception_name_addr_1 (enum ada_exception_catchpoint_kind ex,
    return the message which was associated to the exception, if
    available.  Return NULL if the message could not be retrieved.
 
-   The caller must xfree the string after use.
-
    Note: The exception message can be associated to an exception
    either through the use of the Raise_Exception function, or
    more simply (Ada 2005 and later), via:
@@ -12098,13 +12096,11 @@ ada_exception_name_addr_1 (enum ada_exception_catchpoint_kind ex,
 
    */
 
-static char *
+static gdb::unique_xmalloc_ptr<char>
 ada_exception_message_1 (void)
 {
   struct value *e_msg_val;
-  char *e_msg = NULL;
   int e_msg_len;
-  struct cleanup *cleanups;
 
   /* For runtimes that support this feature, the exception message
      is passed as an unbounded string argument called "message".  */
@@ -12121,34 +12117,30 @@ ada_exception_message_1 (void)
   if (e_msg_len <= 0)
     return NULL;
 
-  e_msg = (char *) xmalloc (e_msg_len + 1);
-  cleanups = make_cleanup (xfree, e_msg);
+  gdb::unique_xmalloc_ptr<char> result ((char *) xmalloc (e_msg_len + 1));
+  char *e_msg = result.get ();
   read_memory_string (value_address (e_msg_val), e_msg, e_msg_len + 1);
   e_msg[e_msg_len] = '\0';
 
-  discard_cleanups (cleanups);
-  return e_msg;
+  return result;
 }
 
 /* Same as ada_exception_message_1, except that all exceptions are
    contained here (returning NULL instead).  */
 
-static char *
+static gdb::unique_xmalloc_ptr<char>
 ada_exception_message (void)
 {
-  char *e_msg = NULL;  /* Avoid a spurious uninitialized warning.  */
-
   TRY
     {
-      e_msg = ada_exception_message_1 ();
+      return ada_exception_message_1 ();
     }
   CATCH (e, RETURN_MASK_ERROR)
     {
-      e_msg = NULL;
     }
   END_CATCH
 
-  return e_msg;
+  return nullptr;
 }
 
 /* Same as ada_exception_name_addr_1, except that it intercepts and contains
@@ -12383,7 +12375,6 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
 {
   struct ui_out *uiout = current_uiout;
   struct breakpoint *b = bs->breakpoint_at;
-  char *exception_message;
 
   annotate_catchpoint (b->number);
 
@@ -12450,16 +12441,12 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
 	break;
     }
 
-  exception_message = ada_exception_message ();
+  gdb::unique_xmalloc_ptr<char> exception_message = ada_exception_message ();
   if (exception_message != NULL)
     {
-      struct cleanup *cleanups = make_cleanup (xfree, exception_message);
-
       uiout->text (" (");
-      uiout->field_string ("exception-message", exception_message);
+      uiout->field_string ("exception-message", exception_message.get ());
       uiout->text (")");
-
-      do_cleanups (cleanups);
     }
 
   uiout->text (" at ");
-- 
2.13.6

[RFA 5/6] Change Ada exceptions to use std::string

[Tom Tromey]

This changes the Ada exception code to use std::string, allowing the
removal of some more cleanups.

ChangeLog
2017-11-29  Tom Tromey  <tom@tromey.com>

	* mi/mi-cmd-catch.c (mi_cmd_catch_assert)
	(mi_cmd_catch_exception): Update.
	* ada-lang.h (create_ada_exception_catchpoint): Update.
	* ada-lang.c (struct ada_catchpoint) <excep_string>: Now a
	std::string.
	(create_excep_cond_exprs, ~ada_catchpoint)
	(should_stop_exception, print_one_exception)
	(print_mention_exception, print_recreate_exception): Update.
	(ada_get_next_arg): Remove.
	(catch_ada_exception_command_split): Change two arguments to
	"std::string *".  Remove cleanups.
	(ada_exception_catchpoint_cond_string): Change "string" to
	std::string.
	(ada_exception_sal): Remove excep_string parameter.
	(create_ada_exception_catchpoint): Change two arguments to
	"std::string &&".
	(catch_ada_exception_command): Update.
---
 gdb/ChangeLog         |  20 +++++++++
 gdb/ada-lang.c        | 118 +++++++++++++++-----------------------------------
 gdb/ada-lang.h        |   4 +-
 gdb/mi/mi-cmd-catch.c |  19 ++++----
 4 files changed, 66 insertions(+), 95 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 8d0ac60575..d222918362 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12170,7 +12170,7 @@ ada_exception_name_addr (enum ada_exception_catchpoint_kind ex,
 }
 
 static std::string ada_exception_catchpoint_cond_string
-    (const char *excep_string);
+    (const std::string &excep_string);
 
 /* Ada catchpoints.
 
@@ -12226,7 +12226,7 @@ struct ada_catchpoint : public breakpoint
   ~ada_catchpoint () override;
 
   /* The name of the specific exception the user specified.  */
-  char *excep_string;
+  std::string excep_string;
 };
 
 /* Parse the exception condition string in the context of each of the
@@ -12238,7 +12238,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
   struct bp_location *bl;
 
   /* Nothing to do if there's no specific exception to catch.  */
-  if (c->excep_string == NULL)
+  if (c->excep_string.empty ())
     return;
 
   /* Same if there are no locations... */
@@ -12286,7 +12286,6 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 
 ada_catchpoint::~ada_catchpoint ()
 {
-  xfree (this->excep_string);
 }
 
 /* Implement the ALLOCATE_LOCATION method in the breakpoint_ops
@@ -12329,7 +12328,7 @@ should_stop_exception (const struct bp_location *bl)
   int stop;
 
   /* With no specific exception, should always stop.  */
-  if (c->excep_string == NULL)
+  if (c->excep_string.empty ())
     return 1;
 
   if (ada_loc->excep_cond_expr == NULL)
@@ -12478,12 +12477,12 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
   switch (ex)
     {
       case ada_catch_exception:
-        if (c->excep_string != NULL)
+        if (!c->excep_string.empty ())
           {
-            char *msg = xstrprintf (_("`%s' Ada exception"), c->excep_string);
+	    std::string msg = string_printf (_("`%s' Ada exception"),
+					     c->excep_string.c_str ());
 
-            uiout->field_string ("what", msg);
-            xfree (msg);
+            uiout->field_string ("what", msg.c_str ());
           }
         else
           uiout->field_string ("what", "all Ada exceptions");
@@ -12522,10 +12521,10 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
   switch (ex)
     {
       case ada_catch_exception:
-        if (c->excep_string != NULL)
+        if (!c->excep_string.empty ())
 	  {
 	    std::string info = string_printf (_("`%s' Ada exception"),
-					      c->excep_string);
+					      c->excep_string.c_str ());
 	    uiout->text (info.c_str ());
 	  }
         else
@@ -12559,8 +12558,8 @@ print_recreate_exception (enum ada_exception_catchpoint_kind ex,
     {
       case ada_catch_exception:
 	fprintf_filtered (fp, "catch exception");
-	if (c->excep_string != NULL)
-	  fprintf_filtered (fp, " %s", c->excep_string);
+	if (!c->excep_string.empty ())
+	  fprintf_filtered (fp, " %s", c->excep_string.c_str ());
 	break;
 
       case ada_catch_exception_unhandled:
@@ -12717,40 +12716,6 @@ print_recreate_catch_assert (struct breakpoint *b, struct ui_file *fp)
 
 static struct breakpoint_ops catch_assert_breakpoint_ops;
 
-/* Return a newly allocated copy of the first space-separated token
-   in ARGSP, and then adjust ARGSP to point immediately after that
-   token.
-
-   Return NULL if ARGPS does not contain any more tokens.  */
-
-static char *
-ada_get_next_arg (const char **argsp)
-{
-  const char *args = *argsp;
-  const char *end;
-  char *result;
-
-  args = skip_spaces (args);
-  if (args[0] == '\0')
-    return NULL; /* No more arguments.  */
-  
-  /* Find the end of the current argument.  */
-
-  end = skip_to_space (args);
-
-  /* Adjust ARGSP to point to the start of the next argument.  */
-
-  *argsp = end;
-
-  /* Make a copy of the current argument and return it.  */
-
-  result = (char *) xmalloc (end - args + 1);
-  strncpy (result, args, end - args);
-  result[end - args] = '\0';
-  
-  return result;
-}
-
 /* Split the arguments specified in a "catch exception" command.  
    Set EX to the appropriate catchpoint type.
    Set EXCEP_STRING to the name of the specific exception if
@@ -12762,28 +12727,21 @@ ada_get_next_arg (const char **argsp)
 static void
 catch_ada_exception_command_split (const char *args,
                                    enum ada_exception_catchpoint_kind *ex,
-				   char **excep_string,
-				   char **cond_string)
+				   std::string *excep_string,
+				   std::string *cond_string)
 {
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-  char *exception_name;
-  char *cond = NULL;
-
-  exception_name = ada_get_next_arg (&args);
-  if (exception_name != NULL && strcmp (exception_name, "if") == 0)
+  std::string exception_name = extract_arg (&args);
+  if (exception_name == "if")
     {
       /* This is not an exception name; this is the start of a condition
 	 expression for a catchpoint on all exceptions.  So, "un-get"
 	 this token, and set exception_name to NULL.  */
-      xfree (exception_name);
-      exception_name = NULL;
+      exception_name.clear ();
       args -= 2;
     }
-  make_cleanup (xfree, exception_name);
 
   /* Check to see if we have a condition.  */
 
-  args = skip_spaces (args);
   if (startswith (args, "if")
       && (isspace (args[2]) || args[2] == '\0'))
     {
@@ -12792,8 +12750,7 @@ catch_ada_exception_command_split (const char *args,
 
       if (args[0] == '\0')
         error (_("Condition missing after `if' keyword"));
-      cond = xstrdup (args);
-      make_cleanup (xfree, cond);
+      *cond_string = args;
 
       args += strlen (args);
     }
@@ -12804,19 +12761,15 @@ catch_ada_exception_command_split (const char *args,
   if (args[0] != '\0')
     error (_("Junk at end of expression"));
 
-  discard_cleanups (old_chain);
-
-  if (exception_name == NULL)
+  if (exception_name.empty ())
     {
       /* Catch all exceptions.  */
       *ex = ada_catch_exception;
-      *excep_string = NULL;
     }
-  else if (strcmp (exception_name, "unhandled") == 0)
+  else if (exception_name == "unhandled")
     {
       /* Catch unhandled exceptions.  */
       *ex = ada_catch_exception_unhandled;
-      *excep_string = NULL;
     }
   else
     {
@@ -12824,7 +12777,6 @@ catch_ada_exception_command_split (const char *args,
       *ex = ada_catch_exception;
       *excep_string = exception_name;
     }
-  *cond_string = cond;
 }
 
 /* Return the name of the symbol on which we should break in order to
@@ -12883,7 +12835,7 @@ ada_exception_breakpoint_ops (enum ada_exception_catchpoint_kind ex)
    an exception catchpoint.  */
 
 static std::string
-ada_exception_catchpoint_cond_string (const char *excep_string)
+ada_exception_catchpoint_cond_string (const std::string &excep_string)
 {
   int i;
 
@@ -12908,29 +12860,26 @@ ada_exception_catchpoint_cond_string (const char *excep_string)
 
   for (i = 0; i < sizeof (standard_exc) / sizeof (char *); i++)
     {
-      if (strcmp (standard_exc [i], excep_string) == 0)
+      if (standard_exc [i] == excep_string)
 	{
           return
 	    string_printf ("long_integer (e) = long_integer (&standard.%s)",
-			   excep_string);
+			   excep_string.c_str ());
 	}
     }
   return string_printf ("long_integer (e) = long_integer (&%s)",
-			excep_string);
+			excep_string.c_str ());
 }
 
 /* Return the symtab_and_line that should be used to insert an exception
    catchpoint of the TYPE kind.
 
-   EXCEP_STRING should contain the name of a specific exception that
-   the catchpoint should catch, or NULL otherwise.
-
    ADDR_STRING returns the name of the function where the real
    breakpoint that implements the catchpoints is set, depending on the
    type of catchpoint we need to create.  */
 
 static struct symtab_and_line
-ada_exception_sal (enum ada_exception_catchpoint_kind ex, char *excep_string,
+ada_exception_sal (enum ada_exception_catchpoint_kind ex,
 		   const char **addr_string, const struct breakpoint_ops **ops)
 {
   const char *sym_name;
@@ -12984,8 +12933,8 @@ ada_exception_sal (enum ada_exception_catchpoint_kind ex, char *excep_string,
 void
 create_ada_exception_catchpoint (struct gdbarch *gdbarch,
 				 enum ada_exception_catchpoint_kind ex_kind,
-				 char *excep_string,
-				 char *cond_string,
+				 std::string &&excep_string,
+				 std::string &&cond_string,
 				 int tempflag,
 				 int disabled,
 				 int from_tty)
@@ -12993,15 +12942,15 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
   const char *addr_string = NULL;
   const struct breakpoint_ops *ops = NULL;
   struct symtab_and_line sal
-    = ada_exception_sal (ex_kind, excep_string, &addr_string, &ops);
+    = ada_exception_sal (ex_kind, &addr_string, &ops);
 
   std::unique_ptr<ada_catchpoint> c (new ada_catchpoint ());
   init_ada_exception_breakpoint (c.get (), gdbarch, sal, addr_string,
 				 ops, tempflag, disabled, from_tty);
   c->excep_string = excep_string;
   create_excep_cond_exprs (c.get ());
-  if (cond_string != NULL)
-    set_breakpoint_condition (c.get (), cond_string, from_tty);
+  if (!cond_string.empty ())
+    set_breakpoint_condition (c.get (), cond_string.c_str (), from_tty);
   install_breakpoint (0, std::move (c), 1);
 }
 
@@ -13015,8 +12964,8 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
   struct gdbarch *gdbarch = get_current_arch ();
   int tempflag;
   enum ada_exception_catchpoint_kind ex_kind;
-  char *excep_string = NULL;
-  char *cond_string = NULL;
+  std::string excep_string;
+  std::string cond_string;
 
   tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
 
@@ -13025,7 +12974,8 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
   catch_ada_exception_command_split (arg, &ex_kind, &excep_string,
 				     &cond_string);
   create_ada_exception_catchpoint (gdbarch, ex_kind,
-				   excep_string, cond_string,
+				   std::move (excep_string),
+				   std::move (cond_string),
 				   tempflag, 1 /* enabled */,
 				   from_tty);
 }
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 0530e9aacd..f1190dca7c 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -378,8 +378,8 @@ extern std::string ada_name_for_lookup (const char *name);
 
 extern void create_ada_exception_catchpoint
   (struct gdbarch *gdbarch, enum ada_exception_catchpoint_kind ex_kind,
-   char *excep_string, char *cond_string, int tempflag, int disabled,
-   int from_tty);
+   std::string &&excep_string, std::string &&cond_string, int tempflag,
+   int disabled, int from_tty);
 
 /* Some information about a given Ada exception.  */
 
diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
index 9c103d25dc..f69cdaa3f7 100644
--- a/gdb/mi/mi-cmd-catch.c
+++ b/gdb/mi/mi-cmd-catch.c
@@ -79,12 +79,12 @@ mi_cmd_catch_assert (const char *cmd, char *argv[], int argc)
     error (_("Invalid argument: %s"), argv[oind]);
 
   scoped_restore restore_breakpoint_reporting = setup_breakpoint_reporting ();
-  /* create_ada_exception_catchpoint needs CONDITION to be xstrdup'ed,
-     and will assume control of its lifetime.  */
+  std::string condition_str;
   if (condition != NULL)
-    condition = xstrdup (condition);
+    condition_str = condition;
   create_ada_exception_catchpoint (gdbarch, ada_catch_assert,
-				   NULL, condition, temp, enabled, 0);
+				   std::string (), std::move (condition_str),
+				   temp, enabled, 0);
 }
 
 /* Handler for the -catch-exception command.  */
@@ -156,14 +156,15 @@ mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
     error (_("\"-e\" and \"-u\" are mutually exclusive"));
 
   scoped_restore restore_breakpoint_reporting = setup_breakpoint_reporting ();
-  /* create_ada_exception_catchpoint needs EXCEPTION_NAME and CONDITION
-     to be xstrdup'ed, and will assume control of their lifetime.  */
+  std::string except_str;
   if (exception_name != NULL)
-    exception_name = xstrdup (exception_name);
+    except_str = exception_name;
+  std::string cond_str;
   if (condition != NULL)
-    condition = xstrdup (condition);
+    cond_str = condition;
   create_ada_exception_catchpoint (gdbarch, ex_kind,
-				   exception_name, condition,
+				   std::move (exception_name),
+				   std::move (condition),
 				   temp, enabled, 0);
 }
 
-- 
2.13.6

Re: [RFA 5/6] Change Ada exceptions to use std::string

[Joel Brobecker]

Hi Tom,

> This changes the Ada exception code to use std::string, allowing the
> removal of some more cleanups.
> 
> ChangeLog
> 2017-11-29  Tom Tromey  <tom@tromey.com>
> 
> 	* mi/mi-cmd-catch.c (mi_cmd_catch_assert)
> 	(mi_cmd_catch_exception): Update.
> 	* ada-lang.h (create_ada_exception_catchpoint): Update.
> 	* ada-lang.c (struct ada_catchpoint) <excep_string>: Now a
> 	std::string.
> 	(create_excep_cond_exprs, ~ada_catchpoint)
> 	(should_stop_exception, print_one_exception)
> 	(print_mention_exception, print_recreate_exception): Update.
> 	(ada_get_next_arg): Remove.
> 	(catch_ada_exception_command_split): Change two arguments to
> 	"std::string *".  Remove cleanups.
> 	(ada_exception_catchpoint_cond_string): Change "string" to
> 	std::string.
> 	(ada_exception_sal): Remove excep_string parameter.
> 	(create_ada_exception_catchpoint): Change two arguments to
> 	"std::string &&".
> 	(catch_ada_exception_command): Update.

As hinted in my answer to the cover email, I found a few issues.
Attached is a commit that explains and fixes them. If the fixes
look good to you, a new commit combining the two is preapproved.

(I tested my patch on x86_64-linux, limiting myself to gdb.ada)

Thanks!
-- 
Joel

Re: [RFA 5/6] Change Ada exceptions to use std::string

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

[really attached, this time!]

> > This changes the Ada exception code to use std::string, allowing the
> > removal of some more cleanups.
> > 
> > ChangeLog
> > 2017-11-29  Tom Tromey  <tom@tromey.com>
> > 
> > 	* mi/mi-cmd-catch.c (mi_cmd_catch_assert)
> > 	(mi_cmd_catch_exception): Update.
> > 	* ada-lang.h (create_ada_exception_catchpoint): Update.
> > 	* ada-lang.c (struct ada_catchpoint) <excep_string>: Now a
> > 	std::string.
> > 	(create_excep_cond_exprs, ~ada_catchpoint)
> > 	(should_stop_exception, print_one_exception)
> > 	(print_mention_exception, print_recreate_exception): Update.
> > 	(ada_get_next_arg): Remove.
> > 	(catch_ada_exception_command_split): Change two arguments to
> > 	"std::string *".  Remove cleanups.
> > 	(ada_exception_catchpoint_cond_string): Change "string" to
> > 	std::string.
> > 	(ada_exception_sal): Remove excep_string parameter.
> > 	(create_ada_exception_catchpoint): Change two arguments to
> > 	"std::string &&".
> > 	(catch_ada_exception_command): Update.
> 
> As hinted in my answer to the cover email, I found a few issues.
> Attached is a commit that explains and fixes them. If the fixes
> look good to you, a new commit combining the two is preapproved.
> 
> (I tested my patch on x86_64-linux, limiting myself to gdb.ada)


-- 
Joel

[-- Attachment #2: 0001-Fix-a-couple-of-regressions-introduced-by-the-previo.patch --]
[-- Type: text/x-diff, Size: 5373 bytes --]

From c2ad16ea37be009bf58afb7467f2bdd67b31710a Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 30 Nov 2017 16:15:09 -0500
Subject: [PATCH] Fix a couple of regressions introduced by the previous patch

Pb #1: Crash while trying to insert an assert catchpoint:

    With any program built with "gnatmake -g -gnata ...", trying
    to insert a catchpoint on failed assertions would cause an internal
    error:

        (gdb) catch assert
        /[...]/common/cleanups.c:264: internal-warning: restore_my_cleanups has found a stale cleanup

    I tracked this down to an issue in the call to
    create_ada_exception_catchpoint, where we pass NULL for at least
    one of the parameters declared as a "std::string &&". I don't know
    C++ well enough, but instruction-by-instruction leads me to believe
    that the constructor doesn't like NULL as a char *, so we get an
    exception, and that exception eventually somehow leads to the
    cleanup error above (not sure how, unfortunately). The fix was
    to pass an std::string() instead.

    And by the time I understood this for the cond string parameter
    (on which I had target fixation), I had C++-ifed
    catch_ada_assert_command_split.

Pb #2: Same kind of crash insertin an exception in GDB/MI mode.

    In this case, it was just a case of getting confused between
    a couple of temporary "char *" variables, and the corresponding
    std::string parameters that Tom introduced.

    I first fixed the confusion, but then I wondered why using
    intermediate "char *" variables at all. So I changed this function
    to only use std::string variables.

Pb #3: condition handling in exception catchpoints is broken

    (gdb) catch exception constraint_error if True
    Junk at end of expression

    Tom removed a call to "skip_spaces" before checking for the next
    argument, and I am not sure I understand why. But debugging
    the problem showed by arg was equal to...

        "constraint_error if True"

    ... and that once extract_arg is called, args is now equal to...

        " if True"

    ... so the condition identifying an "if" block no longer works.
    I restored the call to skip_spaces.
---
 gdb/ada-lang.c        |  9 +++++----
 gdb/mi/mi-cmd-catch.c | 12 +++---------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 5e9aebf..1caa3dc 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12744,6 +12744,7 @@ catch_ada_exception_command_split (const char *args,
 
   /* Check to see if we have a condition.  */
 
+  args = skip_spaces (args);
   if (startswith (args, "if")
       && (isspace (args[2]) || args[2] == '\0'))
     {
@@ -12991,7 +12992,7 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
    (the memory needs to be deallocated after use).  */
 
 static void
-catch_ada_assert_command_split (const char *args, char **cond_string)
+catch_ada_assert_command_split (const char *args, std::string *cond_string)
 {
   args = skip_spaces (args);
 
@@ -13003,7 +13004,7 @@ catch_ada_assert_command_split (const char *args, char **cond_string)
       args = skip_spaces (args);
       if (args[0] == '\0')
         error (_("condition missing after `if' keyword"));
-      *cond_string = xstrdup (args);
+      *cond_string = args;
     }
 
   /* Otherwise, there should be no other argument at the end of
@@ -13021,7 +13022,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
   const char *arg = arg_entry;
   struct gdbarch *gdbarch = get_current_arch ();
   int tempflag;
-  char *cond_string = NULL;
+  std::string cond_string;
 
   tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
 
@@ -13029,7 +13030,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
     arg = "";
   catch_ada_assert_command_split (arg, &cond_string);
   create_ada_exception_catchpoint (gdbarch, ada_catch_assert,
-				   NULL, cond_string,
+				   std::string (), std::move (cond_string),
 				   tempflag, 1 /* enabled */,
 				   from_tty);
 }
diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
index f69cdaa..70939ee 100644
--- a/gdb/mi/mi-cmd-catch.c
+++ b/gdb/mi/mi-cmd-catch.c
@@ -93,9 +93,9 @@ void
 mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
 {
   struct gdbarch *gdbarch = get_current_arch();
-  char *condition = NULL;
+  std::string condition;
   int enabled = 1;
-  char *exception_name = NULL;
+  std::string exception_name;
   int temp = 0;
   enum ada_exception_catchpoint_kind ex_kind = ada_catch_exception;
 
@@ -152,16 +152,10 @@ mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
 
   /* Specifying an exception name does not make sense when requesting
      an unhandled exception breakpoint.  */
-  if (ex_kind == ada_catch_exception_unhandled && exception_name != NULL)
+  if (ex_kind == ada_catch_exception_unhandled && ! exception_name.empty())
     error (_("\"-e\" and \"-u\" are mutually exclusive"));
 
   scoped_restore restore_breakpoint_reporting = setup_breakpoint_reporting ();
-  std::string except_str;
-  if (exception_name != NULL)
-    except_str = exception_name;
-  std::string cond_str;
-  if (condition != NULL)
-    cond_str = condition;
   create_ada_exception_catchpoint (gdbarch, ex_kind,
 				   std::move (exception_name),
 				   std::move (condition),
-- 
2.1.4

Re: [RFA 5/6] Change Ada exceptions to use std::string

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> (I tested my patch on x86_64-linux, limiting myself to gdb.ada)

Joel> Pb #1: Crash while trying to insert an assert catchpoint:

I wonder why I didn't see any problems from the buildbot.

Joel> Pb #3: condition handling in exception catchpoints is broken

Joel>     Tom removed a call to "skip_spaces" before checking for the next
Joel>     argument, and I am not sure I understand why.

Here I was confused and thought extract_arg skipped spaces again after
extracting the arg.  But, it doesn't.

Still - why not test failures?  This is disturbing.

Tom

Re: [RFA 5/6] Change Ada exceptions to use std::string

[Joel Brobecker]

> Joel> Pb #1: Crash while trying to insert an assert catchpoint:
> 
> I wonder why I didn't see any problems from the buildbot.
> 
> Joel> Pb #3: condition handling in exception catchpoints is broken
> 
> Joel>     Tom removed a call to "skip_spaces" before checking for the next
> Joel>     argument, and I am not sure I understand why.
> 
> Here I was confused and thought extract_arg skipped spaces again after
> extracting the arg.  But, it doesn't.
> 
> Still - why not test failures?  This is disturbing.

That's indeed strange. Do we have access to the reference report?
Maybe the Ada testing is not run (eg: missing Ada compiler on
the buildbot), or maybe the Ada compiler there is such that the tests
were failing in the first place, preventing it from detecting
regressions?

In any case, I'm quite happy to be the safety net in this case...

-- 
Joel

[RFA 2/6] Remove cleanup from create_excep_cond_exprs

[Tom Tromey]

This removes a cleanup from create_excep_cond_exprs by changing
ada_exception_catchpoint_cond_string to return a std::string.

ChangeLog
2017-11-29  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (create_excep_cond_exprs): Update.
	(ada_exception_catchpoint_cond_string): Return std::string.
---
 gdb/ChangeLog  |  5 +++++
 gdb/ada-lang.c | 28 ++++++++++++----------------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d5dfa07b78..28df250104 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12177,7 +12177,8 @@ ada_exception_name_addr (enum ada_exception_catchpoint_kind ex,
   return result;
 }
 
-static char *ada_exception_catchpoint_cond_string (const char *excep_string);
+static std::string ada_exception_catchpoint_cond_string
+    (const char *excep_string);
 
 /* Ada catchpoints.
 
@@ -12242,9 +12243,7 @@ struct ada_catchpoint : public breakpoint
 static void
 create_excep_cond_exprs (struct ada_catchpoint *c)
 {
-  struct cleanup *old_chain;
   struct bp_location *bl;
-  char *cond_string;
 
   /* Nothing to do if there's no specific exception to catch.  */
   if (c->excep_string == NULL)
@@ -12256,8 +12255,8 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 
   /* Compute the condition expression in text form, from the specific
      expection we want to catch.  */
-  cond_string = ada_exception_catchpoint_cond_string (c->excep_string);
-  old_chain = make_cleanup (xfree, cond_string);
+  std::string cond_string
+    = ada_exception_catchpoint_cond_string (c->excep_string);
 
   /* Iterate over all the catchpoint's locations, and parse an
      expression for each.  */
@@ -12271,7 +12270,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 	{
 	  const char *s;
 
-	  s = cond_string;
+	  s = cond_string.c_str ();
 	  TRY
 	    {
 	      exp = parse_exp_1 (&s, bl->address,
@@ -12289,8 +12288,6 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 
       ada_loc->excep_cond_expr = std::move (exp);
     }
-
-  do_cleanups (old_chain);
 }
 
 /* ada_catchpoint destructor.  */
@@ -12898,12 +12895,9 @@ ada_exception_breakpoint_ops (enum ada_exception_catchpoint_kind ex)
 /* Return the condition that will be used to match the current exception
    being raised with the exception that the user wants to catch.  This
    assumes that this condition is used when the inferior just triggered
-   an exception catchpoint.
-   
-   The string returned is a newly allocated string that needs to be
-   deallocated later.  */
+   an exception catchpoint.  */
 
-static char *
+static std::string
 ada_exception_catchpoint_cond_string (const char *excep_string)
 {
   int i;
@@ -12931,11 +12925,13 @@ ada_exception_catchpoint_cond_string (const char *excep_string)
     {
       if (strcmp (standard_exc [i], excep_string) == 0)
 	{
-          return xstrprintf ("long_integer (e) = long_integer (&standard.%s)",
-                             excep_string);
+          return
+	    string_printf ("long_integer (e) = long_integer (&standard.%s)",
+			   excep_string);
 	}
     }
-  return xstrprintf ("long_integer (e) = long_integer (&%s)", excep_string);
+  return string_printf ("long_integer (e) = long_integer (&%s)",
+			excep_string);
 }
 
 /* Return the symtab_and_line that should be used to insert an exception
-- 
2.13.6

Re: [RFA 0/6] remove cleanups from Ada

[Joel Brobecker]

Hi Tom,

> This series removes the remaining cleanups from ada-lang.c.
> Regression tested by the buildbot.

Thanks for doing that. All patches looked good to me, except one of
them, where I found a number of issues (thanks to testing). All
those issues except potentially one of them (still to be verified)
were also caught by the testsuite already available, so I'm not sure
why the buildbot didn't catch them. Perhaps the Ada compiler available
there doesn't allow clean results for exception catchpoints to start
with. In any case, I will send my fixes as answer to that one patch.
All other patches are OK.

-- 
Joel