diff mbox series

[3/6] Use std::vector in linespec_state

Message ID 20250108-linespec-state-cxx-v1-3-a721e95ee050@tromey.com
State New
Headers
Series More linespec cleanups and C++-ification |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply

Commit Message

Tom Tromey Jan. 8, 2025, 7:23 a.m. UTC 
  This changes linespec_state to use a std::vector, and changes
linespec_canonical_name to use std::string.  This removes some manual
memory management, including some odd cleanup code in in
decode_line_full.
---
 gdb/linespec.c | 38 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

Comments

Simon Marchi Jan. 9, 2025, 3:29 a.m. UTC | #1 
> @@ -1111,9 +1106,7 @@ add_sal_to_sals (struct linespec_state *self,
>      {
>        struct linespec_canonical_name *canonical;
>  
> -      self->canonical_names = XRESIZEVEC (struct linespec_canonical_name,
> -					  self->canonical_names,
> -					  sals->size ());
> +      self->canonical_names.resize (sals->size ());
>        canonical = &self->canonical_names[sals->size () - 1];

This follows the old strategy for appending an item, but now that it's
an std::vector, it would now be clearer to do:

  auto &canonical = self->canonical_names.emplace_back ();

Or, keep suffix and symtab as local variables and append the new value
afterwards.

Otherwise, LGTM.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon
diff mbox series

Patch

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 689a8791d8561098a435262a32cc6883bfd38abf..053af6f9bb595294bc9a512a9f465ddc3ff73f65 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -126,7 +126,7 @@  struct linespec
 struct linespec_canonical_name
 {
   /* Remaining text part of the linespec string.  */
-  char *suffix;
+  std::string suffix;
 
   /* If NULL then SUFFIX is the whole linespec string.  */
   struct symtab *symtab;
@@ -181,11 +181,6 @@  struct linespec_state
   {
   }
 
-  ~linespec_state ()
-  {
-    xfree (canonical_names);
-  }
-
   /* Add ADDR to the address set.  Return true if this is a new
      entry.  */
   bool add_address (program_space *pspace, CORE_ADDR addr)
@@ -221,7 +216,7 @@  struct linespec_state
   struct linespec_result *canonical;
 
   /* Canonical strings that mirror the std::vector<symtab_and_line> result.  */
-  struct linespec_canonical_name *canonical_names = nullptr;
+  std::vector<linespec_canonical_name> canonical_names;
 
   /* Are we building a linespec?  */
   int is_linespec = 0;
@@ -1111,9 +1106,7 @@  add_sal_to_sals (struct linespec_state *self,
     {
       struct linespec_canonical_name *canonical;
 
-      self->canonical_names = XRESIZEVEC (struct linespec_canonical_name,
-					  self->canonical_names,
-					  sals->size ());
+      self->canonical_names.resize (sals->size ());
       canonical = &self->canonical_names[sals->size () - 1];
       if (!literal_canonical && sal->symtab)
 	{
@@ -1124,20 +1117,20 @@  add_sal_to_sals (struct linespec_state *self,
 	     the time being.  */
 	  if (symname != NULL && sal->line != 0
 	      && self->language->la_language == language_ada)
-	    canonical->suffix = xstrprintf ("%s:%d", symname,
-					    sal->line).release ();
+	    canonical->suffix = string_printf ("%s:%d", symname,
+					       sal->line);
 	  else if (symname != NULL)
-	    canonical->suffix = xstrdup (symname);
+	    canonical->suffix = symname;
 	  else
-	    canonical->suffix = xstrprintf ("%d", sal->line).release ();
+	    canonical->suffix = string_printf ("%d", sal->line);
 	  canonical->symtab = sal->symtab;
 	}
       else
 	{
 	  if (symname != NULL)
-	    canonical->suffix = xstrdup (symname);
+	    canonical->suffix = symname;
 	  else
-	    canonical->suffix = xstrdup ("<unknown>");
+	    canonical->suffix = "<unknown>";
 	  canonical->symtab = NULL;
 	}
     }
@@ -1333,7 +1326,7 @@  canonical_to_fullform (const struct linespec_canonical_name *canonical)
     return canonical->suffix;
   else
     return string_printf ("%s:%s", symtab_to_fullname (canonical->symtab),
-			  canonical->suffix);
+			  canonical->suffix.c_str ());
 }
 
 /* Given FILTERS, a list of canonical names, filter the sals in RESULT
@@ -1444,7 +1437,6 @@  decode_line_2 (struct linespec_state *self,
       std::string displayform;
 
       canonical = &self->canonical_names[i];
-      gdb_assert (canonical->suffix != NULL);
 
       std::string fullform = canonical_to_fullform (canonical);
 
@@ -1456,7 +1448,7 @@  decode_line_2 (struct linespec_state *self,
 
 	  fn_for_display = symtab_to_filename_for_display (canonical->symtab);
 	  displayform = string_printf ("%s:%s", fn_for_display,
-				       canonical->suffix);
+				       canonical->suffix.c_str ());
 	}
 
       items.emplace_back (std::move (fullform), std::move (displayform),
@@ -3156,14 +3148,6 @@  decode_line_full (struct location_spec *locspec, int flags,
   gdb_assert (result.size () == 1 || canonical->pre_expanded);
   canonical->pre_expanded = 1;
 
-  /* Arrange for allocated canonical names to be freed.  */
-  std::vector<gdb::unique_xmalloc_ptr<char>> hold_names;
-  for (int i = 0; i < result.size (); ++i)
-    {
-      gdb_assert (state->canonical_names[i].suffix != NULL);
-      hold_names.emplace_back (state->canonical_names[i].suffix);
-    }
-
   if (select_mode == NULL)
     {
       if (top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())