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

login
register
mail settings
Submitter Tom Tromey
Date Nov. 30, 2017, 3:01 a.m.
Message ID <20171130030140.14830-6-tom@tromey.com>
Download mbox | patch
Permalink /patch/24621/
State New
Headers show

Comments

Tom Tromey - Nov. 30, 2017, 3:01 a.m.
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(-)
Joel Brobecker - Nov. 30, 2017, 10:03 p.m.
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!

Patch

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);
 }