[2/2] Use unique_xmalloc_ptr in explicit_location_spec

Message ID 20231210-location-stuff-v1-2-9b6ae74fe5b0@tromey.com
State New
Headers
Series Use unique_xmalloc_ptr in location.[ch] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Dec. 10, 2023, 3:20 p.m. UTC
  This changes explicit_location_spec to use unique_xmalloc_ptr,
removing some manual memory management.
---
 gdb/breakpoint.c           |  5 ++--
 gdb/completer.c            | 14 +++++----
 gdb/linespec.c             | 73 +++++++++++++++++++++++++---------------------
 gdb/location.c             | 30 ++++++++-----------
 gdb/location.h             | 24 +++++++--------
 gdb/mi/mi-cmd-break.c      |  6 ++--
 gdb/python/py-breakpoint.c | 12 ++++----
 7 files changed, 83 insertions(+), 81 deletions(-)
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 957bef290f2..bd7f74671ce 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12890,9 +12890,8 @@  update_static_tracepoint (tracepoint *tp, struct symtab_and_line sal)
 	  tp->first_loc ().symtab = sym != NULL ? sal2.symtab : NULL;
 
 	  std::unique_ptr<explicit_location_spec> els
-	    (new explicit_location_spec ());
-	  els->source_filename
-	    = xstrdup (symtab_to_filename_for_display (sal2.symtab));
+	    (new explicit_location_spec
+	     (symtab_to_filename_for_display (sal2.symtab)));
 	  els->line_offset.offset = tp->first_loc ().line_number;
 	  els->line_offset.sign = LINE_OFFSET_NONE;
 
diff --git a/gdb/completer.c b/gdb/completer.c
index d5799c0d1ff..d69ddcceca9 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -725,7 +725,8 @@  collect_explicit_location_matches (completion_tracker &tracker,
     {
     case MATCH_SOURCE:
       {
-	const char *source = string_or_empty (explicit_loc->source_filename);
+	const char *source
+	  = string_or_empty (explicit_loc->source_filename.get ());
 	completion_list matches
 	  = make_source_files_completion_list (source, source);
 	tracker.add_completions (std::move (matches));
@@ -734,10 +735,11 @@  collect_explicit_location_matches (completion_tracker &tracker,
 
     case MATCH_FUNCTION:
       {
-	const char *function = string_or_empty (explicit_loc->function_name);
+	const char *function
+	  = string_or_empty (explicit_loc->function_name.get ());
 	linespec_complete_function (tracker, function,
 				    explicit_loc->func_name_match_type,
-				    explicit_loc->source_filename);
+				    explicit_loc->source_filename.get ());
       }
       break;
 
@@ -750,10 +752,10 @@  collect_explicit_location_matches (completion_tracker &tracker,
 
     case MATCH_LABEL:
       {
-	const char *label = string_or_empty (explicit_loc->label_name);
+	const char *label = string_or_empty (explicit_loc->label_name.get ());
 	linespec_complete_label (tracker, language,
-				 explicit_loc->source_filename,
-				 explicit_loc->function_name,
+				 explicit_loc->source_filename.get (),
+				 explicit_loc->function_name.get (),
 				 explicit_loc->func_name_match_type,
 				 label);
       }
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1643174de88..76a93d7a35b 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1786,7 +1786,7 @@  linespec_parse_basic (linespec_parser *parser)
 	{
 	  completion_tracker tmp_tracker;
 	  const char *source_filename
-	    = PARSER_EXPLICIT (parser)->source_filename;
+	    = PARSER_EXPLICIT (parser)->source_filename.get ();
 	  symbol_name_match_type match_type
 	    = PARSER_EXPLICIT (parser)->func_name_match_type;
 
@@ -1806,7 +1806,7 @@  linespec_parse_basic (linespec_parser *parser)
 	    }
 	}
 
-      PARSER_EXPLICIT (parser)->function_name = name.release ();
+      PARSER_EXPLICIT (parser)->function_name = std::move (name);
     }
   else
     {
@@ -1823,7 +1823,7 @@  linespec_parse_basic (linespec_parser *parser)
 	{
 	  PARSER_RESULT (parser)->function_symbols = std::move (symbols);
 	  PARSER_RESULT (parser)->minimal_symbols = std::move (minimal_symbols);
-	  PARSER_EXPLICIT (parser)->function_name = name.release ();
+	  PARSER_EXPLICIT (parser)->function_name = std::move (name);
 	}
       else
 	{
@@ -1838,7 +1838,7 @@  linespec_parse_basic (linespec_parser *parser)
 	      PARSER_RESULT (parser)->labels.label_symbols = std::move (labels);
 	      PARSER_RESULT (parser)->labels.function_symbols
 		  = std::move (symbols);
-	      PARSER_EXPLICIT (parser)->label_name = name.release ();
+	      PARSER_EXPLICIT (parser)->label_name = std::move (name);
 	    }
 	  else if (token.type == LSTOKEN_STRING
 		   && *LS_TOKEN_STOKEN (token).ptr == '$')
@@ -1851,7 +1851,7 @@  linespec_parse_basic (linespec_parser *parser)
 		{
 		  /* The user-specified variable was not valid.  Do not
 		     throw an error here.  parse_linespec will do it for us.  */
-		  PARSER_EXPLICIT (parser)->function_name = name.release ();
+		  PARSER_EXPLICIT (parser)->function_name = std::move (name);
 		  return;
 		}
 	    }
@@ -1861,7 +1861,7 @@  linespec_parse_basic (linespec_parser *parser)
 		 an error here.  parse_linespec will do it for us.  */
 
 	      /* Save a copy of the name we were trying to lookup.  */
-	      PARSER_EXPLICIT (parser)->function_name = name.release ();
+	      PARSER_EXPLICIT (parser)->function_name = std::move (name);
 	      return;
 	    }
 	}
@@ -1947,13 +1947,14 @@  linespec_parse_basic (linespec_parser *parser)
 		    = std::move (labels);
 		  PARSER_RESULT (parser)->labels.function_symbols
 		    = std::move (symbols);
-		  PARSER_EXPLICIT (parser)->label_name = name.release ();
+		  PARSER_EXPLICIT (parser)->label_name = std::move (name);
 		}
 	      else
 		{
 		  /* We don't know what it was, but it isn't a label.  */
 		  undefined_label_error
-		    (PARSER_EXPLICIT (parser)->function_name, name.get ());
+		    (PARSER_EXPLICIT (parser)->function_name.get (),
+		     name.get ());
 		}
 
 	    }
@@ -2012,7 +2013,8 @@  canonicalize_linespec (struct linespec_state *state, const linespec *ls)
 	  /* No function was specified, so add the symbol name.  */
 	  gdb_assert (ls->labels.function_symbols.size () == 1);
 	  block_symbol s = ls->labels.function_symbols.front ();
-	  explicit_loc->function_name = xstrdup (s.symbol->natural_name ());
+	  explicit_loc->function_name
+	    = make_unique_xstrdup (s.symbol->natural_name ());
 	}
     }
 
@@ -2143,7 +2145,7 @@  create_sals_line_offset (struct linespec_state *self,
     {
       if (ls->explicit_loc.source_filename)
 	throw_error (NOT_FOUND_ERROR, _("No line %d in file \"%s\"."),
-		     val.line, ls->explicit_loc.source_filename);
+		     val.line, ls->explicit_loc.source_filename.get ());
       else
 	throw_error (NOT_FOUND_ERROR, _("No line %d in the current file."),
 		     val.line);
@@ -2289,7 +2291,7 @@  convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	       form so that displaying SOURCE_FILENAME can follow the current
 	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
 	       it has been kept for code simplicity only in absolute form.  */
-	    ls->explicit_loc.source_filename = xstrdup (filename);
+	    ls->explicit_loc.source_filename = make_unique_xstrdup (filename);
 	  }
     }
   else
@@ -2334,7 +2336,8 @@  convert_explicit_location_spec_to_linespec
 	{
 	  source_file_not_found_error (source_filename);
 	}
-      result->explicit_loc.source_filename = xstrdup (source_filename);
+      result->explicit_loc.source_filename
+	= make_unique_xstrdup (source_filename);
     }
   else
     {
@@ -2352,9 +2355,10 @@  convert_explicit_location_spec_to_linespec
 
       if (symbols.empty () && minimal_symbols.empty ())
 	symbol_not_found_error (function_name,
-				result->explicit_loc.source_filename);
+				result->explicit_loc.source_filename.get ());
 
-      result->explicit_loc.function_name = xstrdup (function_name);
+      result->explicit_loc.function_name
+	= make_unique_xstrdup (function_name);
       result->function_symbols = std::move (symbols);
       result->minimal_symbols = std::move (minimal_symbols);
     }
@@ -2367,10 +2371,10 @@  convert_explicit_location_spec_to_linespec
 			      &symbols, label_name);
 
       if (labels.empty ())
-	undefined_label_error (result->explicit_loc.function_name,
+	undefined_label_error (result->explicit_loc.function_name.get (),
 			       label_name);
 
-      result->explicit_loc.label_name = xstrdup (label_name);
+      result->explicit_loc.label_name = make_unique_xstrdup (label_name);
       result->labels.label_symbols = labels;
       result->labels.function_symbols = std::move (symbols);
     }
@@ -2388,10 +2392,10 @@  convert_explicit_location_spec_to_sals
    const explicit_location_spec *explicit_spec)
 {
   convert_explicit_location_spec_to_linespec (self, result,
-					      explicit_spec->source_filename,
-					      explicit_spec->function_name,
+					      explicit_spec->source_filename.get (),
+					      explicit_spec->function_name.get (),
 					      explicit_spec->func_name_match_type,
-					      explicit_spec->label_name,
+					      explicit_spec->label_name.get (),
 					      explicit_spec->line_offset);
   return convert_linespec_to_sals (self, result);
 }
@@ -2565,7 +2569,7 @@  parse_linespec (linespec_parser *parser, const char *arg,
       if (file_exception.reason >= 0)
 	{
 	  /* Symtabs were found for the file.  Record the filename.  */
-	  PARSER_EXPLICIT (parser)->source_filename = user_filename.release ();
+	  PARSER_EXPLICIT (parser)->source_filename = std::move (user_filename);
 
 	  /* Get the next token.  */
 	  token = linespec_lexer_consume_token (parser);
@@ -2610,8 +2614,9 @@  parse_linespec (linespec_parser *parser, const char *arg,
 	throw_exception (std::move (file_exception));
 
       /* Otherwise, the symbol is not found.  */
-      symbol_not_found_error (PARSER_EXPLICIT (parser)->function_name,
-			      PARSER_EXPLICIT (parser)->source_filename);
+      symbol_not_found_error
+	(PARSER_EXPLICIT (parser)->function_name.get (),
+	 PARSER_EXPLICIT (parser)->source_filename.get ());
     }
 
  convert_to_sals:
@@ -2928,7 +2933,7 @@  linespec_complete (completion_tracker &tracker, const char *text,
     {
       parser.complete_what = linespec_complete_what::NOTHING;
 
-      const char *func_name = PARSER_EXPLICIT (&parser)->function_name;
+      const char *func_name = PARSER_EXPLICIT (&parser)->function_name.get ();
 
       std::vector<block_symbol> function_symbols;
       std::vector<bound_minimal_symbol> minimal_symbols;
@@ -2985,10 +2990,11 @@  linespec_complete (completion_tracker &tracker, const char *text,
 
       const char *word = parser.completion_word;
 
-      complete_linespec_component (&parser, tracker,
-				   parser.completion_word,
-				   linespec_complete_what::FUNCTION,
-				   PARSER_EXPLICIT (&parser)->source_filename);
+      complete_linespec_component
+	(&parser, tracker,
+	 parser.completion_word,
+	 linespec_complete_what::FUNCTION,
+	 PARSER_EXPLICIT (&parser)->source_filename.get ());
 
       parser.complete_what = linespec_complete_what::NOTHING;
 
@@ -3028,10 +3034,11 @@  linespec_complete (completion_tracker &tracker, const char *text,
 
   tracker.advance_custom_word_point_by (parser.completion_word - orig);
 
-  complete_linespec_component (&parser, tracker,
-			       parser.completion_word,
-			       parser.complete_what,
-			       PARSER_EXPLICIT (&parser)->source_filename);
+  complete_linespec_component
+    (&parser, tracker,
+     parser.completion_word,
+     parser.complete_what,
+     PARSER_EXPLICIT (&parser)->source_filename.get ());
 
   /* If we're past the "filename:function:label:offset" linespec, and
      didn't find any match, then assume the user might want to create
@@ -3335,7 +3342,7 @@  decode_objc (struct linespec_state *self, linespec *ls, const char *arg)
       memcpy (saved_arg, arg, new_argptr - arg);
       saved_arg[new_argptr - arg] = '\0';
 
-      ls->explicit_loc.function_name = xstrdup (saved_arg);
+      ls->explicit_loc.function_name = make_unique_xstrdup (saved_arg);
       ls->function_symbols = std::move (symbols);
       ls->minimal_symbols = std::move (minimal_symbols);
       values = convert_linespec_to_sals (self, ls);
@@ -3350,7 +3357,7 @@  decode_objc (struct linespec_state *self, linespec *ls, const char *arg)
 	  if (ls->explicit_loc.source_filename)
 	    {
 	      holder = string_printf ("%s:%s",
-				      ls->explicit_loc.source_filename,
+				      ls->explicit_loc.source_filename.get (),
 				      saved_arg);
 	      str = holder.c_str ();
 	    }
diff --git a/gdb/location.c b/gdb/location.c
index df9a7d575da..876f13881f6 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -156,25 +156,19 @@  address_location_spec::compute_string () const
   return std::string ("*") + addr_string;
 }
 
-explicit_location_spec::explicit_location_spec ()
-  : location_spec (EXPLICIT_LOCATION_SPEC)
+explicit_location_spec::explicit_location_spec (const char *function_name)
+  : location_spec (EXPLICIT_LOCATION_SPEC),
+    function_name (maybe_xstrdup (function_name))
 {
 }
 
-explicit_location_spec::~explicit_location_spec ()
-{
-  xfree (source_filename);
-  xfree (function_name);
-  xfree (label_name);
-}
-
 explicit_location_spec::explicit_location_spec
   (const explicit_location_spec &other)
   : location_spec (other),
-    source_filename (maybe_xstrdup (other.source_filename)),
-    function_name (maybe_xstrdup (other.function_name)),
+    source_filename (maybe_xstrdup (other.source_filename.get ())),
+    function_name (maybe_xstrdup (other.function_name.get ())),
     func_name_match_type (other.func_name_match_type),
-    label_name (maybe_xstrdup (other.label_name)),
+    label_name (maybe_xstrdup (other.label_name.get ())),
     line_offset (other.line_offset)
 {
 }
@@ -291,7 +285,7 @@  explicit_to_string_internal (bool as_linespec,
     {
       if (!as_linespec)
 	buf.puts ("-source ");
-      buf.puts (explicit_loc->source_filename);
+      buf.puts (explicit_loc->source_filename.get ());
       need_space = true;
     }
 
@@ -303,7 +297,7 @@  explicit_to_string_internal (bool as_linespec,
 	buf.puts ("-qualified ");
       if (!as_linespec)
 	buf.puts ("-function ");
-      buf.puts (explicit_loc->function_name);
+      buf.puts (explicit_loc->function_name.get ());
       need_space = true;
     }
 
@@ -313,7 +307,7 @@  explicit_to_string_internal (bool as_linespec,
 	buf.putc (space);
       if (!as_linespec)
 	buf.puts ("-label ");
-      buf.puts (explicit_loc->label_name);
+      buf.puts (explicit_loc->label_name.get ());
       need_space = true;
     }
 
@@ -705,13 +699,13 @@  string_to_explicit_location_spec (const char **argp,
 	{
 	  set_oarg (explicit_location_spec_lex_one (argp, language,
 						    completion_info));
-	  locspec->source_filename = oarg.release ();
+	  locspec->source_filename = std::move (oarg);
 	}
       else if (strncmp (opt.get (), "-function", len) == 0)
 	{
 	  set_oarg (explicit_location_spec_lex_one_function (argp, language,
 							     completion_info));
-	  locspec->function_name = oarg.release ();
+	  locspec->function_name = std::move (oarg);
 	}
       else if (strncmp (opt.get (), "-qualified", len) == 0)
 	{
@@ -731,7 +725,7 @@  string_to_explicit_location_spec (const char **argp,
 	{
 	  set_oarg (explicit_location_spec_lex_one (argp, language,
 						    completion_info));
-	  locspec->label_name = oarg.release ();
+	  locspec->label_name = std::move (oarg);
 	}
       /* Only emit an "invalid argument" error for options
 	 that look like option strings.  */
diff --git a/gdb/location.h b/gdb/location.h
index e39a948f680..acbc2860f2d 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -190,9 +190,12 @@  struct address_location_spec : public location_spec
 
 struct explicit_location_spec : public location_spec
 {
-  explicit_location_spec ();
+  explicit explicit_location_spec (const char *function_name);
 
-  ~explicit_location_spec ();
+  explicit_location_spec ()
+    : explicit_location_spec (nullptr)
+  {
+  }
 
   location_spec_up clone () const override;
 
@@ -203,18 +206,18 @@  struct explicit_location_spec : public location_spec
      canonicalized/valid.  */
   std::string to_linespec () const;
 
-  /* The source filename. Malloc'd.  */
-  char *source_filename = nullptr;
+  /* The source filename.  */
+  gdb::unique_xmalloc_ptr<char> source_filename;
 
-  /* The function name.  Malloc'd.  */
-  char *function_name = nullptr;
+  /* The function name.  */
+  gdb::unique_xmalloc_ptr<char> function_name;
 
   /* Whether the function name is fully-qualified or not.  */
   symbol_name_match_type func_name_match_type
     = symbol_name_match_type::WILD;
 
-  /* The name of a label.  Malloc'd.  */
-  char *label_name = nullptr;
+  /* The name of a label.  */
+  gdb::unique_xmalloc_ptr<char> label_name;
 
   /* A line offset relative to the start of the symbol
      identified by the above fields or the current symtab
@@ -284,10 +287,7 @@  const probe_location_spec *
 static inline location_spec_up
 new_explicit_location_spec_function (const char *function_name)
 {
-  explicit_location_spec *spec
-    = new explicit_location_spec ();
-  spec->function_name
-    = (function_name != nullptr ? xstrdup (function_name) : nullptr);
+  explicit_location_spec *spec = new explicit_location_spec (function_name);
   return location_spec_up (spec);
 }
 
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 44835c7f7d0..a4b62cb6f99 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -267,15 +267,15 @@  mi_cmd_break_insert_1 (int dprintf, const char *command,
 	  break;
 	case EXPLICIT_SOURCE_OPT:
 	  is_explicit = 1;
-	  explicit_loc->source_filename = xstrdup (oarg);
+	  explicit_loc->source_filename = make_unique_xstrdup (oarg);
 	  break;
 	case EXPLICIT_FUNC_OPT:
 	  is_explicit = 1;
-	  explicit_loc->function_name = xstrdup (oarg);
+	  explicit_loc->function_name = make_unique_xstrdup (oarg);
 	  break;
 	case EXPLICIT_LABEL_OPT:
 	  is_explicit = 1;
-	  explicit_loc->label_name = xstrdup (oarg);
+	  explicit_loc->label_name = make_unique_xstrdup (oarg);
 	  break;
 	case EXPLICIT_LINE_OPT:
 	  is_explicit = 1;
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 71182cc1b1b..5155d41e675 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1010,12 +1010,12 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 		std::unique_ptr<explicit_location_spec> explicit_loc
 		  (new explicit_location_spec ());
 
-		explicit_loc->source_filename
-		  = source != nullptr ? xstrdup (source) : nullptr;
-		explicit_loc->function_name
-		  = function != nullptr ? xstrdup (function) : nullptr;
-		explicit_loc->label_name
-		  = label != nullptr ? xstrdup (label) : nullptr;
+		if (source != nullptr)
+		  explicit_loc->source_filename = make_unique_xstrdup (source);
+		if (function != nullptr)
+		  explicit_loc->function_name = make_unique_xstrdup (function);
+		if (label != nullptr)
+		  explicit_loc->label_name = make_unique_xstrdup (label);
 
 		if (line != NULL)
 		  explicit_loc->line_offset