public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA v2 07/10] Change streq to return bool
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
@ 2018-04-04  4:40 ` Tom Tromey
  2018-04-04  4:40 ` [RFA v2 02/10] Fix some indentation in linespec.c Tom Tromey
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I wanted to use streq with std::unique in another (upcoming) patch in
this seres, so I changed it to return bool.  To my surprise, this lead
to regressions.  The cause turned out to be that streq was used as an
htab callback -- by casting it to the correct function type.  This
sort of cast is invalid, so this patch adds a variant which is
directly suitable for use by htab.  (Note that I did not add an
overload, as I could not get that to work with template deduction in
the other patch.)

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* completer.c (completion_tracker::completion_tracker): Remove
	cast.
	(completion_tracker::discard_completions): Likewise.
	* breakpoint.c (ambiguous_names_p): Remove cast.
	* ada-lang.c (_initialize_ada_language): Remove cast.
	* utils.h (streq): Update.
	(streq_hash): Add new declaration.
	* utils.c (streq): Return bool.
	(streq_hash): New function.
---
 gdb/ChangeLog    | 12 ++++++++++++
 gdb/ada-lang.c   |  5 ++---
 gdb/breakpoint.c |  6 ++----
 gdb/completer.c  |  4 ++--
 gdb/utils.c      | 13 +++++++++++--
 gdb/utils.h      |  9 ++++++++-
 6 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 11939d7798..de20c43bed 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14722,9 +14722,8 @@ When enabled, the debugger will stop using the DW_AT_GNAT_descriptive_type\n\
 DWARF attribute."),
      NULL, NULL, &maint_set_ada_cmdlist, &maint_show_ada_cmdlist);
 
-  decoded_names_store = htab_create_alloc
-    (256, htab_hash_string, (int (*)(const void *, const void *)) streq,
-     NULL, xcalloc, xfree);
+  decoded_names_store = htab_create_alloc (256, htab_hash_string, streq_hash,
+					   NULL, xcalloc, xfree);
 
   /* The ada-lang observers.  */
   gdb::observers::new_objfile.attach (ada_new_objfile_observer);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 991c29c1e2..f84fef2bea 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13384,10 +13384,8 @@ static int
 ambiguous_names_p (struct bp_location *loc)
 {
   struct bp_location *l;
-  htab_t htab = htab_create_alloc (13, htab_hash_string,
-				   (int (*) (const void *, 
-					     const void *)) streq,
-				   NULL, xcalloc, xfree);
+  htab_t htab = htab_create_alloc (13, htab_hash_string, streq_hash, NULL,
+				   xcalloc, xfree);
 
   for (l = loc; l != NULL; l = l->next)
     {
diff --git a/gdb/completer.c b/gdb/completer.c
index 4de1bcff32..769dcf0eb6 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1469,7 +1469,7 @@ int max_completions = 200;
 completion_tracker::completion_tracker ()
 {
   m_entries_hash = htab_create_alloc (INITIAL_COMPLETION_HTAB_SIZE,
-				      htab_hash_string, (htab_eq) streq,
+				      htab_hash_string, streq_hash,
 				      NULL, xcalloc, xfree);
 }
 
@@ -1487,7 +1487,7 @@ completion_tracker::discard_completions ()
 
   htab_delete (m_entries_hash);
   m_entries_hash = htab_create_alloc (INITIAL_COMPLETION_HTAB_SIZE,
-				      htab_hash_string, (htab_eq) streq,
+				      htab_hash_string, streq_hash,
 				      NULL, xcalloc, xfree);
 }
 
diff --git a/gdb/utils.c b/gdb/utils.c
index ee19fed172..bd7553e5f4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2601,13 +2601,22 @@ strcmp_iw_ordered (const char *string1, const char *string2)
     }
 }
 
-/* A simple comparison function with opposite semantics to strcmp.  */
+/* See utils.h.  */
 
-int
+bool
 streq (const char *lhs, const char *rhs)
 {
   return !strcmp (lhs, rhs);
 }
+
+/* See utils.h.  */
+
+int
+streq_hash (const void *lhs, const void *rhs)
+{
+  return streq ((const char *) lhs, (const char *) rhs);
+}
+
 \f
 
 /*
diff --git a/gdb/utils.h b/gdb/utils.h
index 0de0fe2baa..d3b8871b58 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -89,7 +89,14 @@ extern int strcmp_iw (const char *string1, const char *string2);
 
 extern int strcmp_iw_ordered (const char *, const char *);
 
-extern int streq (const char *, const char *);
+/* Return true if the strings are equal.  */
+
+extern bool streq (const char *, const char *);
+
+/* A variant of streq that is suitable for use as an htab
+   callback.  */
+
+extern int streq_hash (const void *, const void *);
 
 extern int subset_compare (const char *, const char *);
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 02/10] Fix some indentation in linespec.c
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
  2018-04-04  4:40 ` [RFA v2 07/10] Change streq to return bool Tom Tromey
@ 2018-04-04  4:40 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes some leftover comments and fixes the indentation in a
couple of spots in linespec.c.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.c (linespec_parse_basic): Reindent.
---
 gdb/ChangeLog  |   4 ++
 gdb/linespec.c | 137 ++++++++++++++++++++++++++++-----------------------------
 2 files changed, 71 insertions(+), 70 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 31ebca37c4..a081a8bebf 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1897,68 +1897,66 @@ linespec_parse_basic (linespec_parser *parser)
     }
   else
     {
-      /* XXX Reindent before pushing.  */
-
-  /* Try looking it up as a function/method.  */
-  find_linespec_symbols (PARSER_STATE (parser),
-			 PARSER_RESULT (parser)->file_symtabs, name,
-			 PARSER_EXPLICIT (parser)->func_name_match_type,
-			 &symbols, &minimal_symbols);
+      /* Try looking it up as a function/method.  */
+      find_linespec_symbols (PARSER_STATE (parser),
+			     PARSER_RESULT (parser)->file_symtabs, name,
+			     PARSER_EXPLICIT (parser)->func_name_match_type,
+			     &symbols, &minimal_symbols);
 
-  if (symbols != NULL || minimal_symbols != NULL)
-    {
-      PARSER_RESULT (parser)->function_symbols = symbols;
-      PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
-      PARSER_EXPLICIT (parser)->function_name = name;
-      symbols = NULL;
-      discard_cleanups (cleanup);
-    }
-  else
-    {
-      /* NAME was not a function or a method.  So it must be a label
-	 name or user specified variable like "break foo.c:$zippo".  */
-      labels = find_label_symbols (PARSER_STATE (parser), NULL,
-				   &symbols, name);
-      if (labels != NULL)
+      if (symbols != NULL || minimal_symbols != NULL)
 	{
-	  PARSER_RESULT (parser)->labels.label_symbols = labels;
-	  PARSER_RESULT (parser)->labels.function_symbols = symbols;
-	  PARSER_EXPLICIT (parser)->label_name = name;
+	  PARSER_RESULT (parser)->function_symbols = symbols;
+	  PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
+	  PARSER_EXPLICIT (parser)->function_name = name;
 	  symbols = NULL;
 	  discard_cleanups (cleanup);
 	}
-      else if (token.type == LSTOKEN_STRING
-	       && *LS_TOKEN_STOKEN (token).ptr == '$')
+      else
 	{
-	  /* User specified a convenience variable or history value.  */
-	  PARSER_EXPLICIT (parser)->line_offset
-	    = linespec_parse_variable (PARSER_STATE (parser), name);
+	  /* NAME was not a function or a method.  So it must be a label
+	     name or user specified variable like "break foo.c:$zippo".  */
+	  labels = find_label_symbols (PARSER_STATE (parser), NULL,
+				       &symbols, name);
+	  if (labels != NULL)
+	    {
+	      PARSER_RESULT (parser)->labels.label_symbols = labels;
+	      PARSER_RESULT (parser)->labels.function_symbols = symbols;
+	      PARSER_EXPLICIT (parser)->label_name = name;
+	      symbols = NULL;
+	      discard_cleanups (cleanup);
+	    }
+	  else if (token.type == LSTOKEN_STRING
+		   && *LS_TOKEN_STOKEN (token).ptr == '$')
+	    {
+	      /* User specified a convenience variable or history value.  */
+	      PARSER_EXPLICIT (parser)->line_offset
+		= linespec_parse_variable (PARSER_STATE (parser), name);
 
-	  if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
+	      if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
+		{
+		  /* 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;
+		  discard_cleanups (cleanup);
+		  return;
+		}
+
+	      /* The convenience variable/history value parsed correctly.
+		 NAME is no longer needed.  */
+	      do_cleanups (cleanup);
+	    }
+	  else
 	    {
-	      /* The user-specified variable was not valid.  Do not
-		 throw an error here.  parse_linespec will do it for us.  */
+	      /* The name is also not a label.  Abort parsing.  Do not throw
+		 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;
 	      discard_cleanups (cleanup);
 	      return;
 	    }
-
-	  /* The convenience variable/history value parsed correctly.
-	     NAME is no longer needed.  */
-	  do_cleanups (cleanup);
-	}
-      else
-	{
-	  /* The name is also not a label.  Abort parsing.  Do not throw
-	     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;
-	  discard_cleanups (cleanup);
-	  return;
 	}
     }
-    }
 
   int previous_qc = parser->completion_quote_char;
 
@@ -2027,29 +2025,28 @@ linespec_parse_basic (linespec_parser *parser)
 	    }
 	  else
 	    {
-	      /* XXX Reindent before pushing.  */
-
-	  /* Grab a copy of the label's name and look it up.  */
-	  name = copy_token_string (token);
-	  cleanup = make_cleanup (xfree, name);
-	  labels = find_label_symbols (PARSER_STATE (parser),
-				       PARSER_RESULT (parser)->function_symbols,
-				       &symbols, name);
+	      /* Grab a copy of the label's name and look it up.  */
+	      name = copy_token_string (token);
+	      cleanup = make_cleanup (xfree, name);
+	      labels
+		= find_label_symbols (PARSER_STATE (parser),
+				      PARSER_RESULT (parser)->function_symbols,
+				      &symbols, name);
 
-	  if (labels != NULL)
-	    {
-	      PARSER_RESULT (parser)->labels.label_symbols = labels;
-	      PARSER_RESULT (parser)->labels.function_symbols = symbols;
-	      PARSER_EXPLICIT (parser)->label_name = name;
-	      symbols = NULL;
-	      discard_cleanups (cleanup);
-	    }
-	  else
-	    {
-	      /* We don't know what it was, but it isn't a label.  */
-	      undefined_label_error (PARSER_EXPLICIT (parser)->function_name,
-				     name);
-	    }
+	      if (labels != NULL)
+		{
+		  PARSER_RESULT (parser)->labels.label_symbols = labels;
+		  PARSER_RESULT (parser)->labels.function_symbols = symbols;
+		  PARSER_EXPLICIT (parser)->label_name = name;
+		  symbols = NULL;
+		  discard_cleanups (cleanup);
+		}
+	      else
+		{
+		  /* We don't know what it was, but it isn't a label.  */
+		  undefined_label_error
+		    (PARSER_EXPLICIT (parser)->function_name, name);
+		}
 
 	    }
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 00/10] Remove some cleanups from linespec.c
@ 2018-04-04  4:40 Tom Tromey
  2018-04-04  4:40 ` [RFA v2 07/10] Change streq to return bool Tom Tromey
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:40 UTC (permalink / raw)
  To: gdb-patches

Here's the second version of the series to remove some cleanups from
linespec.c.

I believe this version addresses all the review comments.

Regression tested by the buildbot.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 06/10] Remove a string copy from event_location_to_sals
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (7 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 09/10] Remove typep and VEC(typep) from linespec.c Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 10/10] Remove unnecessary include from linespec.h Tom Tromey
  2018-04-05  3:08 ` [RFA v2 00/10] Remove some cleanups from linespec.c Simon Marchi
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The use of "const" showed that a string copy in event_location_to_sals
was unnecessary.  This patch removes it.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.c (event_location_to_sals) <case ADDRESS_LOCATION>:
	Remove a string copy.
---
 gdb/ChangeLog  | 5 +++++
 gdb/linespec.c | 8 +-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 91dabb6117..1e1ce2a239 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3165,16 +3165,10 @@ event_location_to_sals (linespec_parser *parser,
 
 	if (addr_string != NULL)
 	  {
-	    char *expr = xstrdup (addr_string);
-	    const char *const_expr = expr;
-	    struct cleanup *cleanup = make_cleanup (xfree, expr);
-
-	    addr = linespec_expression_to_pc (&const_expr);
+	    addr = linespec_expression_to_pc (&addr_string);
 	    if (PARSER_STATE (parser)->canonical != NULL)
 	      PARSER_STATE (parser)->canonical->location
 		= copy_event_location (location);
-
-	    do_cleanups (cleanup);
 	  }
 
 	result = convert_address_location_to_sals (PARSER_STATE (parser),
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 10/10] Remove unnecessary include from linespec.h
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (8 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 06/10] Remove a string copy from event_location_to_sals Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-05  3:08 ` [RFA v2 00/10] Remove some cleanups from linespec.c Simon Marchi
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

linespec.h was inculding vec.h, but doesn't expose any VECs.
So, this include can be removed.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.h: Remove include of "vec.h".
---
 gdb/ChangeLog  | 4 ++++
 gdb/linespec.h | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/linespec.h b/gdb/linespec.h
index eced085e3e..7251a963af 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -20,7 +20,6 @@
 struct symtab;
 
 #include "location.h"
-#include "vec.h"
 
 /* Flags to pass to decode_line_1 and decode_line_full.  */
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 05/10] Have filter_results take a std::vector
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (5 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 08/10] More use of std::vector in linespec.c Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 09/10] Remove typep and VEC(typep) from linespec.c Tom Tromey
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This chagnes filter_results to take a std::vector, allowing the
removal of some cleanups in its callers.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.c (filter_results): Use std::vector.
	(decode_line_2, decode_line_full): Update.
---
 gdb/ChangeLog  |  5 +++++
 gdb/linespec.c | 22 ++++++----------------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index c445a0be57..91dabb6117 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1402,12 +1402,9 @@ canonical_to_fullform (const struct linespec_canonical_name *canonical)
 static void
 filter_results (struct linespec_state *self,
 		std::vector<symtab_and_line> *result,
-		VEC (const_char_ptr) *filters)
+		const std::vector<const char *> &filters)
 {
-  int i;
-  const char *name;
-
-  for (i = 0; VEC_iterate (const_char_ptr, filters, i, name); ++i)
+  for (const char *name : filters)
     {
       linespec_sals lsal;
 
@@ -1497,16 +1494,13 @@ decode_line_2 (struct linespec_state *self,
   char *args;
   const char *prompt;
   int i;
-  struct cleanup *old_chain;
-  VEC (const_char_ptr) *filters = NULL;
+  std::vector<const char *> filters;
   std::vector<struct decode_line_2_item> items;
 
   gdb_assert (select_mode != multiple_symbols_all);
   gdb_assert (self->canonical != NULL);
   gdb_assert (!result->empty ());
 
-  old_chain = make_cleanup (VEC_cleanup (const_char_ptr), &filters);
-
   /* Prepare ITEMS array.  */
   for (i = 0; i < result->size (); ++i)
     {
@@ -1553,7 +1547,6 @@ decode_line_2 (struct linespec_state *self,
   
   if (select_mode == multiple_symbols_all || items.size () == 1)
     {
-      do_cleanups (old_chain);
       convert_results_to_lsals (self, result);
       return;
     }
@@ -1587,7 +1580,6 @@ decode_line_2 (struct linespec_state *self,
 	     multiple_symbols_all behavior even with the 'ask'
 	     setting; and he can get separate breakpoints by entering
 	     "2-57" at the query.  */
-	  do_cleanups (old_chain);
 	  convert_results_to_lsals (self, result);
 	  return;
 	}
@@ -1601,7 +1593,7 @@ decode_line_2 (struct linespec_state *self,
 
 	  if (!item->selected)
 	    {
-	      VEC_safe_push (const_char_ptr, filters, item->fullform.c_str ());
+	      filters.push_back (item->fullform.c_str ());
 	      item->selected = 1;
 	    }
 	  else
@@ -1613,7 +1605,6 @@ decode_line_2 (struct linespec_state *self,
     }
 
   filter_results (self, result, filters);
-  do_cleanups (old_chain);
 }
 
 \f
@@ -3225,7 +3216,7 @@ decode_line_full (const struct event_location *location, int flags,
 		  const char *filter)
 {
   struct cleanup *cleanups;
-  VEC (const_char_ptr) *filters = NULL;
+  std::vector<const char *> filters;
   linespec_parser parser;
   struct linespec_state *state;
 
@@ -3277,8 +3268,7 @@ decode_line_full (const struct event_location *location, int flags,
     {
       if (filter != NULL)
 	{
-	  make_cleanup (VEC_cleanup (const_char_ptr), &filters);
-	  VEC_safe_push (const_char_ptr, filters, filter);
+	  filters.push_back (filter);
 	  filter_results (state, &result, filters);
 	}
       else
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 08/10] More use of std::vector in linespec.c
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (4 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 04/10] Return std::string from canonical_to_fullform Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 05/10] Have filter_results take a std::vector Tom Tromey
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes some spots in linespec.c to take a std::vector.  This
patch spilled out to objc-lang.c a bit as well.  This change allows
for the removal of some cleanups.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* utils.c (compare_strings): Remove.
	* utils.h (compare_strings): Remove.
	* objc-lang.h (find_imps): Update.
	* objc-lang.c (find_methods): Take a std::vector.
	(uniquify_strings, find_imps): Likewise.
	* linespec.c (find_methods): Take a std::vector.
	(decode_objc): Use std::vector.
	(add_all_symbol_names_from_pspace, find_superclass_methods): Take
	a std::vector.
	(find_method, find_function_symbols): Use std::vector.
---
 gdb/ChangeLog   | 13 +++++++++++++
 gdb/linespec.c  | 52 ++++++++++++++++++++--------------------------------
 gdb/objc-lang.c | 44 ++++++++++++--------------------------------
 gdb/objc-lang.h |  4 ++--
 gdb/utils.c     | 11 -----------
 gdb/utils.h     |  1 -
 6 files changed, 47 insertions(+), 78 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1e1ce2a239..b52e0061da 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -377,10 +377,9 @@ static void add_matching_symbols_to_info (const char *name,
 					  struct collect_info *info,
 					  struct program_space *pspace);
 
-static void add_all_symbol_names_from_pspace (struct collect_info *info,
-					      struct program_space *pspace,
-					      VEC (const_char_ptr) *names,
-					      enum search_domain search_domain);
+static void add_all_symbol_names_from_pspace
+    (struct collect_info *info, struct program_space *pspace,
+     const std::vector<const char *> &names, enum search_domain search_domain);
 
 static VEC (symtab_ptr) *
   collect_symtabs_from_filename (const char *file,
@@ -1211,7 +1210,7 @@ iterate_over_file_blocks
 
 static void
 find_methods (struct type *t, enum language t_lang, const char *name,
-	      VEC (const_char_ptr) **result_names,
+	      std::vector<const char *> *result_names,
 	      VEC (typep) **superclasses)
 {
   int ibase;
@@ -1266,7 +1265,7 @@ find_methods (struct type *t, enum language t_lang, const char *name,
 		  if (TYPE_FN_FIELD_STUB (f, field_counter))
 		    continue;
 		  phys_name = TYPE_FN_FIELD_PHYSNAME (f, field_counter);
-		  VEC_safe_push (const_char_ptr, *result_names, phys_name);
+		  result_names->push_back (phys_name);
 		}
 	    }
 	}
@@ -3398,20 +3397,19 @@ static std::vector<symtab_and_line>
 decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
 {
   struct collect_info info;
-  VEC (const_char_ptr) *symbol_names = NULL;
+  std::vector<const char *> symbol_names;
   const char *new_argptr;
-  struct cleanup *cleanup = make_cleanup (VEC_cleanup (const_char_ptr),
-					  &symbol_names);
 
   info.state = self;
   info.file_symtabs = NULL;
   VEC_safe_push (symtab_ptr, info.file_symtabs, NULL);
-  make_cleanup (VEC_cleanup (symtab_ptr), &info.file_symtabs);
+  struct cleanup *cleanup = make_cleanup (VEC_cleanup (symtab_ptr),
+					  &info.file_symtabs);
   info.result.symbols = NULL;
   info.result.minimal_symbols = NULL;
 
   new_argptr = find_imps (arg, &symbol_names);
-  if (VEC_empty (const_char_ptr, symbol_names))
+  if (symbol_names.empty ())
     {
       do_cleanups (cleanup);
       return {};
@@ -3637,13 +3635,10 @@ compare_msymbols (const void *a, const void *b)
 static void
 add_all_symbol_names_from_pspace (struct collect_info *info,
 				  struct program_space *pspace,
-				  VEC (const_char_ptr) *names,
+				  const std::vector<const char *> &names,
 				  enum search_domain search_domain)
 {
-  int ix;
-  const char *iter;
-
-  for (ix = 0; VEC_iterate (const_char_ptr, names, ix, iter); ++ix)
+  for (const char *iter : names)
     add_matching_symbols_to_info (iter,
 				  symbol_name_match_type::FULL,
 				  search_domain, info, pspace);
@@ -3652,9 +3647,9 @@ add_all_symbol_names_from_pspace (struct collect_info *info,
 static void
 find_superclass_methods (VEC (typep) *superclasses,
 			 const char *name, enum language name_lang,
-			 VEC (const_char_ptr) **result_names)
+			 std::vector<const char *> *result_names)
 {
-  int old_len = VEC_length (const_char_ptr, *result_names);
+  size_t old_len = result_names->size ();
   VEC (typep) *iter_classes;
   struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
 
@@ -3669,8 +3664,7 @@ find_superclass_methods (VEC (typep) *superclasses,
       for (ix = 0; VEC_iterate (typep, iter_classes, ix, t); ++ix)
 	find_methods (t, name_lang, name, result_names, &new_supers);
 
-      if (VEC_length (const_char_ptr, *result_names) != old_len
-	  || VEC_empty (typep, new_supers))
+      if (result_names->size () != old_len || VEC_empty (typep, new_supers))
 	break;
 
       iter_classes = new_supers;
@@ -3692,9 +3686,9 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
   struct symbol *sym;
   struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
   int ix;
-  int last_result_len;
+  size_t last_result_len;
   VEC (typep) *superclass_vec;
-  VEC (const_char_ptr) *result_names;
+  std::vector<const char *> result_names;
   struct collect_info info;
 
   /* Sort symbols so that symbols with the same program space are next
@@ -3720,8 +3714,6 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
      what to do.  */
   superclass_vec = NULL;
   make_cleanup (VEC_cleanup (typep), &superclass_vec);
-  result_names = NULL;
-  make_cleanup (VEC_cleanup (const_char_ptr), &result_names);
   last_result_len = 0;
   for (ix = 0; VEC_iterate (symbolp, sym_classes, ix, sym); ++ix)
     {
@@ -3746,7 +3738,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
 	{
 	  /* If we did not find a direct implementation anywhere in
 	     this program space, consider superclasses.  */
-	  if (VEC_length (const_char_ptr, result_names) == last_result_len)
+	  if (result_names.size () == last_result_len)
 	    find_superclass_methods (superclass_vec, method_name,
 				     SYMBOL_LANGUAGE (sym), &result_names);
 
@@ -3757,7 +3749,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
 					    FUNCTIONS_DOMAIN);
 
 	  VEC_truncate (typep, superclass_vec, 0);
-	  last_result_len = VEC_length (const_char_ptr, result_names);
+	  last_result_len = result_names.size ();
 	}
     }
 
@@ -3902,9 +3894,7 @@ find_function_symbols (struct linespec_state *state,
 		       VEC (bound_minimal_symbol_d) **minsyms)
 {
   struct collect_info info;
-  VEC (const_char_ptr) *symbol_names = NULL;
-  struct cleanup *cleanup = make_cleanup (VEC_cleanup (const_char_ptr),
-					  &symbol_names);
+  std::vector<const char *> symbol_names;
 
   info.state = state;
   info.result.symbols = NULL;
@@ -3913,15 +3903,13 @@ find_function_symbols (struct linespec_state *state,
 
   /* Try NAME as an Objective-C selector.  */
   find_imps (name, &symbol_names);
-  if (!VEC_empty (const_char_ptr, symbol_names))
+  if (!symbol_names.empty ())
     add_all_symbol_names_from_pspace (&info, state->search_pspace,
 				      symbol_names, FUNCTIONS_DOMAIN);
   else
     add_matching_symbols_to_info (name, name_match_type, FUNCTIONS_DOMAIN,
 				  &info, state->search_pspace);
 
-  do_cleanups (cleanup);
-
   if (VEC_empty (symbolp, info.result.symbols))
     {
       VEC_free (symbolp, info.result.symbols);
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 4f7dc36660..8c24fde3ee 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -45,6 +45,7 @@
 #include "cli/cli-utils.h"
 
 #include <ctype.h>
+#include <algorithm>
 
 struct objc_object {
   CORE_ADDR isa;
@@ -974,7 +975,7 @@ parse_method (char *method, char *type, char **theclass,
 static void
 find_methods (char type, const char *theclass, const char *category, 
 	      const char *selector,
-	      VEC (const_char_ptr) **symbol_names)
+	      std::vector<const char *> *symbol_names)
 {
   struct objfile *objfile = NULL;
 
@@ -1050,7 +1051,7 @@ find_methods (char type, const char *theclass, const char *category,
 	      ((nselector == NULL) || (strcmp (selector, nselector) != 0)))
 	    continue;
 
-	  VEC_safe_push (const_char_ptr, *symbol_names, symname);
+	  symbol_names->push_back (symname);
 	}
 
       if (objc_csym == NULL)
@@ -1068,33 +1069,14 @@ find_methods (char type, const char *theclass, const char *category,
 /* Uniquify a VEC of strings.  */
 
 static void
-uniquify_strings (VEC (const_char_ptr) **strings)
+uniquify_strings (std::vector<const char *> *strings)
 {
-  int ix;
-  const char *elem, *last = NULL;
-  int out;
-
-  /* If the vector is empty, there's nothing to do.  This explicit
-     check is needed to avoid invoking qsort with NULL. */
-  if (VEC_empty (const_char_ptr, *strings))
+  if (strings->empty ())
     return;
 
-  qsort (VEC_address (const_char_ptr, *strings),
-	 VEC_length (const_char_ptr, *strings),
-	 sizeof (const_char_ptr),
-	 compare_strings);
-  out = 0;
-  for (ix = 0; VEC_iterate (const_char_ptr, *strings, ix, elem); ++ix)
-    {
-      if (last == NULL || strcmp (last, elem) != 0)
-	{
-	  /* Keep ELEM.  */
-	  VEC_replace (const_char_ptr, *strings, out, elem);
-	  ++out;
-	}
-      last = elem;
-    }
-  VEC_truncate (const_char_ptr, *strings, out);
+  std::sort (strings->begin (), strings->end (), compare_cstrings);
+  strings->erase (std::unique (strings->begin (), strings->end (), streq),
+		  strings->end ());
 }
 
 /* 
@@ -1128,7 +1110,7 @@ uniquify_strings (VEC (const_char_ptr) **strings)
  */
 
 const char *
-find_imps (const char *method, VEC (const_char_ptr) **symbol_names)
+find_imps (const char *method, std::vector<const char *> *symbol_names)
 {
   char type = '\0';
   char *theclass = NULL;
@@ -1161,22 +1143,20 @@ find_imps (const char *method, VEC (const_char_ptr) **symbol_names)
 
   /* If we hit the "selector" case, and we found some methods, then
      add the selector itself as a symbol, if it exists.  */
-  if (selector_case && !VEC_empty (const_char_ptr, *symbol_names))
+  if (selector_case && !symbol_names->empty ())
     {
       struct symbol *sym = lookup_symbol (selector, NULL, VAR_DOMAIN,
 					  0).symbol;
 
       if (sym != NULL) 
-	VEC_safe_push (const_char_ptr, *symbol_names,
-		       SYMBOL_NATURAL_NAME (sym));
+	symbol_names->push_back (SYMBOL_NATURAL_NAME (sym));
       else
 	{
 	  struct bound_minimal_symbol msym
 	    = lookup_minimal_symbol (selector, 0, 0);
 
 	  if (msym.minsym != NULL) 
-	    VEC_safe_push (const_char_ptr, *symbol_names,
-			   MSYMBOL_NATURAL_NAME (msym.minsym));
+	    symbol_names->push_back (MSYMBOL_NATURAL_NAME (msym.minsym));
 	}
     }
 
diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h
index 0727dfb3f0..d3ed1d5fa6 100644
--- a/gdb/objc-lang.h
+++ b/gdb/objc-lang.h
@@ -37,8 +37,8 @@ extern char *objc_demangle (const char *mangled, int options);
 
 extern int find_objc_msgcall (CORE_ADDR pc, CORE_ADDR *new_pc);
 
-extern const char *
-  find_imps (const char *method, VEC (const_char_ptr) **symbol_names);
+extern const char *find_imps (const char *method,
+			      std::vector<const char *> *symbol_names);
 
 extern struct value *value_nsstring (struct gdbarch *gdbarch,
 				     char *ptr, int len);
diff --git a/gdb/utils.c b/gdb/utils.c
index bd7553e5f4..df0a2cecc4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2937,17 +2937,6 @@ compare_positive_ints (const void *ap, const void *bp)
   return * (int *) ap - * (int *) bp;
 }
 
-/* String compare function for qsort.  */
-
-int
-compare_strings (const void *arg1, const void *arg2)
-{
-  const char **s1 = (const char **) arg1;
-  const char **s2 = (const char **) arg2;
-
-  return strcmp (*s1, *s2);
-}
-
 #define AMBIGUOUS_MESS1	".\nMatching formats:"
 #define AMBIGUOUS_MESS2	\
   ".\nUse \"set gnutarget format-name\" to specify the format."
diff --git a/gdb/utils.h b/gdb/utils.h
index d3b8871b58..9a1fb34544 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -101,7 +101,6 @@ extern int streq_hash (const void *, const void *);
 extern int subset_compare (const char *, const char *);
 
 int compare_positive_ints (const void *ap, const void *bp);
-int compare_strings (const void *ap, const void *bp);
 
 /* Compare C strings for std::sort.  */
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 03/10] Make copy_token_string return unique_xmalloc_ptr
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (2 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 04/10] Return std::string from canonical_to_fullform Tom Tromey
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes copy_token_string to return a unique_xmalloc_ptr, which
allows the removal of some cleanups.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.c (copy_token_string): Return a unique_xmalloc_ptr.
	(unexpected_linespec_error): Update.
	(linespec_parse_basic, parse_linespec): Update.
---
 gdb/ChangeLog  |  6 ++++
 gdb/linespec.c | 96 +++++++++++++++++++---------------------------------------
 2 files changed, 37 insertions(+), 65 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index a081a8bebf..5f4e95156b 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -536,20 +536,18 @@ skip_quote_char (const char *string, char quote_char)
 /* Make a writable copy of the string given in TOKEN, trimming
    any trailing whitespace.  */
 
-static char *
+static gdb::unique_xmalloc_ptr<char>
 copy_token_string (linespec_token token)
 {
-  char *str, *s;
+  const char *str, *s;
 
   if (token.type == LSTOKEN_KEYWORD)
-    return xstrdup (LS_TOKEN_KEYWORD (token));
+    return gdb::unique_xmalloc_ptr<char> (xstrdup (LS_TOKEN_KEYWORD (token)));
 
-  str = savestring (LS_TOKEN_STOKEN (token).ptr,
-		    LS_TOKEN_STOKEN (token).length);
+  str = LS_TOKEN_STOKEN (token).ptr;
   s = remove_trailing_whitespace (str, str + LS_TOKEN_STOKEN (token).length);
-  *s = '\0';
 
-  return str;
+  return gdb::unique_xmalloc_ptr<char> (savestring (str, s - str));
 }
 
 /* Does P represent the end of a quote-enclosed linespec?  */
@@ -1692,13 +1690,10 @@ unexpected_linespec_error (linespec_parser *parser)
   if (token.type == LSTOKEN_STRING || token.type == LSTOKEN_NUMBER
       || token.type == LSTOKEN_KEYWORD)
     {
-      char *string;
-
-      string = copy_token_string (token);
-      make_cleanup (xfree, string);
+      gdb::unique_xmalloc_ptr<char> string = copy_token_string (token);
       throw_error (GENERIC_ERROR,
 		   _("malformed linespec error: unexpected %s, \"%s\""),
-		   token_type_strings[token.type], string);
+		   token_type_strings[token.type], string.get ());
     }
   else
     throw_error (GENERIC_ERROR,
@@ -1792,11 +1787,10 @@ set_completion_after_number (linespec_parser *parser,
 static void
 linespec_parse_basic (linespec_parser *parser)
 {
-  char *name;
+  gdb::unique_xmalloc_ptr<char> name;
   linespec_token token;
   VEC (symbolp) *symbols, *labels;
   VEC (bound_minimal_symbol_d) *minimal_symbols;
-  struct cleanup *cleanup;
 
   /* Get the next token.  */
   token = linespec_lexer_lex_one (parser);
@@ -1818,9 +1812,8 @@ linespec_parse_basic (linespec_parser *parser)
 
       /* Record the line offset and get the next token.  */
       name = copy_token_string (token);
-      cleanup = make_cleanup (xfree, name);
-      PARSER_EXPLICIT (parser)->line_offset = linespec_parse_line_offset (name);
-      do_cleanups (cleanup);
+      PARSER_EXPLICIT (parser)->line_offset
+	= linespec_parse_line_offset (name.get ());
 
       /* Get the next token.  */
       token = linespec_lexer_consume_token (parser);
@@ -1851,7 +1844,6 @@ linespec_parse_basic (linespec_parser *parser)
   /* The current token will contain the name of a function, method,
      or label.  */
   name = copy_token_string (token);
-  cleanup = make_cleanup (free_current_contents, &name);
 
   if (parser->completion_tracker != NULL)
     {
@@ -1885,21 +1877,19 @@ linespec_parse_basic (linespec_parser *parser)
 	      PARSER_STREAM (parser)++;
 	      LS_TOKEN_STOKEN (token).length++;
 
-	      xfree (name);
-	      name = savestring (parser->completion_word,
-				 (PARSER_STREAM (parser)
-				  - parser->completion_word));
+	      name.reset (savestring (parser->completion_word,
+				      (PARSER_STREAM (parser)
+				       - parser->completion_word)));
 	    }
 	}
 
-      PARSER_EXPLICIT (parser)->function_name = name;
-      discard_cleanups (cleanup);
+      PARSER_EXPLICIT (parser)->function_name = name.release ();
     }
   else
     {
       /* Try looking it up as a function/method.  */
       find_linespec_symbols (PARSER_STATE (parser),
-			     PARSER_RESULT (parser)->file_symtabs, name,
+			     PARSER_RESULT (parser)->file_symtabs, name.get (),
 			     PARSER_EXPLICIT (parser)->func_name_match_type,
 			     &symbols, &minimal_symbols);
 
@@ -1907,43 +1897,36 @@ linespec_parse_basic (linespec_parser *parser)
 	{
 	  PARSER_RESULT (parser)->function_symbols = symbols;
 	  PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
-	  PARSER_EXPLICIT (parser)->function_name = name;
+	  PARSER_EXPLICIT (parser)->function_name = name.release ();
 	  symbols = NULL;
-	  discard_cleanups (cleanup);
 	}
       else
 	{
 	  /* NAME was not a function or a method.  So it must be a label
 	     name or user specified variable like "break foo.c:$zippo".  */
 	  labels = find_label_symbols (PARSER_STATE (parser), NULL,
-				       &symbols, name);
+				       &symbols, name.get ());
 	  if (labels != NULL)
 	    {
 	      PARSER_RESULT (parser)->labels.label_symbols = labels;
 	      PARSER_RESULT (parser)->labels.function_symbols = symbols;
-	      PARSER_EXPLICIT (parser)->label_name = name;
+	      PARSER_EXPLICIT (parser)->label_name = name.release ();
 	      symbols = NULL;
-	      discard_cleanups (cleanup);
 	    }
 	  else if (token.type == LSTOKEN_STRING
 		   && *LS_TOKEN_STOKEN (token).ptr == '$')
 	    {
 	      /* User specified a convenience variable or history value.  */
 	      PARSER_EXPLICIT (parser)->line_offset
-		= linespec_parse_variable (PARSER_STATE (parser), name);
+		= linespec_parse_variable (PARSER_STATE (parser), name.get ());
 
 	      if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
 		{
 		  /* 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;
-		  discard_cleanups (cleanup);
+		  PARSER_EXPLICIT (parser)->function_name = name.release ();
 		  return;
 		}
-
-	      /* The convenience variable/history value parsed correctly.
-		 NAME is no longer needed.  */
-	      do_cleanups (cleanup);
 	    }
 	  else
 	    {
@@ -1951,8 +1934,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;
-	      discard_cleanups (cleanup);
+	      PARSER_EXPLICIT (parser)->function_name = name.release ();
 	      return;
 	    }
 	}
@@ -1980,10 +1962,8 @@ linespec_parse_basic (linespec_parser *parser)
 	  set_completion_after_number (parser, linespec_complete_what::KEYWORD);
 
 	  name = copy_token_string (token);
-	  cleanup = make_cleanup (xfree, name);
 	  PARSER_EXPLICIT (parser)->line_offset
-	    = linespec_parse_line_offset (name);
-	  do_cleanups (cleanup);
+	    = linespec_parse_line_offset (name.get ());
 
 	  /* Get the next token.  */
 	  token = linespec_lexer_consume_token (parser);
@@ -2027,25 +2007,23 @@ linespec_parse_basic (linespec_parser *parser)
 	    {
 	      /* Grab a copy of the label's name and look it up.  */
 	      name = copy_token_string (token);
-	      cleanup = make_cleanup (xfree, name);
 	      labels
 		= find_label_symbols (PARSER_STATE (parser),
 				      PARSER_RESULT (parser)->function_symbols,
-				      &symbols, name);
+				      &symbols, name.get ());
 
 	      if (labels != NULL)
 		{
 		  PARSER_RESULT (parser)->labels.label_symbols = labels;
 		  PARSER_RESULT (parser)->labels.function_symbols = symbols;
-		  PARSER_EXPLICIT (parser)->label_name = name;
+		  PARSER_EXPLICIT (parser)->label_name = name.release ();
 		  symbols = NULL;
-		  discard_cleanups (cleanup);
 		}
 	      else
 		{
 		  /* We don't know what it was, but it isn't a label.  */
 		  undefined_label_error
-		    (PARSER_EXPLICIT (parser)->function_name, name);
+		    (PARSER_EXPLICIT (parser)->function_name, name.get ());
 		}
 
 	    }
@@ -2063,11 +2041,9 @@ linespec_parse_basic (linespec_parser *parser)
 
 	      /* Record the line offset and get the next token.  */
 	      name = copy_token_string (token);
-	      cleanup = make_cleanup (xfree, name);
 
 	      PARSER_EXPLICIT (parser)->line_offset
-		= linespec_parse_line_offset (name);
-	      do_cleanups (cleanup);
+		= linespec_parse_line_offset (name.get ());
 
 	      /* Get the next token.  */
 	      token = linespec_lexer_consume_token (parser);
@@ -2524,7 +2500,6 @@ parse_linespec (linespec_parser *parser, const char *arg,
 {
   linespec_token token;
   struct gdb_exception file_exception = exception_none;
-  struct cleanup *cleanup;
 
   /* A special case to start.  It has become quite popular for
      IDEs to work around bugs in the previous parser by quoting
@@ -2582,18 +2557,14 @@ parse_linespec (linespec_parser *parser, const char *arg,
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
   if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
     {
-      char *var;
-
       /* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB.  */
       if (parser->completion_tracker == NULL)
 	VEC_safe_push (symtab_ptr, PARSER_RESULT (parser)->file_symtabs, NULL);
 
       /* User specified a convenience variable or history value.  */
-      var = copy_token_string (token);
-      cleanup = make_cleanup (xfree, var);
+      gdb::unique_xmalloc_ptr<char> var = copy_token_string (token);
       PARSER_EXPLICIT (parser)->line_offset
-	= linespec_parse_variable (PARSER_STATE (parser), var);
-      do_cleanups (cleanup);
+	= linespec_parse_variable (PARSER_STATE (parser), var.get ());
 
       /* If a line_offset wasn't found (VAR is the name of a user
 	 variable/function), then skip to normal symbol processing.  */
@@ -2622,17 +2593,15 @@ parse_linespec (linespec_parser *parser, const char *arg,
 
   if (token.type == LSTOKEN_COLON)
     {
-      char *user_filename;
-
       /* Get the current token again and extract the filename.  */
       token = linespec_lexer_lex_one (parser);
-      user_filename = copy_token_string (token);
+      gdb::unique_xmalloc_ptr<char> user_filename = copy_token_string (token);
 
       /* Check if the input is a filename.  */
       TRY
 	{
 	  PARSER_RESULT (parser)->file_symtabs
-	    = symtabs_from_filename (user_filename,
+	    = symtabs_from_filename (user_filename.get (),
 				     PARSER_STATE (parser)->search_pspace);
 	}
       CATCH (ex, RETURN_MASK_ERROR)
@@ -2644,7 +2613,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;
+	  PARSER_EXPLICIT (parser)->source_filename = user_filename.release ();
 
 	  /* Get the next token.  */
 	  token = linespec_lexer_consume_token (parser);
@@ -2654,9 +2623,6 @@ parse_linespec (linespec_parser *parser, const char *arg,
 	}
       else
 	{
-	  /* No symtabs found -- discard user_filename.  */
-	  xfree (user_filename);
-
 	  /* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB.  */
 	  VEC_safe_push (symtab_ptr, PARSER_RESULT (parser)->file_symtabs, NULL);
 	}
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 09/10] Remove typep and VEC(typep) from linespec.c
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (6 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 05/10] Have filter_results take a std::vector Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 06/10] Remove a string copy from event_location_to_sals Tom Tromey
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes VEC(typep) from linespec.c in favor of std::vector.  It
also removes the "typep" typedef.  This change allowed the removal of
some cleanups.

I believe the previous cleanup code in find_superclass_methods could
result in a memory leak, so this patch is an improvement in that way
as well.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.c (typep): Remove typedef.
	(find_methods, find_superclass_methods): Take a std::vector.
	(find_method): Use std::vector.
---
 gdb/ChangeLog  |  6 ++++++
 gdb/linespec.c | 35 ++++++++++-------------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index b52e0061da..7ef8012ab5 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -80,9 +80,6 @@ enum class linespec_complete_what
 typedef struct symbol *symbolp;
 DEF_VEC_P (symbolp);
 
-typedef struct type *typep;
-DEF_VEC_P (typep);
-
 /* An address entry is used to ensure that any given location is only
    added to the result a single time.  It holds an address and the
    program space from which the address came.  */
@@ -1211,7 +1208,7 @@ iterate_over_file_blocks
 static void
 find_methods (struct type *t, enum language t_lang, const char *name,
 	      std::vector<const char *> *result_names,
-	      VEC (typep) **superclasses)
+	      std::vector<struct type *> *superclasses)
 {
   int ibase;
   const char *class_name = type_name_no_tag (t);
@@ -1272,7 +1269,7 @@ find_methods (struct type *t, enum language t_lang, const char *name,
     }
 
   for (ibase = 0; ibase < TYPE_N_BASECLASSES (t); ibase++)
-    VEC_safe_push (typep, *superclasses, TYPE_BASECLASS (t, ibase));
+    superclasses->push_back (TYPE_BASECLASS (t, ibase));
 }
 
 /* Find an instance of the character C in the string S that is outside
@@ -3645,32 +3642,24 @@ add_all_symbol_names_from_pspace (struct collect_info *info,
 }
 
 static void
-find_superclass_methods (VEC (typep) *superclasses,
+find_superclass_methods (std::vector<struct type *> &&superclasses,
 			 const char *name, enum language name_lang,
 			 std::vector<const char *> *result_names)
 {
   size_t old_len = result_names->size ();
-  VEC (typep) *iter_classes;
-  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
 
-  iter_classes = superclasses;
   while (1)
     {
-      VEC (typep) *new_supers = NULL;
-      int ix;
-      struct type *t;
+      std::vector<struct type *> new_supers;
 
-      make_cleanup (VEC_cleanup (typep), &new_supers);
-      for (ix = 0; VEC_iterate (typep, iter_classes, ix, t); ++ix)
+      for (struct type *t : superclasses)
 	find_methods (t, name_lang, name, result_names, &new_supers);
 
-      if (result_names->size () != old_len || VEC_empty (typep, new_supers))
+      if (result_names->size () != old_len || new_supers.empty ())
 	break;
 
-      iter_classes = new_supers;
+      superclasses = std::move (new_supers);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* This finds the method METHOD_NAME in the class CLASS_NAME whose type is
@@ -3684,10 +3673,9 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
 	     VEC (bound_minimal_symbol_d) **minsyms)
 {
   struct symbol *sym;
-  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
   int ix;
   size_t last_result_len;
-  VEC (typep) *superclass_vec;
+  std::vector<struct type *> superclass_vec;
   std::vector<const char *> result_names;
   struct collect_info info;
 
@@ -3712,8 +3700,6 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
      those names.  This loop is written in a somewhat funny way
      because we collect data across the program space before deciding
      what to do.  */
-  superclass_vec = NULL;
-  make_cleanup (VEC_cleanup (typep), &superclass_vec);
   last_result_len = 0;
   for (ix = 0; VEC_iterate (symbolp, sym_classes, ix, sym); ++ix)
     {
@@ -3739,7 +3725,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
 	  /* If we did not find a direct implementation anywhere in
 	     this program space, consider superclasses.  */
 	  if (result_names.size () == last_result_len)
-	    find_superclass_methods (superclass_vec, method_name,
+	    find_superclass_methods (std::move (superclass_vec), method_name,
 				     SYMBOL_LANGUAGE (sym), &result_names);
 
 	  /* We have a list of candidate symbol names, so now we
@@ -3748,7 +3734,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
 	  add_all_symbol_names_from_pspace (&info, pspace, result_names,
 					    FUNCTIONS_DOMAIN);
 
-	  VEC_truncate (typep, superclass_vec, 0);
+	  superclass_vec.clear ();
 	  last_result_len = result_names.size ();
 	}
     }
@@ -3758,7 +3744,6 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
     {
       *symbols = info.result.symbols;
       *minsyms = info.result.minimal_symbols;
-      do_cleanups (cleanup);
       return;
     }
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 01/10] Remove some cleanups from search_minsyms_for_name
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
  2018-04-04  4:40 ` [RFA v2 07/10] Change streq to return bool Tom Tromey
  2018-04-04  4:40 ` [RFA v2 02/10] Fix some indentation in linespec.c Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 03/10] Make copy_token_string return unique_xmalloc_ptr Tom Tromey
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes struct collect_minsyms to use a std::vector, which
enables the removal of a cleanup from search_minsyms_for_name.  This
also changes iterate_over_minimal_symbols to take a
gdb::function_view, which makes a function in linespec.c more
type-safe.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* minsyms.h (iterate_over_minimal_symbols): Update.
	* minsyms.c (iterate_over_minimal_symbols): Take a
	gdb::function_view.
	* linespec.c (struct collect_minsyms): Remove.
	(compare_msyms): Now a std::sort comparator.
	(add_minsym): Add parameters.
	(search_minsyms_for_name): Update.  Use std::vector.
---
 gdb/ChangeLog  |  10 +++++
 gdb/linespec.c | 132 ++++++++++++++++++++++-----------------------------------
 gdb/minsyms.c  |  13 +++---
 gdb/minsyms.h  |  13 +++---
 4 files changed, 71 insertions(+), 97 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1236b3f475..31ebca37c4 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -46,6 +46,7 @@
 #include "location.h"
 #include "common/function-view.h"
 #include "common/def-vector.h"
+#include <algorithm>
 
 /* An enumeration of the various things a user might attempt to
    complete for a linespec location.  */
@@ -4370,27 +4371,6 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
     add_sal_to_sals (self, result, &sal, MSYMBOL_NATURAL_NAME (msymbol), 0);
 }
 
-/* A helper struct to pass some data through
-   iterate_over_minimal_symbols.  */
-
-struct collect_minsyms
-{
-  /* The objfile we're examining.  */
-  struct objfile *objfile;
-
-  /* Only search the given symtab, or NULL to search for all symbols.  */
-  struct symtab *symtab;
-
-  /* The funfirstline setting from the initial call.  */
-  int funfirstline;
-
-  /* The list_mode setting from the initial call.  */
-  int list_mode;
-
-  /* The resulting symbols.  */
-  VEC (bound_minimal_symbol_d) *msyms;
-};
-
 /* A helper function to classify a minimal_symbol_type according to
    priority.  */
 
@@ -4415,47 +4395,45 @@ classify_mtype (enum minimal_symbol_type t)
     }
 }
 
-/* Callback for qsort that sorts symbols by priority.  */
+/* Callback for std::sort that sorts symbols by priority.  */
 
-static int
-compare_msyms (const void *a, const void *b)
+static bool
+compare_msyms (const bound_minimal_symbol &a, const bound_minimal_symbol &b)
 {
-  const bound_minimal_symbol_d *moa = (const bound_minimal_symbol_d *) a;
-  const bound_minimal_symbol_d *mob = (const bound_minimal_symbol_d *) b;
-  enum minimal_symbol_type ta = MSYMBOL_TYPE (moa->minsym);
-  enum minimal_symbol_type tb = MSYMBOL_TYPE (mob->minsym);
+  enum minimal_symbol_type ta = MSYMBOL_TYPE (a.minsym);
+  enum minimal_symbol_type tb = MSYMBOL_TYPE (b.minsym);
 
-  return classify_mtype (ta) - classify_mtype (tb);
+  return classify_mtype (ta) < classify_mtype (tb);
 }
 
-/* Callback for iterate_over_minimal_symbols that adds the symbol to
-   the result.  */
+/* Helper for search_minsyms_for_name that adds the symbol to the
+   result.  */
 
 static void
-add_minsym (struct minimal_symbol *minsym, void *d)
+add_minsym (struct minimal_symbol *minsym, struct objfile *objfile,
+	    struct symtab *symtab, int list_mode,
+	    std::vector<struct bound_minimal_symbol> *msyms)
 {
-  struct collect_minsyms *info = (struct collect_minsyms *) d;
-
-  if (info->symtab != NULL)
+  if (symtab != NULL)
     {
       /* We're looking for a label for which we don't have debug
 	 info.  */
       CORE_ADDR func_addr;
-      if (msymbol_is_function (info->objfile, minsym, &func_addr))
+      if (msymbol_is_function (objfile, minsym, &func_addr))
 	{
 	  symtab_and_line sal = find_pc_sect_line (func_addr, NULL, 0);
 
-	  if (info->symtab != sal.symtab)
+	  if (symtab != sal.symtab)
 	    return;
 	}
     }
 
   /* Exclude data symbols when looking for breakpoint locations.  */
-  if (!info->list_mode && !msymbol_is_function (info->objfile, minsym))
+  if (!list_mode && !msymbol_is_function (objfile, minsym))
     return;
 
-  bound_minimal_symbol_d mo = {minsym, info->objfile};
-  VEC_safe_push (bound_minimal_symbol_d, info->msyms, &mo);
+  struct bound_minimal_symbol mo = {minsym, objfile};
+  msyms->push_back (mo);
 }
 
 /* Search for minimal symbols called NAME.  If SEARCH_PSPACE
@@ -4471,15 +4449,7 @@ search_minsyms_for_name (struct collect_info *info,
 			 struct program_space *search_pspace,
 			 struct symtab *symtab)
 {
-  struct collect_minsyms local;
-  struct cleanup *cleanup;
-
-  memset (&local, 0, sizeof (local));
-  local.funfirstline = info->state->funfirstline;
-  local.list_mode = info->state->list_mode;
-  local.symtab = symtab;
-
-  cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d), &local.msyms);
+  std::vector<struct bound_minimal_symbol> minsyms;
 
   if (symtab == NULL)
     {
@@ -4498,8 +4468,13 @@ search_minsyms_for_name (struct collect_info *info,
 
 	ALL_OBJFILES (objfile)
 	{
-	  local.objfile = objfile;
-	  iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+	  iterate_over_minimal_symbols (objfile, name,
+					[&] (struct minimal_symbol *msym)
+					  {
+					    add_minsym (msym, objfile, nullptr,
+							info->state->list_mode,
+							&minsyms);
+					  });
 	}
       }
     }
@@ -4508,41 +4483,36 @@ search_minsyms_for_name (struct collect_info *info,
       if (search_pspace == NULL || SYMTAB_PSPACE (symtab) == search_pspace)
 	{
 	  set_current_program_space (SYMTAB_PSPACE (symtab));
-	  local.objfile = SYMTAB_OBJFILE(symtab);
-	  iterate_over_minimal_symbols (local.objfile, name, add_minsym, &local);
+	  iterate_over_minimal_symbols
+	    (SYMTAB_OBJFILE (symtab), name,
+	     [&] (struct minimal_symbol *msym)
+	       {
+		 add_minsym (msym, SYMTAB_OBJFILE (symtab), symtab,
+			     info->state->list_mode, &minsyms);
+	       });
 	}
     }
 
-    if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
-      {
-	int classification;
-	int ix;
-	bound_minimal_symbol_d *item;
-
-	qsort (VEC_address (bound_minimal_symbol_d, local.msyms),
-	       VEC_length (bound_minimal_symbol_d, local.msyms),
-	       sizeof (bound_minimal_symbol_d),
-	       compare_msyms);
-
-	/* Now the minsyms are in classification order.  So, we walk
-	   over them and process just the minsyms with the same
-	   classification as the very first minsym in the list.  */
-	item = VEC_index (bound_minimal_symbol_d, local.msyms, 0);
-	classification = classify_mtype (MSYMBOL_TYPE (item->minsym));
-
-	for (ix = 0;
-	     VEC_iterate (bound_minimal_symbol_d, local.msyms, ix, item);
-	     ++ix)
-	  {
-	    if (classify_mtype (MSYMBOL_TYPE (item->minsym)) != classification)
-	      break;
+  if (!minsyms.empty ())
+    {
+      int classification;
 
-	    VEC_safe_push (bound_minimal_symbol_d,
-			   info->result.minimal_symbols, item);
-	  }
-      }
+      std::sort (minsyms.begin (), minsyms.end (), compare_msyms);
 
-    do_cleanups (cleanup);
+      /* Now the minsyms are in classification order.  So, we walk
+	 over them and process just the minsyms with the same
+	 classification as the very first minsym in the list.  */
+      classification = classify_mtype (MSYMBOL_TYPE (minsyms[0].minsym));
+
+      for (const struct bound_minimal_symbol &item : minsyms)
+	{
+	  if (classify_mtype (MSYMBOL_TYPE (item.minsym)) != classification)
+	    break;
+
+	  VEC_safe_push (bound_minimal_symbol_d,
+			 info->result.minimal_symbols, &item);
+	}
+    }
 }
 
 /* A helper function to add all symbols matching NAME to INFO.  If
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 72969b7778..9d23c4fd4f 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -469,13 +469,10 @@ linkage_name_str (const lookup_name_info &lookup_name)
 /* See minsyms.h.  */
 
 void
-iterate_over_minimal_symbols (struct objfile *objf,
-			      const lookup_name_info &lookup_name,
-			      void (*callback) (struct minimal_symbol *,
-						void *),
-			      void *user_data)
+iterate_over_minimal_symbols
+    (struct objfile *objf, const lookup_name_info &lookup_name,
+     gdb::function_view<void (struct minimal_symbol *)> callback)
 {
-
   /* The first pass is over the ordinary hash table.  */
     {
       const char *name = linkage_name_str (lookup_name);
@@ -490,7 +487,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
 	   iter = iter->hash_next)
 	{
 	  if (mangled_cmp (MSYMBOL_LINKAGE_NAME (iter), name) == 0)
-	    (*callback) (iter, user_data);
+	    callback (iter);
 	}
     }
 
@@ -509,7 +506,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
 	   iter != NULL;
 	   iter = iter->demangled_hash_next)
 	if (name_match (MSYMBOL_SEARCH_NAME (iter), lookup_name, NULL))
-	  (*callback) (iter, user_data);
+	  callback (iter);
     }
 }
 
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index 11a202025d..a2b7ddd703 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -265,14 +265,11 @@ struct bound_minimal_symbol lookup_minimal_symbol_by_pc (CORE_ADDR);
    are considered.  The caller is responsible for canonicalizing NAME,
    should that need to be done.
 
-   For each matching symbol, CALLBACK is called with the symbol and
-   USER_DATA as arguments.  */
-
-void iterate_over_minimal_symbols (struct objfile *objf,
-				   const lookup_name_info &name,
-				   void (*callback) (struct minimal_symbol *,
-						     void *),
-				   void *user_data);
+   For each matching symbol, CALLBACK is called with the symbol.  */
+
+void iterate_over_minimal_symbols
+    (struct objfile *objf, const lookup_name_info &name,
+     gdb::function_view<void (struct minimal_symbol *)> callback);
 
 /* Compute the upper bound of MINSYM.  The upper bound is the last
    address thought to be part of the symbol.  If the symbol has a
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFA v2 04/10] Return std::string from canonical_to_fullform
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (3 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 03/10] Make copy_token_string return unique_xmalloc_ptr Tom Tromey
@ 2018-04-04  4:41 ` Tom Tromey
  2018-04-04  4:41 ` [RFA v2 08/10] More use of std::vector in linespec.c Tom Tromey
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-04-04  4:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes canonical_to_fullform to return a std::string, and
changes decode_line_2 to use std::vector.  This allows for the removal
of some cleanups.

gdb/ChangeLog
2018-04-03  Tom Tromey  <tom@tromey.com>

	* linespec.c (canonical_to_fullform): Return std::string.
	(filter_results): Update.
	(struct decode_line_2_item): Add constructor.
	<fullform, displayform>: Now std::string.
	(decode_line_2_compare_items): Now a std::sort comparator.
	(decode_line_2): Update.
---
 gdb/ChangeLog  |   9 +++++
 gdb/linespec.c | 115 ++++++++++++++++++++++++++-------------------------------
 2 files changed, 61 insertions(+), 63 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 5f4e95156b..c445a0be57 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1384,16 +1384,16 @@ find_toplevel_string (const char *haystack, const char *needle)
 }
 
 /* Convert CANONICAL to its string representation using
-   symtab_to_fullname for SYMTAB.  The caller must xfree the result.  */
+   symtab_to_fullname for SYMTAB.  */
 
-static char *
+static std::string
 canonical_to_fullform (const struct linespec_canonical_name *canonical)
 {
   if (canonical->symtab == NULL)
-    return xstrdup (canonical->suffix);
+    return canonical->suffix;
   else
-    return xstrprintf ("%s:%s", symtab_to_fullname (canonical->symtab),
-		       canonical->suffix);
+    return string_printf ("%s:%s", symtab_to_fullname (canonical->symtab),
+			  canonical->suffix);
 }
 
 /* Given FILTERS, a list of canonical names, filter the sals in RESULT
@@ -1414,17 +1414,12 @@ filter_results (struct linespec_state *self,
       for (size_t j = 0; j < result->size (); ++j)
 	{
 	  const struct linespec_canonical_name *canonical;
-	  char *fullform;
-	  struct cleanup *cleanup;
 
 	  canonical = &self->canonical_names[j];
-	  fullform = canonical_to_fullform (canonical);
-	  cleanup = make_cleanup (xfree, fullform);
+	  std::string fullform = canonical_to_fullform (canonical);
 
-	  if (strcmp (name, fullform) == 0)
+	  if (name == fullform)
 	    lsal.sals.push_back ((*result)[j]);
-
-	  do_cleanups (cleanup);
 	}
 
       if (!lsal.sals.empty ())
@@ -1458,34 +1453,35 @@ convert_results_to_lsals (struct linespec_state *self,
 
 struct decode_line_2_item
 {
-  /* The form using symtab_to_fullname.
-     It must be xfree'ed after use.  */
-  char *fullform;
+  decode_line_2_item (std::string &&fullform_, std::string &&displayform_,
+		      bool selected_)
+    : fullform (std::move (fullform_)),
+      displayform (std::move (displayform_)),
+      selected (selected_)
+  {
+  }
+
+  /* The form using symtab_to_fullname.  */
+  std::string fullform;
 
-  /* The form using symtab_to_filename_for_display.
-     It must be xfree'ed after use.  */
-  char *displayform;
+  /* The form using symtab_to_filename_for_display.  */
+  std::string displayform;
 
   /* Field is initialized to zero and it is set to one if the user
      requested breakpoint for this entry.  */
   unsigned int selected : 1;
 };
 
-/* Helper for qsort to sort decode_line_2_item entries by DISPLAYFORM and
-   secondarily by FULLFORM.  */
+/* Helper for std::sort to sort decode_line_2_item entries by
+   DISPLAYFORM and secondarily by FULLFORM.  */
 
-static int
-decode_line_2_compare_items (const void *ap, const void *bp)
+static bool
+decode_line_2_compare_items (const decode_line_2_item &a,
+			     const decode_line_2_item &b)
 {
-  const struct decode_line_2_item *a = (const struct decode_line_2_item *) ap;
-  const struct decode_line_2_item *b = (const struct decode_line_2_item *) bp;
-  int retval;
-
-  retval = strcmp (a->displayform, b->displayform);
-  if (retval != 0)
-    return retval;
-
-  return strcmp (a->fullform, b->fullform);
+  if (a.displayform != b.displayform)
+    return a.displayform < b.displayform;
+  return a.fullform < b.fullform;
 }
 
 /* Handle multiple results in RESULT depending on SELECT_MODE.  This
@@ -1503,8 +1499,7 @@ decode_line_2 (struct linespec_state *self,
   int i;
   struct cleanup *old_chain;
   VEC (const_char_ptr) *filters = NULL;
-  struct decode_line_2_item *items;
-  int items_count;
+  std::vector<struct decode_line_2_item> items;
 
   gdb_assert (select_mode != multiple_symbols_all);
   gdb_assert (self->canonical != NULL);
@@ -1513,56 +1508,50 @@ decode_line_2 (struct linespec_state *self,
   old_chain = make_cleanup (VEC_cleanup (const_char_ptr), &filters);
 
   /* Prepare ITEMS array.  */
-  items_count = result->size ();
-  items = XNEWVEC (struct decode_line_2_item, items_count);
-  make_cleanup (xfree, items);
-  for (i = 0; i < items_count; ++i)
+  for (i = 0; i < result->size (); ++i)
     {
       const struct linespec_canonical_name *canonical;
       struct decode_line_2_item *item;
 
+      std::string displayform;
+
       canonical = &self->canonical_names[i];
       gdb_assert (canonical->suffix != NULL);
-      item = &items[i];
 
-      item->fullform = canonical_to_fullform (canonical);
-      make_cleanup (xfree, item->fullform);
+      std::string fullform = canonical_to_fullform (canonical);
 
       if (canonical->symtab == NULL)
-	item->displayform = canonical->suffix;
+	displayform = canonical->suffix;
       else
 	{
 	  const char *fn_for_display;
 
 	  fn_for_display = symtab_to_filename_for_display (canonical->symtab);
-	  item->displayform = xstrprintf ("%s:%s", fn_for_display,
-					  canonical->suffix);
-	  make_cleanup (xfree, item->displayform);
+	  displayform = string_printf ("%s:%s", fn_for_display,
+				       canonical->suffix);
 	}
 
-      item->selected = 0;
+      items.emplace_back (std::move (fullform), std::move (displayform),
+			  false);
     }
 
   /* Sort the list of method names.  */
-  qsort (items, items_count, sizeof (*items), decode_line_2_compare_items);
+  std::sort (items.begin (), items.end (), decode_line_2_compare_items);
 
   /* Remove entries with the same FULLFORM.  */
-  if (items_count >= 2)
-    {
-      struct decode_line_2_item *dst, *src;
-
-      dst = items;
-      for (src = &items[1]; src < &items[items_count]; src++)
-	if (strcmp (src->fullform, dst->fullform) != 0)
-	  *++dst = *src;
-      items_count = dst + 1 - items;
-    }
-
-  if (select_mode == multiple_symbols_cancel && items_count > 1)
+  items.erase (std::unique (items.begin (), items.end (),
+			    [] (const struct decode_line_2_item &a,
+				const struct decode_line_2_item &b)
+			      {
+				return a.fullform == b.fullform;
+			      }),
+	       items.end ());
+
+  if (select_mode == multiple_symbols_cancel && items.size () > 1)
     error (_("canceled because the command is ambiguous\n"
 	     "See set/show multiple-symbol."));
   
-  if (select_mode == multiple_symbols_all || items_count == 1)
+  if (select_mode == multiple_symbols_all || items.size () == 1)
     {
       do_cleanups (old_chain);
       convert_results_to_lsals (self, result);
@@ -1570,8 +1559,8 @@ decode_line_2 (struct linespec_state *self,
     }
 
   printf_unfiltered (_("[0] cancel\n[1] all\n"));
-  for (i = 0; i < items_count; i++)
-    printf_unfiltered ("[%d] %s\n", i + 2, items[i].displayform);
+  for (i = 0; i < items.size (); i++)
+    printf_unfiltered ("[%d] %s\n", i + 2, items[i].displayform.c_str ());
 
   prompt = getenv ("PS2");
   if (prompt == NULL)
@@ -1604,7 +1593,7 @@ decode_line_2 (struct linespec_state *self,
 	}
 
       num -= 2;
-      if (num >= items_count)
+      if (num >= items.size ())
 	printf_unfiltered (_("No choice number %d.\n"), num);
       else
 	{
@@ -1612,7 +1601,7 @@ decode_line_2 (struct linespec_state *self,
 
 	  if (!item->selected)
 	    {
-	      VEC_safe_push (const_char_ptr, filters, item->fullform);
+	      VEC_safe_push (const_char_ptr, filters, item->fullform.c_str ());
 	      item->selected = 1;
 	    }
 	  else
-- 
2.13.6

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA v2 00/10] Remove some cleanups from linespec.c
  2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
                   ` (9 preceding siblings ...)
  2018-04-04  4:41 ` [RFA v2 10/10] Remove unnecessary include from linespec.h Tom Tromey
@ 2018-04-05  3:08 ` Simon Marchi
  10 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2018-04-05  3:08 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 2018-04-04 12:40 AM, Tom Tromey wrote:
> Here's the second version of the series to remove some cleanups from
> linespec.c.
> 
> I believe this version addresses all the review comments.
> 
> Regression tested by the buildbot.
> 
> Tom
> 

I took a quick look again, LGTM.

Thanks!

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-04-05  3:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-04  4:40 [RFA v2 00/10] Remove some cleanups from linespec.c Tom Tromey
2018-04-04  4:40 ` [RFA v2 07/10] Change streq to return bool Tom Tromey
2018-04-04  4:40 ` [RFA v2 02/10] Fix some indentation in linespec.c Tom Tromey
2018-04-04  4:41 ` [RFA v2 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
2018-04-04  4:41 ` [RFA v2 03/10] Make copy_token_string return unique_xmalloc_ptr Tom Tromey
2018-04-04  4:41 ` [RFA v2 04/10] Return std::string from canonical_to_fullform Tom Tromey
2018-04-04  4:41 ` [RFA v2 08/10] More use of std::vector in linespec.c Tom Tromey
2018-04-04  4:41 ` [RFA v2 05/10] Have filter_results take a std::vector Tom Tromey
2018-04-04  4:41 ` [RFA v2 09/10] Remove typep and VEC(typep) from linespec.c Tom Tromey
2018-04-04  4:41 ` [RFA v2 06/10] Remove a string copy from event_location_to_sals Tom Tromey
2018-04-04  4:41 ` [RFA v2 10/10] Remove unnecessary include from linespec.h Tom Tromey
2018-04-05  3:08 ` [RFA v2 00/10] Remove some cleanups from linespec.c Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).