[RFA 3/3] Make extract_arg return a std::string

[Tom Tromey <tom at tromey dot com>]

Change extract_arg to return a std::string and fix up all the users.
I think string is mildly better than unique_xmalloc_ptr<char>, when
possible, because it provides a more robust API.

I changed the error messages emitted from find_location_by_number to
avoid either writing to a string or an extra allocation; this can be
changed but I thought that the new message was not any less clear.
You can see an example in the testsuite patch.

ChangeLog
2017-09-10  Tom Tromey  <tom@tromey.com>

	* demangle.c (demangle_command): Update.
	* breakpoint.c (disable_command): Update.
	(enable_command): Update.
	(find_location_by_number): Make "number" const.  Use
	get_number_trailer.
	* cli/cli-utils.c (extract_arg): Return std::string.
	* probe.c (parse_probe_linespec): Update.  Change types.
	(collect_probes): Take string arguments.
	(parse_probe_linespec): Likewise.
	(info_probes_for_ops): Update.
	(enable_probes_command): Update.
	(disable_probes_command): Update.
	* break-catch-sig.c (catch_signal_split_args): Update.
	* mi/mi-parse.c (mi_parse): Update.

testsuite/ChangeLog
2017-09-10  Tom Tromey  <tom@tromey.com>

	* gdb.base/ena-dis-br.exp (test_ena_dis_br): Update test.
---
 gdb/ChangeLog                         | 17 ++++++++++++
 gdb/break-catch-sig.c                 | 12 ++++-----
 gdb/breakpoint.c                      | 39 ++++++++++++++-------------
 gdb/cli/cli-utils.c                   | 14 +++++-----
 gdb/cli/cli-utils.h                   | 13 ++++-----
 gdb/demangle.c                        | 14 +++++-----
 gdb/mi/mi-parse.c                     | 12 +++------
 gdb/probe.c                           | 50 +++++++++++++++--------------------
 gdb/testsuite/ChangeLog               |  4 +++
 gdb/testsuite/gdb.base/ena-dis-br.exp |  2 +-
 10 files changed, 91 insertions(+), 86 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4c2a924..3201486 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,22 @@
 2017-09-10  Tom Tromey  <tom@tromey.com>
 
+	* demangle.c (demangle_command): Update.
+	* breakpoint.c (disable_command): Update.
+	(enable_command): Update.
+	(find_location_by_number): Make "number" const.  Use
+	get_number_trailer.
+	* cli/cli-utils.c (extract_arg): Return std::string.
+	* probe.c (parse_probe_linespec): Update.  Change types.
+	(collect_probes): Take string arguments.
+	(parse_probe_linespec): Likewise.
+	(info_probes_for_ops): Update.
+	(enable_probes_command): Update.
+	(disable_probes_command): Update.
+	* break-catch-sig.c (catch_signal_split_args): Update.
+	* mi/mi-parse.c (mi_parse): Update.
+
+2017-09-10  Tom Tromey  <tom@tromey.com>
+
 	* language.h (language_enum): Make argument const.
 	* language.c (language_enum): Make argument const.
 
diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 805084f..237dbaa 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -343,12 +343,12 @@ catch_signal_split_args (char *arg, bool *catch_all)
       gdb_signal signal_number;
       char *endptr;
 
-      gdb::unique_xmalloc_ptr<char> one_arg (extract_arg (&arg));
-      if (one_arg == NULL)
+      std::string one_arg = extract_arg (&arg);
+      if (one_arg.empty ())
 	break;
 
       /* Check for the special flag "all".  */
-      if (strcmp (one_arg.get (), "all") == 0)
+      if (strcmp (one_arg.c_str (), "all") == 0)
 	{
 	  arg = skip_spaces (arg);
 	  if (*arg != '\0' || !first)
@@ -361,14 +361,14 @@ catch_signal_split_args (char *arg, bool *catch_all)
       first = false;
 
       /* Check if the user provided a signal name or a number.  */
-      num = (int) strtol (one_arg.get (), &endptr, 0);
+      num = (int) strtol (one_arg.c_str (), &endptr, 0);
       if (*endptr == '\0')
 	signal_number = gdb_signal_from_command (num);
       else
 	{
-	  signal_number = gdb_signal_from_name (one_arg.get ());
+	  signal_number = gdb_signal_from_name (one_arg.c_str ());
 	  if (signal_number == GDB_SIGNAL_UNKNOWN)
-	    error (_("Unknown signal name '%s'."), one_arg.get ());
+	    error (_("Unknown signal name '%s'."), one_arg.c_str ());
 	}
 
       result.push_back (signal_number);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b642bc6..79543fe 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14489,20 +14489,17 @@ map_breakpoint_numbers (const char *args,
 }
 
 static struct bp_location *
-find_location_by_number (char *number)
+find_location_by_number (const char *number)
 {
-  char *dot = strchr (number, '.');
-  char *p1;
+  const char *p1;
   int bp_num;
   int loc_num;
   struct breakpoint *b;
   struct bp_location *loc;  
 
-  *dot = '\0';
-
   p1 = number;
-  bp_num = get_number (&p1);
-  if (bp_num == 0)
+  bp_num = get_number_trailer (&p1, '.');
+  if (bp_num == 0 || p1[0] != '.')
     error (_("Bad breakpoint number '%s'"), number);
 
   ALL_BREAKPOINTS (b)
@@ -14514,7 +14511,9 @@ find_location_by_number (char *number)
   if (!b || b->number != bp_num)
     error (_("Bad breakpoint number '%s'"), number);
   
-  p1 = dot+1;
+  /* Skip the dot.  */
+  ++p1;
+  const char *save = p1;
   loc_num = get_number (&p1);
   if (loc_num == 0)
     error (_("Bad breakpoint location number '%s'"), number);
@@ -14524,7 +14523,7 @@ find_location_by_number (char *number)
   for (;loc_num && loc; --loc_num, loc = loc->next)
     ;
   if (!loc)
-    error (_("Bad breakpoint location number '%s'"), dot+1);
+    error (_("Bad breakpoint location number '%s'"), save);
     
   return loc;  
 }
@@ -14592,13 +14591,13 @@ disable_command (char *args, int from_tty)
     }
   else
     {
-      char *num = extract_arg (&args);
+      std::string num = extract_arg (&args);
 
-      while (num)
+      while (!num.empty ())
 	{
-	  if (strchr (num, '.'))
+	  if (num.find ('.') != std::string::npos)
 	    {
-	      struct bp_location *loc = find_location_by_number (num);
+	      struct bp_location *loc = find_location_by_number (num.c_str ());
 
 	      if (loc)
 		{
@@ -14615,7 +14614,8 @@ disable_command (char *args, int from_tty)
 	      update_global_location_list (UGLL_DONT_INSERT);
 	    }
 	  else
-	    map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
+	    map_breakpoint_numbers (num.c_str (), do_map_disable_breakpoint,
+				    NULL);
 	  num = extract_arg (&args);
 	}
     }
@@ -14723,13 +14723,13 @@ enable_command (char *args, int from_tty)
     }
   else
     {
-      char *num = extract_arg (&args);
+      std::string num = extract_arg (&args);
 
-      while (num)
+      while (!num.empty ())
 	{
-	  if (strchr (num, '.'))
+	  if (num.find ('.') != std::string::npos)
 	    {
-	      struct bp_location *loc = find_location_by_number (num);
+	      struct bp_location *loc = find_location_by_number (num.c_str ());
 
 	      if (loc)
 		{
@@ -14746,7 +14746,8 @@ enable_command (char *args, int from_tty)
 	      update_global_location_list (UGLL_MAY_INSERT);
 	    }
 	  else
-	    map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
+	    map_breakpoint_numbers (num.c_str (), do_map_enable_breakpoint,
+				    NULL);
 	  num = extract_arg (&args);
 	}
     }
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index a00bc52..d5273b5 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -249,36 +249,36 @@ remove_trailing_whitespace (const char *start, const char *s)
 
 /* See documentation in cli-utils.h.  */
 
-char *
+std::string
 extract_arg (const char **arg)
 {
   const char *result;
 
   if (!*arg)
-    return NULL;
+    return std::string ();
 
   /* Find the start of the argument.  */
   *arg = skip_spaces (*arg);
   if (!**arg)
-    return NULL;
+    return std::string ();
   result = *arg;
 
   /* Find the end of the argument.  */
   *arg = skip_to_space (*arg + 1);
 
   if (result == *arg)
-    return NULL;
+    return std::string ();
 
-  return savestring (result, *arg - result);
+  return std::string (result, *arg - result);
 }
 
 /* See documentation in cli-utils.h.  */
 
-char *
+std::string
 extract_arg (char **arg)
 {
   const char *arg_const = *arg;
-  char *result;
+  std::string result;
 
   result = extract_arg (&arg_const);
   *arg += arg_const - *arg;
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index 038ddad..c17b4dd 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -149,17 +149,14 @@ remove_trailing_whitespace (const char *start, char *s)
 }
 
 /* A helper function to extract an argument from *ARG.  An argument is
-   delimited by whitespace.  The return value is either NULL if no
-   argument was found, or an xmalloc'd string.  */
+   delimited by whitespace.  The return value is either empty if no
+   argument was found.  */
 
-extern char *extract_arg (char **arg);
+extern std::string extract_arg (char **arg);
 
-/* A const-correct version of "extract_arg".
+/* A const-correct version of "extract_arg".  */
 
-   Since the returned value is xmalloc'd, it eventually needs to be
-   xfree'ed, which prevents us from making it const as well.  */
-
-extern char *extract_arg (const char **arg);
+extern std::string extract_arg (const char **arg);
 
 /* A helper function that looks for an argument at the start of a
    string.  The argument must also either be at the end of the string,
diff --git a/gdb/demangle.c b/gdb/demangle.c
index 892ad18..5430fb6 100644
--- a/gdb/demangle.c
+++ b/gdb/demangle.c
@@ -170,21 +170,21 @@ demangle_command (char *args, int from_tty)
   std::string arg_buf = args != NULL ? args : "";
   arg_start = arg_buf.c_str ();
 
-  gdb::unique_xmalloc_ptr<char> lang_name;
+  std::string lang_name;
   while (processing_args
 	 && *arg_start == '-')
     {
       const char *p = skip_to_space (arg_start);
 
       if (strncmp (arg_start, "-l", p - arg_start) == 0)
-	lang_name.reset (extract_arg (&p));
+	lang_name = extract_arg (&p);
       else if (strncmp (arg_start, "--", p - arg_start) == 0)
 	processing_args = 0;
       else
 	{
-	  gdb::unique_xmalloc_ptr<char> option (extract_arg (&p));
+	  std::string option = extract_arg (&p);
 	  error (_("Unrecognized option '%s' to demangle command.  "
-		   "Try \"help demangle\"."), option.get ());
+		   "Try \"help demangle\"."), option.c_str ());
 	}
 
       arg_start = skip_spaces (p);
@@ -195,13 +195,13 @@ demangle_command (char *args, int from_tty)
   if (*name == '\0')
     error (_("Usage: demangle [-l language] [--] name"));
 
-  if (lang_name != NULL)
+  if (!lang_name.empty ())
     {
       enum language lang_enum;
 
-      lang_enum = language_enum (lang_name.get ());
+      lang_enum = language_enum (lang_name.c_str ());
       if (lang_enum == language_unknown)
-	error (_("Unknown language \"%s\""), lang_name.get ());
+	error (_("Unknown language \"%s\""), lang_name.c_str ());
       lang = language_def (lang_enum);
     }
   else
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index ad1478e..cf05fa0 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -347,20 +347,14 @@ mi_parse (const char *cmd, char **token)
 	}
       else if (strncmp (chp, "--language ", ls) == 0)
 	{
-	  char *lang_name;
-	  struct cleanup *old_chain;
-
 	  option = "--language";
 	  chp += ls;
-	  lang_name = extract_arg (&chp);
-	  old_chain = make_cleanup (xfree, lang_name);
+	  std::string lang_name = extract_arg (&chp);
 
-	  parse->language = language_enum (lang_name);
+	  parse->language = language_enum (lang_name.c_str ());
 	  if (parse->language == language_unknown
 	      || parse->language == language_auto)
-	    error (_("Invalid --language argument: %s"), lang_name);
-
-	  do_cleanups (old_chain);
+	    error (_("Invalid --language argument: %s"), lang_name.c_str ());
 	}
       else
 	break;
diff --git a/gdb/probe.c b/gdb/probe.c
index 7b3b888..11fc775 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -275,8 +275,8 @@ find_probe_by_pc (CORE_ADDR pc)
    Each argument is a regexp, or NULL, which matches anything.  */
 
 static VEC (bound_probe_s) *
-collect_probes (char *objname, char *provider, char *probe_name,
-		const struct probe_ops *pops)
+collect_probes (const std::string &objname, const std::string &provider,
+		const std::string &probe_name, const struct probe_ops *pops)
 {
   struct objfile *objfile;
   VEC (bound_probe_s) *result = NULL;
@@ -285,12 +285,15 @@ collect_probes (char *objname, char *provider, char *probe_name,
 
   cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result);
 
-  if (provider != NULL)
-    prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp"));
-  if (probe_name != NULL)
-    probe_pat.emplace (probe_name, REG_NOSUB, _("Invalid probe regexp"));
-  if (objname != NULL)
-    obj_pat.emplace (objname, REG_NOSUB, _("Invalid object file regexp"));
+  if (!provider.empty ())
+    prov_pat.emplace (provider.c_str (), REG_NOSUB,
+		      _("Invalid provider regexp"));
+  if (!probe_name.empty ())
+    probe_pat.emplace (probe_name.c_str (), REG_NOSUB,
+		       _("Invalid probe regexp"));
+  if (!objname.empty ())
+    obj_pat.emplace (objname.c_str (), REG_NOSUB,
+		     _("Invalid object file regexp"));
 
   ALL_OBJFILES (objfile)
     {
@@ -301,7 +304,7 @@ collect_probes (char *objname, char *provider, char *probe_name,
       if (! objfile->sf || ! objfile->sf->sym_probe_fns)
 	continue;
 
-      if (objname)
+      if (obj_pat)
 	{
 	  if (obj_pat->exec (objfile_name (objfile), 0, NULL, 0) != 0)
 	    continue;
@@ -316,11 +319,11 @@ collect_probes (char *objname, char *provider, char *probe_name,
 	  if (pops != NULL && probe->pops != pops)
 	    continue;
 
-	  if (provider
+	  if (prov_pat
 	      && prov_pat->exec (probe->provider, 0, NULL, 0) != 0)
 	    continue;
 
-	  if (probe_name
+	  if (probe_pat
 	      && probe_pat->exec (probe->name, 0, NULL, 0) != 0)
 	    continue;
 
@@ -553,16 +556,14 @@ exists_probe_with_pops (VEC (bound_probe_s) *probes,
    [PROBE [OBJNAME]]] from the provided string STR.  */
 
 static void
-parse_probe_linespec (const char *str, char **provider,
-		      char **probe_name, char **objname)
+parse_probe_linespec (const char *str, std::string *provider,
+		      std::string *probe_name, std::string *objname)
 {
-  *probe_name = *objname = NULL;
-
   *provider = extract_arg (&str);
-  if (*provider != NULL)
+  if (!provider->empty ())
     {
       *probe_name = extract_arg (&str);
-      if (*probe_name != NULL)
+      if (!probe_name->empty ())
 	*objname = extract_arg (&str);
     }
 }
@@ -573,7 +574,7 @@ void
 info_probes_for_ops (const char *arg, int from_tty,
 		     const struct probe_ops *pops)
 {
-  char *provider, *probe_name = NULL, *objname = NULL;
+  std::string provider, probe_name, objname;
   struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
   VEC (bound_probe_s) *probes;
   int i, any_found;
@@ -587,9 +588,6 @@ info_probes_for_ops (const char *arg, int from_tty,
   struct gdbarch *gdbarch = get_current_arch ();
 
   parse_probe_linespec (arg, &provider, &probe_name, &objname);
-  make_cleanup (xfree, provider);
-  make_cleanup (xfree, probe_name);
-  make_cleanup (xfree, objname);
 
   probes = collect_probes (objname, provider, probe_name, pops);
   make_cleanup (VEC_cleanup (probe_p), &probes);
@@ -721,16 +719,13 @@ info_probes_command (char *arg, int from_tty)
 static void
 enable_probes_command (char *arg, int from_tty)
 {
-  char *provider, *probe_name = NULL, *objname = NULL;
+  std::string provider, probe_name, objname;
   struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
   VEC (bound_probe_s) *probes;
   struct bound_probe *probe;
   int i;
 
   parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname);
-  make_cleanup (xfree, provider);
-  make_cleanup (xfree, probe_name);
-  make_cleanup (xfree, objname);
 
   probes = collect_probes (objname, provider, probe_name, NULL);
   if (VEC_empty (bound_probe_s, probes))
@@ -765,16 +760,13 @@ enable_probes_command (char *arg, int from_tty)
 static void
 disable_probes_command (char *arg, int from_tty)
 {
-  char *provider, *probe_name = NULL, *objname = NULL;
+  std::string provider, probe_name, objname;
   struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
   VEC (bound_probe_s) *probes;
   struct bound_probe *probe;
   int i;
 
   parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname);
-  make_cleanup (xfree, provider);
-  make_cleanup (xfree, probe_name);
-  make_cleanup (xfree, objname);
 
   probes = collect_probes (objname, provider, probe_name, NULL /* pops */);
   if (VEC_empty (bound_probe_s, probes))
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 8d8dc3c..d730fda 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2017-09-10  Tom Tromey  <tom@tromey.com>
+
+	* gdb.base/ena-dis-br.exp (test_ena_dis_br): Update test.
+
 2017-09-08  Christoph Weinmann  <christoph.t.weinmann@intel.com>
 
 	* gdb.fortran/printing-types.exp: New file.
diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
index 8abca35..d407408 100644
--- a/gdb/testsuite/gdb.base/ena-dis-br.exp
+++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
@@ -369,7 +369,7 @@ proc test_ena_dis_br { what } {
 
     # Now enable(disable) $b1 fooo.1, it should give error on fooo.
     gdb_test "$what $b1 fooo.1" \
-       "Bad breakpoint number 'fooo'" \
+       "Bad breakpoint number 'fooo\\.1'" \
        "$what \$b1 fooo.1"
 
     # $b1 should be enabled(disabled).
-- 
2.9.4