public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 18/18] Remove the vector return result from the completion API.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (11 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 11/18] Implement completion limiting for reg_or_group_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 04/18] Implement completion limiting for add_filename_to_list Keith Seitz
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch completes the redesign of the completion and completion-
limiting API.  Since the completer's internal data is now responsible
for tracking all completions and the result of the completion operation
no longer uses vectors (except for the final list given to readline),
we can remove the vectors from the API entirely.

gdb/ChangeLog

	* command.h (completer_ftype): Change return type from
	VEC (char_ptr) * to void.  Update all users of this definition.
	* completer.h (maybe_add_completion): Update explanation of usage.
	* symtab.c (free_completion_list): Remove.
	(do_free_completion_list): Remove.
	(return_val): Remove global.
---
 gdb/ada-lang.c            |   26 +++-----
 gdb/break-catch-syscall.c |    8 +-
 gdb/breakpoint.c          |   19 ++----
 gdb/cli/cli-decode.c      |   34 ++++------
 gdb/command.h             |   18 +++--
 gdb/completer.c           |  154 ++++++++++++++++-----------------------------
 gdb/completer.h           |   50 +++++++--------
 gdb/corefile.c            |    4 +
 gdb/cp-abi.c              |    4 +
 gdb/f-lang.c              |    6 +-
 gdb/guile/scm-cmd.c       |   29 ++------
 gdb/infrun.c              |   12 +---
 gdb/interps.c             |   11 +--
 gdb/language.h            |    2 -
 gdb/python/py-cmd.c       |   10 +--
 gdb/symtab.c              |  103 +++++++-----------------------
 gdb/symtab.h              |   15 ++--
 gdb/value.c               |   11 +--
 gdb/value.h               |    4 +
 19 files changed, 184 insertions(+), 336 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 4b9b3d3..077ccf1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -6123,8 +6123,7 @@ symbol_completion_match (const char *sym_name,
    encoded).  */
 
 static enum maybe_add_completion_enum
-symbol_completion_add (VEC(char_ptr) **sv,
-		       struct completer_data *cdata,
+symbol_completion_add (struct completer_data *cdata,
                        const char *sym_name,
                        const char *text, int text_len,
                        const char *orig_text, const char *word,
@@ -6166,7 +6165,6 @@ symbol_completion_add (VEC(char_ptr) **sv,
     {
     case MAYBE_ADD_COMPLETION_OK:
     case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-      VEC_safe_push (char_ptr, *sv, completion);
       break;
     case MAYBE_ADD_COMPLETION_MAX_REACHED:
     case MAYBE_ADD_COMPLETION_DUPLICATE:
@@ -6180,7 +6178,6 @@ symbol_completion_add (VEC(char_ptr) **sv,
    expand_symtabs_matching method.  */
 struct add_partial_datum
 {
-  VEC(char_ptr) **completions;
   const char *text;
   int text_len;
   const char *text0;
@@ -6203,7 +6200,7 @@ ada_complete_symbol_matcher (const char *name, void *user_data)
 /* Return a list of possible symbol names completing TEXT0.  WORD is
    the entire command on which completion is made.  */
 
-static VEC (char_ptr) *
+static void
 ada_make_symbol_completion_list (struct completer_data *cdata,
 				 const char *text0, const char *word,
 				 enum type_code code)
@@ -6212,7 +6209,6 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
   int text_len;
   int wild_match_p;
   int encoded_p;
-  VEC(char_ptr) *completions = VEC_alloc (char_ptr, 128);
   struct symbol *sym;
   struct compunit_symtab *s;
   struct minimal_symbol *msymbol;
@@ -6253,7 +6249,6 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
   {
     struct add_partial_datum data;
 
-    data.completions = &completions;
     data.text = text;
     data.text_len = text_len;
     data.text0 = text0;
@@ -6272,7 +6267,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
   ALL_MSYMBOLS (objfile, msymbol)
   {
     QUIT;
-    add_status = symbol_completion_add (&completions, cdata,
+    add_status = symbol_completion_add (cdata,
 					MSYMBOL_LINKAGE_NAME (msymbol),
 					text, text_len, text0, word,
 					wild_match_p, encoded_p);
@@ -6283,7 +6278,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
       case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
       case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	do_cleanups (old_chain);
-	return completions;
+	return;
       case MAYBE_ADD_COMPLETION_DUPLICATE:
 	break;
       }
@@ -6299,7 +6294,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
 
       ALL_BLOCK_SYMBOLS (b, iter, sym)
       {
-        add_status = symbol_completion_add (&completions, cdata,
+        add_status = symbol_completion_add (cdata,
 					    SYMBOL_LINKAGE_NAME (sym),
 					    text, text_len, text0, word,
 					    wild_match_p, encoded_p);
@@ -6310,7 +6305,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
 	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
 	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	    do_cleanups (old_chain);
-	    return completions;
+	    return;
 	  case MAYBE_ADD_COMPLETION_DUPLICATE:
 	    break;
 	  }
@@ -6326,7 +6321,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
     b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (s), GLOBAL_BLOCK);
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      add_status = symbol_completion_add (&completions, cdata,
+      add_status = symbol_completion_add (cdata,
 					  SYMBOL_LINKAGE_NAME (sym),
 					  text, text_len, text0, word,
 					  wild_match_p, encoded_p);
@@ -6337,7 +6332,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
 	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
 	case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	  do_cleanups (old_chain);
-	  return completions;
+	  return;
 	case MAYBE_ADD_COMPLETION_DUPLICATE:
 	  break;
 	}
@@ -6353,7 +6348,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
       continue;
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      add_status = symbol_completion_add (&completions, cdata,
+      add_status = symbol_completion_add (cdata,
 					 SYMBOL_LINKAGE_NAME (sym),
 					 text, text_len, text0, word,
 					 wild_match_p, encoded_p);
@@ -6364,7 +6359,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
 	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
 	case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	  do_cleanups (old_chain);
-	  return completions;
+	  return;
 	case MAYBE_ADD_COMPLETION_DUPLICATE:
 	  break;
 	}
@@ -6372,7 +6367,6 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
   }
 
   do_cleanups (old_chain);
-  return completions;
 }
 
                                 /* Field Access */
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 4677132..a542826 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -591,17 +591,17 @@ catching_syscall_number (int syscall_number)
 }
 
 /* Complete syscall names.  Used by "catch syscall".  */
-static VEC (char_ptr) *
+static void
 catch_syscall_completer (struct completer_data *cdata,
 			 struct cmd_list_element *cmd,
                          const char *text, const char *word)
 {
   const char **list = get_syscall_names (get_current_arch ());
-  VEC (char_ptr) *retlist
-    = (list == NULL) ? NULL : complete_on_enum (cdata, list, word, word);
+
+  if (list != NULL)
+    complete_on_enum (cdata, list, word, word);
 
   xfree (list);
-  return retlist;
 }
 
 static void
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 04dc734..894e779 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1032,7 +1032,7 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
 
 /* Completion for the "condition" command.  */
 
-static VEC (char_ptr) *
+static void
 condition_completer (struct completer_data *cdata,
 		     struct cmd_list_element *cmd,
 		     const char *text, const char *word)
@@ -1045,14 +1045,13 @@ condition_completer (struct completer_data *cdata,
     {
       int len;
       struct breakpoint *b;
-      VEC (char_ptr) *result = NULL;
 
       if (text[0] == '$')
 	{
 	  /* We don't support completion of history indices.  */
-	  if (isdigit (text[1]))
-	    return NULL;
-	  return complete_internalvar (cdata, &text[1]);
+	  if (!isdigit (text[1]))
+	    complete_internalvar (cdata, &text[1]);
+	  return;
 	}
 
       /* We're completing the breakpoint number.  */
@@ -1073,14 +1072,12 @@ condition_completer (struct completer_data *cdata,
 	      switch (add_status)
 		{
 		case MAYBE_ADD_COMPLETION_OK:
-		  VEC_safe_push (char_ptr, result, completion);
 		  break;
 		case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-		  VEC_safe_push (char_ptr, result, completion);
-		  return result;
+		  return;
 		case MAYBE_ADD_COMPLETION_MAX_REACHED:
 		  xfree (completion);
-		  return result;
+		  return;
 		case MAYBE_ADD_COMPLETION_DUPLICATE:
 		  xfree (completion);
 		  break;
@@ -1088,12 +1085,12 @@ condition_completer (struct completer_data *cdata,
 	    }
 	}
 
-      return result;
+      return;
     }
 
   /* We're completing the expression part.  */
   text = skip_spaces_const (space);
-  return expression_completer (cdata, cmd, text, word);
+  expression_completer (cdata, cmd, text, word);
 }
 
 /* condition N EXP -- set break condition of breakpoint N to EXP.  */
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 4e0fbf7..3c205c1 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -647,7 +647,7 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass
 /* Completes on literal "unlimited".  Used by integer commands that
    support a special "unlimited" value.  */
 
-static VEC (char_ptr) *
+static void
 integer_unlimited_completer (struct completer_data *cdata,
 			     struct cmd_list_element *ignore,
 			     const char *text, const char *word)
@@ -658,7 +658,7 @@ integer_unlimited_completer (struct completer_data *cdata,
       NULL,
     };
 
-  return complete_on_enum (cdata, keywords, text, word);
+  complete_on_enum (cdata, keywords, text, word);
 }
 
 /* Add element named NAME to both the set and show command LISTs (the
@@ -1757,22 +1757,21 @@ lookup_cmd_composition (const char *text,
 
 /* Helper function for SYMBOL_COMPLETION_FUNCTION.  */
 
-/* Return a vector of char pointers which point to the different
-   possible completions in LIST of TEXT.
+/* Add completions to CDATA for the different possible completions
+   in LIST of TEXT.
 
    WORD points in the same buffer as TEXT, and completions should be
    returned relative to this position.  For example, suppose TEXT is
    "foo" and we want to complete to "foobar".  If WORD is "oo", return
    "oobar"; if WORD is "baz/foo", return "baz/foobar".  */
 
-VEC (char_ptr) *
+void
 complete_on_cmdlist (struct completer_data *cdata,
 		     struct cmd_list_element *list,
 		     const char *text, const char *word,
 		     int ignore_help_classes)
 {
   struct cmd_list_element *ptr;
-  VEC (char_ptr) *matchlist = NULL;
   int textlen = strlen (text);
   int pass;
   int saw_deprecated_match = 0;
@@ -1820,14 +1819,12 @@ complete_on_cmdlist (struct completer_data *cdata,
 	    switch (add_status)
 	      {
 	      case MAYBE_ADD_COMPLETION_OK:
-		VEC_safe_push (char_ptr, matchlist, match);
 		break;
 	      case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-		VEC_safe_push (char_ptr, matchlist, match);
-		return matchlist;
+		return;
 	      case MAYBE_ADD_COMPLETION_MAX_REACHED:
 		xfree (match);
-		return matchlist;
+		return;
 	      case MAYBE_ADD_COMPLETION_DUPLICATE:
 		xfree (match);
 		break;
@@ -1838,26 +1835,23 @@ complete_on_cmdlist (struct completer_data *cdata,
       if (!saw_deprecated_match)
 	break;
     }
-
-  return matchlist;
 }
 
 /* Helper function for SYMBOL_COMPLETION_FUNCTION.  */
 
-/* Return a vector of char pointers which point to the different
-   possible completions in CMD of TEXT.
+/* Add completions to CDATA for the different possible completions in
+   CMD of TEXT.
 
    WORD points in the same buffer as TEXT, and completions should be
    returned relative to this position.  For example, suppose TEXT is "foo"
    and we want to complete to "foobar".  If WORD is "oo", return
    "oobar"; if WORD is "baz/foo", return "baz/foobar".  */
 
-VEC (char_ptr) *
+void
 complete_on_enum (struct completer_data *cdata,
 		  const char *const *enumlist,
 		  const char *text, const char *word)
 {
-  VEC (char_ptr) *matchlist = NULL;
   int textlen = strlen (text);
   int i;
   const char *name;
@@ -1888,21 +1882,17 @@ complete_on_enum (struct completer_data *cdata,
 	switch (add_status)
 	  {
 	  case MAYBE_ADD_COMPLETION_OK:
-	    VEC_safe_push (char_ptr, matchlist, match);
 	    break;
 	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	    VEC_safe_push (char_ptr, matchlist, match);
-	    return matchlist;
+	    return;
 	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	    xfree (match);
-	    return matchlist;
+	    return;
 	  case MAYBE_ADD_COMPLETION_DUPLICATE:
 	    xfree (match);
 	    break;
 	  }
       }
-
-  return matchlist;
 }
 
 
diff --git a/gdb/command.h b/gdb/command.h
index b2aeb30..82872c0 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -158,9 +158,9 @@ typedef void cmd_sfunc_ftype (char *args, int from_tty,
 extern void set_cmd_sfunc (struct cmd_list_element *cmd,
 			   cmd_sfunc_ftype *sfunc);
 
-typedef VEC (char_ptr) *completer_ftype (struct completer_data *,
-					 struct cmd_list_element *,
-					 const char *, const char *);
+typedef void completer_ftype (struct completer_data *,
+			      struct cmd_list_element *,
+			      const char *, const char *);
 
 typedef void completer_ftype_void (struct cmd_list_element *,
 				   const char *, const char *);
@@ -228,13 +228,13 @@ extern struct cmd_list_element *add_info (const char *,
 extern struct cmd_list_element *add_info_alias (const char *, const char *,
 						int);
 
-extern VEC (char_ptr) *complete_on_cmdlist (struct completer_data *,
-					    struct cmd_list_element *,
-					    const char *, const char *, int);
+extern void complete_on_cmdlist (struct completer_data *,
+				 struct cmd_list_element *,
+				 const char *, const char *, int);
 
-extern VEC (char_ptr) *complete_on_enum (struct completer_data *,
-					 const char *const *enumlist,
-					 const char *, const char *);
+extern void complete_on_enum (struct completer_data *,
+			      const char *const *enumlist,
+			      const char *, const char *);
 
 /* Functions that implement commands about CLI commands.  */
 
diff --git a/gdb/completer.c b/gdb/completer.c
index 425b5aa..d29df1a 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -122,22 +122,23 @@ readline_line_completion_function (const char *text, int matches)
 
 /* This can be used for functions which don't want to complete on
    symbols but don't want to complete on anything else either.  */
-VEC (char_ptr) *
+
+void
 noop_completer (struct completer_data *cdata,
 		struct cmd_list_element *ignore,
 		const char *text, const char *prefix)
 {
-  return NULL;
+  /* Nothing.  */
 }
 
 /* Complete on filenames.  */
-VEC (char_ptr) *
+
+void
 filename_completer (struct completer_data *cdata,
 		    struct cmd_list_element *ignore,
 		    const char *text, const char *word)
 {
   int subsequent_name;
-  VEC (char_ptr) *return_val = NULL;
 
   subsequent_name = 0;
   while (1)
@@ -184,14 +185,12 @@ filename_completer (struct completer_data *cdata,
       switch (add_status)
 	{
 	case MAYBE_ADD_COMPLETION_OK:
-	  VEC_safe_push (char_ptr, return_val, q);
 	  break;
 	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	  VEC_safe_push (char_ptr, return_val, q);
-	  return return_val;
+	  return;
 	case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	  xfree (q);
-	  return return_val;
+	  return;
 	case MAYBE_ADD_COMPLETION_DUPLICATE:
 	  xfree (q);
 	  break;
@@ -205,7 +204,6 @@ filename_completer (struct completer_data *cdata,
      with respect to inserting quotes.  */
   rl_completer_word_break_characters = "";
 #endif
-  return return_val;
 }
 
 /* A hashtable traversal function to remove leading file name
@@ -231,14 +229,12 @@ remove_leading_fn_component (void **slot, void *calldata)
    This is intended to be used in commands that set breakpoints
    etc.  */
 
-VEC (char_ptr) *
+void
 location_completer (struct completer_data *cdata,
 		    struct cmd_list_element *ignore,
 		    const char *text, const char *word)
 {
   int n_syms, n_files, ix;
-  VEC (char_ptr) *fn_list = NULL;
-  VEC (char_ptr) *list = NULL;
   const char *p;
   int quote_found = 0;
   int quoted = *text == '\'' || *text == '"';
@@ -309,15 +305,15 @@ location_completer (struct completer_data *cdata,
      symbols as well as on files.  */
   if (colon)
     {
-      list = make_file_symbol_completion_list (cdata, symbol_start, word,
-					       file_to_match);
+      make_file_symbol_completion_list (cdata, symbol_start, word,
+					file_to_match);
       n_syms = get_completion_count (cdata);
       n_files = 0;
       xfree (file_to_match);
     }
   else
     {
-      list = make_symbol_completion_list (cdata, symbol_start, word);
+      make_symbol_completion_list (cdata, symbol_start, word);
       n_syms = get_completion_count (cdata);
       n_files = 0;
       /* If text includes characters which cannot appear in a file
@@ -325,27 +321,11 @@ location_completer (struct completer_data *cdata,
       if (strcspn (text, 
 		   gdb_completer_file_name_break_characters) == text_len)
 	{
-	  fn_list = make_source_files_completion_list (cdata, text, text);
+	  make_source_files_completion_list (cdata, text, text);
 	  n_files = get_completion_count (cdata) - n_syms;
 	}
     }
 
-  /* Catenate fn_list[] onto the end of list[].  */
-  if (!n_syms)
-    {
-      VEC_free (char_ptr, list); /* Paranoia.  */
-      list = fn_list;
-      fn_list = NULL;
-    }
-  else
-    {
-      char *fn;
-
-      for (ix = 0; VEC_iterate (char_ptr, fn_list, ix, fn); ++ix)
-	VEC_safe_push (char_ptr, list, fn);
-      VEC_free (char_ptr, fn_list);
-    }
-
   if (n_syms && n_files)
     {
       /* Nothing.  */
@@ -376,18 +356,15 @@ location_completer (struct completer_data *cdata,
     {
       /* No completions at all.  As the final resort, try completing
 	 on the entire text as a symbol.  */
-      list = make_symbol_completion_list (cdata, orig_text, word);
+      make_symbol_completion_list (cdata, orig_text, word);
     }
-
-  return list;
 }
 
 /* Helper for expression_completer which recursively adds field and
    method names from TYPE, a struct or union type, to the array
    OUTPUT.  */
 static void
-add_struct_fields (struct completer_data *cdata,
-		   struct type *type, VEC (char_ptr) **output,
+add_struct_fields (struct completer_data *cdata, struct type *type,
 		   char *fieldname, int namelen)
 {
   int i;
@@ -399,7 +376,7 @@ add_struct_fields (struct completer_data *cdata,
     {
       if (i < TYPE_N_BASECLASSES (type))
 	add_struct_fields (cdata, TYPE_BASECLASS (type, i),
-			   output, fieldname, namelen);
+			   fieldname, namelen);
       else if (TYPE_FIELD_NAME (type, i))
 	{
 	  if (TYPE_FIELD_NAME (type, i)[0] != '\0')
@@ -414,10 +391,8 @@ add_struct_fields (struct completer_data *cdata,
 		  switch (add_status)
 		    {
 		    case MAYBE_ADD_COMPLETION_OK:
-		      VEC_safe_push (char_ptr, *output, match);
 		      break;
 		    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-		      VEC_safe_push (char_ptr, *output, match);
 		      return;
 		    case MAYBE_ADD_COMPLETION_MAX_REACHED:
 		      xfree (match);
@@ -432,7 +407,7 @@ add_struct_fields (struct completer_data *cdata,
 	    {
 	      /* Recurse into anonymous unions.  */
 	      add_struct_fields (cdata, TYPE_FIELD_TYPE (type, i),
-				 output, fieldname, namelen);
+				 fieldname, namelen);
 	    }
 	}
     }
@@ -458,10 +433,8 @@ add_struct_fields (struct completer_data *cdata,
 	      switch (add_status)
 		{
 		case MAYBE_ADD_COMPLETION_OK:
-		  VEC_safe_push (char_ptr, *output, match);
 		  break;
 		case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-		  VEC_safe_push (char_ptr, *output, match);
 		  return;
 		case MAYBE_ADD_COMPLETION_MAX_REACHED:
 		  xfree (match);
@@ -478,7 +451,8 @@ add_struct_fields (struct completer_data *cdata,
 /* Complete on expressions.  Often this means completing on symbol
    names, but some language parsers also have support for completing
    field names.  */
-VEC (char_ptr) *
+
+void
 expression_completer (struct completer_data *cdata,
 		      struct cmd_list_element *ignore,
 		      const char *text, const char *word)
@@ -497,7 +471,7 @@ expression_completer (struct completer_data *cdata,
     }
   CATCH (except, RETURN_MASK_ERROR)
     {
-      return NULL;
+      return;
     }
   END_CATCH
 
@@ -516,22 +490,19 @@ expression_completer (struct completer_data *cdata,
 	  || TYPE_CODE (type) == TYPE_CODE_STRUCT)
 	{
 	  int flen = strlen (fieldname);
-	  VEC (char_ptr) *result = NULL;
 
-	  add_struct_fields (cdata, type, &result, fieldname, flen);
+	  add_struct_fields (cdata, type, fieldname, flen);
 	  xfree (fieldname);
-	  return result;
+	  return;
 	}
     }
   else if (fieldname && code != TYPE_CODE_UNDEF)
     {
-      VEC (char_ptr) *result;
       struct cleanup *cleanup = make_cleanup (xfree, fieldname);
 
-      result
-	= make_symbol_completion_type (cdata, fieldname, fieldname, code);
+      make_symbol_completion_type (cdata, fieldname, fieldname, code);
       do_cleanups (cleanup);
-      return result;
+      return;
     }
   xfree (fieldname);
 
@@ -543,7 +514,7 @@ expression_completer (struct completer_data *cdata,
     ;
 
   /* Not ideal but it is what we used to do before...  */
-  return location_completer (cdata, ignore, p, word);
+  location_completer (cdata, ignore, p, word);
 }
 
 /* See definition in completer.h.  */
@@ -620,12 +591,11 @@ complete_line_internal_reason;
    once sub-command completions are exhausted, we simply return NULL.
  */
 
-static VEC (char_ptr) *
+static void
 complete_line_internal (struct completer_data *cdata,
 			const char *text, const char *line_buffer, int point,
 			complete_line_internal_reason reason)
 {
-  VEC (char_ptr) *list = NULL;
   char *tmp_command;
   const char *p;
   int ignore_help_classes;
@@ -680,7 +650,7 @@ complete_line_internal (struct completer_data *cdata,
     {
       /* It is an unrecognized command.  So there are no
 	 possible completions.  */
-      list = NULL;
+      return;
     }
   else if (c == CMD_LIST_AMBIGUOUS)
     {
@@ -698,7 +668,7 @@ complete_line_internal (struct completer_data *cdata,
 	     example, "info t " or "info t foo" does not complete
 	     to anything, because "info t" can be "info target" or
 	     "info terminal".  */
-	  list = NULL;
+	  return;
 	}
       else
 	{
@@ -707,14 +677,14 @@ complete_line_internal (struct completer_data *cdata,
 	  if (result_list)
 	    {
 	      if (reason != handle_brkchars)
-		list = complete_on_cmdlist (cdata, *result_list->prefixlist,
-					    p, word, ignore_help_classes);
+		complete_on_cmdlist (cdata, *result_list->prefixlist,
+				     p, word, ignore_help_classes);
 	    }
 	  else
 	    {
 	      if (reason != handle_brkchars)
-		list = complete_on_cmdlist (cdata, cmdlist, p, word,
-					    ignore_help_classes);
+		complete_on_cmdlist (cdata, cmdlist, p, word,
+				     ignore_help_classes);
 	    }
 	  /* Ensure that readline does the right thing with respect to
 	     inserting quotes.  */
@@ -740,8 +710,8 @@ complete_line_internal (struct completer_data *cdata,
 		  /* It is a prefix command; what comes after it is
 		     a subcommand (e.g. "info ").  */
 		  if (reason != handle_brkchars)
-		    list = complete_on_cmdlist (cdata, *c->prefixlist, p,
-						word, ignore_help_classes);
+		    complete_on_cmdlist (cdata, *c->prefixlist, p,
+					 word, ignore_help_classes);
 
 		  /* Ensure that readline does the right thing
 		     with respect to inserting quotes.  */
@@ -749,11 +719,11 @@ complete_line_internal (struct completer_data *cdata,
 		    gdb_completer_command_word_break_characters;
 		}
 	      else if (reason == handle_help)
-		list = NULL;
+		return;
 	      else if (c->enums)
 		{
 		  if (reason != handle_brkchars)
-		    list = complete_on_enum (cdata, c->enums, p, word);
+		    complete_on_enum (cdata, c->enums, p, word);
 		  rl_completer_word_break_characters =
 		    gdb_completer_command_word_break_characters;
 		}
@@ -793,7 +763,7 @@ complete_line_internal (struct completer_data *cdata,
 		      && c->completer_handle_brkchars != NULL)
 		    (*c->completer_handle_brkchars) (c, p, word);
 		  if (reason != handle_brkchars && c->completer != NULL)
-		    list = (*c->completer) (cdata, c, p, word);
+		    (*c->completer) (cdata, c, p, word);
 		}
 	    }
 	  else
@@ -815,8 +785,8 @@ complete_line_internal (struct completer_data *cdata,
 		}
 
 	      if (reason != handle_brkchars)
-		list = complete_on_cmdlist (cdata, result_list, q, word,
-					    ignore_help_classes);
+		complete_on_cmdlist (cdata, result_list, q, word,
+				     ignore_help_classes);
 
 	      /* Ensure that readline does the right thing
 		 with respect to inserting quotes.  */
@@ -825,7 +795,7 @@ complete_line_internal (struct completer_data *cdata,
 	    }
 	}
       else if (reason == handle_help)
-	list = NULL;
+	return;
       else
 	{
 	  /* There is non-whitespace beyond the command.  */
@@ -834,12 +804,12 @@ complete_line_internal (struct completer_data *cdata,
 	    {
 	      /* It is an unrecognized subcommand of a prefix command,
 		 e.g. "info adsfkdj".  */
-	      list = NULL;
+	      return;
 	    }
 	  else if (c->enums)
 	    {
 	      if (reason != handle_brkchars)
-		list = complete_on_enum (cdata, c->enums, p, word);
+		complete_on_enum (cdata, c->enums, p, word);
 	    }
 	  else
 	    {
@@ -869,12 +839,10 @@ complete_line_internal (struct completer_data *cdata,
 		  && c->completer_handle_brkchars != NULL)
 		(*c->completer_handle_brkchars) (c, p, word);
 	      if (reason != handle_brkchars && c->completer != NULL)
-		list = (*c->completer) (cdata, c, p, word);
+		(*c->completer) (cdata, c, p, word);
 	    }
 	}
     }
-
-  return list;
 }
 
 /* Allocate a new completer data structure.  */
@@ -955,7 +923,6 @@ get_completion_count (const struct completer_data *cdata)
   return htab_elements (cdata->tracker);
 }
 
-
 /* See completer.h.  */
 
 int
@@ -1033,23 +1000,23 @@ complete_line (const char *text, const char *line_buffer, int point)
 }
 
 /* Complete on command names.  Used by "help".  */
-VEC (char_ptr) *
+
+void
 command_completer (struct completer_data *cdata,
 		   struct cmd_list_element *ignore,
 		   const char *text, const char *word)
 {
-  return complete_line_internal (cdata, word, text,
-				 strlen (text), handle_help);
+  complete_line_internal (cdata, word, text,
+			  strlen (text), handle_help);
 }
 
 /* Complete on signals.  */
 
-VEC (char_ptr) *
+void
 signal_completer (struct completer_data *cdata,
 		  struct cmd_list_element *ignore,
 		  const char *text, const char *word)
 {
-  VEC (char_ptr) *return_val = NULL;
   size_t len = strlen (word);
   int signum;
   const char *signame;
@@ -1075,32 +1042,27 @@ signal_completer (struct completer_data *cdata,
 	  switch (add_status)
 	    {
 	    case MAYBE_ADD_COMPLETION_OK:
-	      VEC_safe_push (char_ptr, return_val, completion);
 	      break;
 	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	      VEC_safe_push (char_ptr, return_val, completion);
-	      return return_val;
+	      return;
 	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	      xfree (completion);
-	      return return_val;
+	      return;
 	    case MAYBE_ADD_COMPLETION_DUPLICATE:
 	      xfree (completion);
 	      break;
 	    }
 	}
     }
-
-  return return_val;
 }
 
 /* Complete on a register or reggroup.  */
 
-VEC (char_ptr) *
+void
 reg_or_group_completer (struct completer_data *cdata,
 			struct cmd_list_element *ignore,
 			const char *text, const char *word)
 {
-  VEC (char_ptr) *result = NULL;
   size_t len = strlen (word);
   struct gdbarch *gdbarch;
   struct reggroup *group;
@@ -1109,7 +1071,7 @@ reg_or_group_completer (struct completer_data *cdata,
   enum maybe_add_completion_enum add_status;
 
   if (!target_has_registers)
-    return result;
+    return;
 
   gdbarch = get_frame_arch (get_selected_frame (NULL));
 
@@ -1125,14 +1087,12 @@ reg_or_group_completer (struct completer_data *cdata,
 	  switch (add_status)
 	    {
 	    case MAYBE_ADD_COMPLETION_OK:
-	      VEC_safe_push (char_ptr, result, match);
 	      break;
 	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	      VEC_safe_push (char_ptr, result, match);
-	      return result;
+	      return;
 	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	      xfree (match);
-	      return result;
+	      return;
 	    case MAYBE_ADD_COMPLETION_DUPLICATE:
 	      xfree (match);
 	      break;
@@ -1152,22 +1112,18 @@ reg_or_group_completer (struct completer_data *cdata,
 	  switch (add_status)
 	    {
 	    case MAYBE_ADD_COMPLETION_OK:
-	      VEC_safe_push (char_ptr, result, match);
 	      break;
 	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	      VEC_safe_push (char_ptr, result, match);
-	      return result;
+	      return;
 	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	      xfree (match);
-	      return result;
+	      return;
 	    case MAYBE_ADD_COMPLETION_DUPLICATE:
 	      xfree (match);
 	      break;
 	    }
 	}
     }
-
-  return result;
 }
 
 
diff --git a/gdb/completer.h b/gdb/completer.h
index 0724590..8a01d9f 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -76,33 +76,33 @@ extern VEC (char_ptr) *complete_line (const char *text,
 extern char *readline_line_completion_function (const char *text,
 						int matches);
 
-extern VEC (char_ptr) *noop_completer (struct completer_data *,
-				       struct cmd_list_element *,
-				       const char *, const char *);
+extern void noop_completer (struct completer_data *,
+			    struct cmd_list_element *,
+			    const char *, const char *);
 
-extern VEC (char_ptr) *filename_completer (struct completer_data *,
-					   struct cmd_list_element *,
-					   const char *, const char *);
+extern void filename_completer (struct completer_data *,
+				struct cmd_list_element *,
+				const char *, const char *);
 
-extern VEC (char_ptr) *expression_completer (struct completer_data *,
-					     struct cmd_list_element *,
-					     const char *, const char *);
+extern void expression_completer (struct completer_data *,
+				  struct cmd_list_element *,
+				  const char *, const char *);
 
-extern VEC (char_ptr) *location_completer (struct completer_data *,
-					   struct cmd_list_element *,
-					   const char *, const char *);
+extern void location_completer (struct completer_data *,
+				struct cmd_list_element *,
+				const char *, const char *);
 
-extern VEC (char_ptr) *command_completer (struct completer_data *,
-					  struct cmd_list_element *,
-					  const char *, const char *);
+extern void command_completer (struct completer_data *,
+			       struct cmd_list_element *,
+			       const char *, const char *);
 
-extern VEC (char_ptr) *signal_completer (struct completer_data *,
-					 struct cmd_list_element *,
-					 const char *, const char *);
+extern void signal_completer (struct completer_data *,
+			      struct cmd_list_element *,
+			      const char *, const char *);
 
-extern VEC (char_ptr) *reg_or_group_completer (struct completer_data *,
-					       struct cmd_list_element *,
-					       const char *, const char *);
+extern void reg_or_group_completer (struct completer_data *,
+				    struct cmd_list_element *,
+				    const char *, const char *);
 
 extern char *get_gdb_completer_quote_characters (void);
 
@@ -161,11 +161,11 @@ enum maybe_add_completion_enum
 
 /* Add the completion NAME to the list of generated completions if
    it is not there already.
-   If max_completions is negative, nothing is done, not even watching
-   for duplicates, and MAYBE_ADD_COMPLETION_OK is always returned.
+   If max_completions is negative, completions are not limited, but
+   duplicates are filtered.  MAYBE_ADD_COMPLETION_OK is always returned.
 
-   If MAYBE_ADD_COMPLETION_MAX_REACHED is returned, callers are required to
-   record at least one more completion.  The final list will be pruned to
+   If MAYBE_ADD_COMPLETION_OK_MAX_REACHED is returned, callers are required
+   to record at least one more completion.  The final list will be limited to
    max_completions, but recording at least one more than max_completions is
    the signal to the completion machinery that too many completions were
    found.  */
diff --git a/gdb/corefile.c b/gdb/corefile.c
index b5ec0e0..4b8d504 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -467,7 +467,7 @@ set_gnutarget_command (char *ignore, int from_tty,
 
 /* A completion function for "set gnutarget".  */
 
-static VEC (char_ptr) *
+static void
 complete_set_gnutarget (struct completer_data *cdata,
 			struct cmd_list_element *cmd,
 			const char *text, const char *word)
@@ -487,7 +487,7 @@ complete_set_gnutarget (struct completer_data *cdata,
       bfd_targets[last + 1] = NULL;
     }
 
-  return complete_on_enum (cdata, bfd_targets, text, word);
+  complete_on_enum (cdata, bfd_targets, text, word);
 }
 
 /* Set the gnutarget.  */
diff --git a/gdb/cp-abi.c b/gdb/cp-abi.c
index 504bb20..cddc382 100644
--- a/gdb/cp-abi.c
+++ b/gdb/cp-abi.c
@@ -358,7 +358,7 @@ set_cp_abi_cmd (char *args, int from_tty)
 
 /* A completion function for "set cp-abi".  */
 
-static VEC (char_ptr) *
+static void
 cp_abi_completer (struct completer_data *cdata,
 		  struct cmd_list_element *ignore,
 		  const char *text, const char *word)
@@ -375,7 +375,7 @@ cp_abi_completer (struct completer_data *cdata,
       cp_abi_names[i] = NULL;
     }
 
-  return complete_on_enum (cdata, cp_abi_names, text, word);
+  complete_on_enum (cdata, cp_abi_names, text, word);
 }
 
 /* Show the currently selected C++ ABI.  */
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 6852b08..ed278fc 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -228,13 +228,13 @@ f_word_break_characters (void)
 /* Consider the modules separator :: as a valid symbol name character
    class.  */
 
-static VEC (char_ptr) *
+static void
 f_make_symbol_completion_list (struct completer_data *cdata,
 			       const char *text, const char *word,
 			       enum type_code code)
 {
-  return default_make_symbol_completion_list_break_on (cdata, text, word,
-						       ":", code);
+  default_make_symbol_completion_list_break_on (cdata, text, word,
+						":", code);
 }
 
 const struct language_defn f_language_defn =
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 1680c27..86fc321 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -348,8 +348,7 @@ cmdscm_bad_completion_result (const char *msg, SCM completion)
    The result is a boolean indicating success.  */
 
 static int
-cmdscm_add_completion (SCM completion, struct completer_data *cdata,
-		       VEC (char_ptr) **result)
+cmdscm_add_completion (SCM completion, struct completer_data *cdata)
 {
   char *item;
   SCM except_scm;
@@ -377,7 +376,6 @@ cmdscm_add_completion (SCM completion, struct completer_data *cdata,
     {
     case MAYBE_ADD_COMPLETION_OK:
     case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-      VEC_safe_push (char_ptr, *result, item);
       break;
     case MAYBE_ADD_COMPLETION_MAX_REACHED:
     case MAYBE_ADD_COMPLETION_DUPLICATE:
@@ -390,7 +388,7 @@ cmdscm_add_completion (SCM completion, struct completer_data *cdata,
 
 /* Called by gdb for command completion.  */
 
-static VEC (char_ptr) *
+static void
 cmdscm_completer (struct completer_data *cdata,
 		  struct cmd_list_element *command,
 		  const char *text, const char *word)
@@ -398,7 +396,6 @@ cmdscm_completer (struct completer_data *cdata,
   command_smob *c_smob/*obj*/ = (command_smob *) get_cmd_context (command);
   SCM completer_result_scm;
   SCM text_scm, word_scm, result_scm;
-  VEC (char_ptr) *result = NULL;
 
   gdb_assert (c_smob != NULL);
   gdb_assert (gdbscm_is_procedure (c_smob->complete));
@@ -420,7 +417,7 @@ cmdscm_completer (struct completer_data *cdata,
     {
       /* Inform the user, but otherwise ignore.  */
       gdbscm_print_gdb_exception (SCM_BOOL_F, completer_result_scm);
-      goto done;
+      return;
     }
 
   if (gdbscm_is_true (scm_list_p (completer_result_scm)))
@@ -431,11 +428,8 @@ cmdscm_completer (struct completer_data *cdata,
 	{
 	  SCM next = scm_car (list);
 
-	  if (!cmdscm_add_completion (next, cdata, &result))
-	    {
-	      VEC_free (char_ptr, result);
-	      goto done;
-	    }
+	  if (!cmdscm_add_completion (next, cdata))
+	    return;
 
 	  list = scm_cdr (list);
 	}
@@ -451,15 +445,11 @@ cmdscm_completer (struct completer_data *cdata,
 	    {
 	      /* Inform the user, but otherwise ignore the entire result.  */
 	      gdbscm_print_gdb_exception (SCM_BOOL_F, completer_result_scm);
-	      VEC_free (char_ptr, result);
-	      goto done;
+	      return;
 	    }
 
-	  if (!cmdscm_add_completion (next, cdata, &result))
-	    {
-	      VEC_free (char_ptr, result);
-	      goto done;
-	    }
+	  if (!cmdscm_add_completion (next, cdata))
+	    return;
 
 	  next = itscm_safe_call_next_x (iter, NULL);
 	}
@@ -470,9 +460,6 @@ cmdscm_completer (struct completer_data *cdata,
       cmdscm_bad_completion_result (_("Bad completer result: "),
 				    completer_result_scm);
     }
-
- done:
-  return result;
 }
 
 /* Helper for gdbscm_make_command which locates the command list to use and
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 23f193f..645d854 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7071,12 +7071,11 @@ Are you sure you want to change it? "),
 
 /* Complete the "handle" command.  */
 
-static VEC (char_ptr) *
+static void
 handle_completer (struct completer_data *cdata,
 		  struct cmd_list_element *ignore,
 		  const char *text, const char *word)
 {
-  VEC (char_ptr) *vec_signals, *vec_keywords, *return_val;
   static const char * const keywords[] =
     {
       "all",
@@ -7091,13 +7090,8 @@ handle_completer (struct completer_data *cdata,
       NULL,
     };
 
-  vec_signals = signal_completer (cdata, ignore, text, word);
-  vec_keywords = complete_on_enum (cdata, keywords, word, word);
-
-  return_val = VEC_merge (char_ptr, vec_signals, vec_keywords);
-  VEC_free (char_ptr, vec_signals);
-  VEC_free (char_ptr, vec_keywords);
-  return return_val;
+  signal_completer (cdata, ignore, text, word);
+  complete_on_enum (cdata, keywords, word, word);
 }
 
 enum gdb_signal
diff --git a/gdb/interps.c b/gdb/interps.c
index 085cac5..87c38d1 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -436,13 +436,12 @@ interpreter_exec_cmd (char *args, int from_tty)
 }
 
 /* List the possible interpreters which could complete the given text.  */
-static VEC (char_ptr) *
+static void
 interpreter_completer (struct completer_data *cdata,
 		       struct cmd_list_element *ignore,
 		       const char *text, const char *word)
 {
   int textlen;
-  VEC (char_ptr) *matches = NULL;
   struct interp *interp;
 
   textlen = strlen (text);
@@ -473,22 +472,18 @@ interpreter_completer (struct completer_data *cdata,
 	  switch (add_status)
 	    {
 	    case MAYBE_ADD_COMPLETION_OK:
-	      VEC_safe_push (char_ptr, matches, match);
 	      break;
 	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	      VEC_safe_push (char_ptr, matches, match);
-	      return matches;
+	      return;
 	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	      xfree (match);
-	      return matches;
+	      return;
 	    case MAYBE_ADD_COMPLETION_DUPLICATE:
 	      xfree (match);
 	      break;
 	    }
 	}
     }
-
-  return matches;
 }
 
 struct interp *
diff --git a/gdb/language.h b/gdb/language.h
index 17b670e..b76f496 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -301,7 +301,7 @@ struct language_defn
        completion is being made.  If CODE is TYPE_CODE_UNDEF, then all
        symbols should be examined; otherwise, only STRUCT_DOMAIN
        symbols whose type has a code of CODE should be matched.  */
-    VEC (char_ptr) *
+    void
       (*la_make_symbol_completion_list) (struct completer_data *,
 					 const char *text,
 					 const char *word,
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 7926157..1880529 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -330,13 +330,12 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 
 /* Called by gdb for command completion.  */
 
-static VEC (char_ptr) *
+static void
 cmdpy_completer (struct completer_data *cdata,
 		 struct cmd_list_element *command,
 		 const char *text, const char *word)
 {
   PyObject *resultobj = NULL;
-  VEC (char_ptr) *result = NULL;
   struct cleanup *cleanup;
 
   cleanup = ensure_python_env (get_current_arch (), current_language);
@@ -351,7 +350,6 @@ cmdpy_completer (struct completer_data *cdata,
   if (resultobj == NULL)
     goto done;
 
-  result = NULL;
   if (PyInt_Check (resultobj))
     {
       /* User code may also return one of the completion constants,
@@ -364,7 +362,7 @@ cmdpy_completer (struct completer_data *cdata,
 	  PyErr_Clear ();
 	}
       else if (value >= 0 && value < (long) N_COMPLETERS)
-	result = completers[value].completer (cdata, command, text, word);
+	completers[value].completer (cdata, command, text, word);
     }
   else
     {
@@ -399,10 +397,8 @@ cmdpy_completer (struct completer_data *cdata,
 	  switch (add_status)
 	    {
 	    case MAYBE_ADD_COMPLETION_OK:
-	      VEC_safe_push (char_ptr, result, item);
 	      break;
 	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	      VEC_safe_push (char_ptr, result, item);
 	      stop = 1;
 	      break;
 	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
@@ -427,8 +423,6 @@ cmdpy_completer (struct completer_data *cdata,
 
   Py_XDECREF (resultobj);
   do_cleanups (cleanup);
-
-  return result;
 }
 
 /* Helper for cmdpy_init which locates the command list to use and
diff --git a/gdb/symtab.c b/gdb/symtab.c
index d997a33..a5a3c2a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4982,31 +4982,8 @@ compare_symbol_name (const char *name, const char *sym_text, int sym_text_len)
   return 1;
 }
 
-/* Free any memory associated with a completion list.  */
-
-static void
-free_completion_list (VEC (char_ptr) **list_ptr)
-{
-  int i;
-  char *p;
-
-  for (i = 0; VEC_iterate (char_ptr, *list_ptr, i, p); ++i)
-    xfree (p);
-  VEC_free (char_ptr, *list_ptr);
-}
-
-/* Callback for make_cleanup.  */
-
-static void
-do_free_completion_list (void *list)
-{
-  free_completion_list (list);
-}
-
 /* Helper routine for make_symbol_completion_list.  */
 
-static VEC (char_ptr) *return_val;
-
 #define COMPLETION_LIST_ADD_SYMBOL(cdata, symbol, sym_text, len,	\
 				   text, word)				\
   completion_list_add_name						\
@@ -5063,10 +5040,8 @@ completion_list_add_name (struct completer_data *cdata,
     switch (add_status)
       {
       case MAYBE_ADD_COMPLETION_OK:
-	VEC_safe_push (char_ptr, return_val, newobj);
 	break;
       case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	VEC_safe_push (char_ptr, return_val, newobj);
 	throw_max_completions_reached_error ();
       case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	xfree (newobj);
@@ -5509,18 +5484,13 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata,
     }
 }
 
-VEC (char_ptr) *
+void
 default_make_symbol_completion_list_break_on (struct completer_data *cdata,
 					      const char *text,
 					      const char *word,
 					      const char *break_on,
 					      enum type_code code)
 {
-  struct cleanup *back_to;
-
-  return_val = NULL;
-  back_to = make_cleanup (do_free_completion_list, &return_val);
-
   TRY
     {
       default_make_symbol_completion_list_break_on_1 (cdata, text, word,
@@ -5532,61 +5502,56 @@ default_make_symbol_completion_list_break_on (struct completer_data *cdata,
 	throw_exception (except);
     }
   END_CATCH
-
-  discard_cleanups (back_to);
-  return return_val;
 }
 
-VEC (char_ptr) *
+void
 default_make_symbol_completion_list (struct completer_data *cdata,
 				     const char *text, const char *word,
 				     enum type_code code)
 {
-  return default_make_symbol_completion_list_break_on (cdata, text, word, "",
-						       code);
+  default_make_symbol_completion_list_break_on (cdata, text, word, "", code);
 }
 
 /* Return a vector of all symbols (regardless of class) which begin by
    matching TEXT.  If the answer is no symbols, then the return value
    is NULL.  */
 
-VEC (char_ptr) *
+void
 make_symbol_completion_list (struct completer_data *cdata,
 			     const char *text, const char *word)
 {
-  return current_language->la_make_symbol_completion_list (cdata, text, word,
-							   TYPE_CODE_UNDEF);
+  current_language->la_make_symbol_completion_list (cdata, text, word,
+						    TYPE_CODE_UNDEF);
 }
 
 /* Like make_symbol_completion_list, but only return STRUCT_DOMAIN
    symbols whose type code is CODE.  */
 
-VEC (char_ptr) *
+void
 make_symbol_completion_type (struct completer_data *cdata, const char *text,
 			     const char *word, enum type_code code)
 {
   gdb_assert (code == TYPE_CODE_UNION
 	      || code == TYPE_CODE_STRUCT
 	      || code == TYPE_CODE_ENUM);
-  return current_language->la_make_symbol_completion_list (cdata, text, word,
-							   code);
+  current_language->la_make_symbol_completion_list (cdata, text, word, code);
 }
 
 /* Like make_symbol_completion_list, but suitable for use as a
    completion function.  */
 
-VEC (char_ptr) *
+void
 make_symbol_completion_list_fn (struct completer_data *cdata,
 				struct cmd_list_element *ignore,
 				const char *text, const char *word)
 {
-  return make_symbol_completion_list (cdata, text, word);
+  make_symbol_completion_list (cdata, text, word);
 }
 
 /* Like make_symbol_completion_list, but returns a list of symbols
    defined in a source file FILE.  */
 
-VEC (char_ptr) *
+void
 make_file_symbol_completion_list (struct completer_data *cdata,
 				  const char *text, const char *word,
 				  const char *srcfile)
@@ -5634,7 +5599,7 @@ make_file_symbol_completion_list (struct completer_data *cdata,
       /* A double-quoted string is never a symbol, nor does it make sense
          to complete it any other way.  */
       {
-	return NULL;
+	return;
       }
     else
       {
@@ -5645,8 +5610,6 @@ make_file_symbol_completion_list (struct completer_data *cdata,
 
   sym_text_len = strlen (sym_text);
 
-  return_val = NULL;
-
   /* Find the symtab for SRCFILE (this loads it if it was not yet read
      in).  */
   s = lookup_symtab (srcfile);
@@ -5662,7 +5625,7 @@ make_file_symbol_completion_list (struct completer_data *cdata,
 
   /* If we have no symtab for that file, return an empty list.  */
   if (s == NULL)
-    return (return_val);
+    return;
 
   /* Go through this symtab and check the externs and statics for
      symbols which match.  */
@@ -5680,8 +5643,6 @@ make_file_symbol_completion_list (struct completer_data *cdata,
       COMPLETION_LIST_ADD_SYMBOL (cdata, sym, sym_text, sym_text_len,
 				  text, word);
     }
-
-  return (return_val);
 }
 
 /* A helper function for make_source_files_completion_list.  It adds
@@ -5690,8 +5651,7 @@ make_file_symbol_completion_list (struct completer_data *cdata,
 
 static void
 add_filename_to_list (struct completer_data *cdata, const char *fname,
-		      const char *text, const char *word,
-		      VEC (char_ptr) **list)
+		      const char *text, const char *word)
 {
   char *newobj;
   size_t fnlen = strlen (fname);
@@ -5724,16 +5684,10 @@ add_filename_to_list (struct completer_data *cdata, const char *fname,
   switch (add_status)
     {
     case MAYBE_ADD_COMPLETION_OK:
-      VEC_safe_push (char_ptr, *list, newobj);
-      discard_cleanups (cleanup);
-      break;
     case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-      VEC_safe_push (char_ptr, *list, newobj);
       discard_cleanups (cleanup);
       break;
     case MAYBE_ADD_COMPLETION_MAX_REACHED:
-      do_cleanups (cleanup);
-      break;
     case MAYBE_ADD_COMPLETION_DUPLICATE:
       do_cleanups (cleanup);
       break;
@@ -5765,7 +5719,6 @@ struct add_partial_filename_data
   const char *text;
   const char *word;
   int text_len;
-  VEC (char_ptr) **list;
 
   /* Completion data used by the completer function.  */
   struct completer_data *completer_data;
@@ -5787,7 +5740,7 @@ maybe_add_partial_symtab_filename (const char *filename, const char *fullname,
       /* This file matches for a completion; add it to the
 	 current list of matches.  */
       add_filename_to_list (data->completer_data, filename, data->text,
-			    data->word, data->list);
+			    data->word);
     }
   else
     {
@@ -5798,17 +5751,16 @@ maybe_add_partial_symtab_filename (const char *filename, const char *fullname,
 	  && filename_ncmp (base_name, data->text, data->text_len) == 0)
 	{
 	  add_filename_to_list (data->completer_data, base_name, data->text,
-				data->word, data->list);
+				data->word);
 	}
     }
 }
 
-/* Return a vector of all source files whose names begin with matching
-   TEXT.  The file names are looked up in the symbol tables of this
-   program.  If the answer is no matchess, then the return value is
-   NULL.  */
+/* Add all source files whose names begin with matching TEXT to CDATA.
+   The file names are looked up in the symbol tables of this program.
+   If the answer is no matchess, then the return value is NULL.  */
 
-VEC (char_ptr) *
+void
 make_source_files_completion_list (struct completer_data *cdata,
 				   const char *text, const char *word)
 {
@@ -5816,16 +5768,13 @@ make_source_files_completion_list (struct completer_data *cdata,
   struct symtab *s;
   struct objfile *objfile;
   size_t text_len = strlen (text);
-  VEC (char_ptr) *list = NULL;
   const char *base_name;
   struct add_partial_filename_data datum;
   struct filename_seen_cache *filename_seen_cache;
-  struct cleanup *back_to, *cache_cleanup;
+  struct cleanup *cache_cleanup;
 
   if (!have_full_symbols () && !have_partial_symbols ())
-    return list;
-
-  back_to = make_cleanup (do_free_completion_list, &list);
+    return;
 
   filename_seen_cache = create_filename_seen_cache ();
   cache_cleanup = make_cleanup (delete_filename_seen_cache,
@@ -5840,7 +5789,7 @@ make_source_files_completion_list (struct completer_data *cdata,
 	{
 	  /* This file matches for a completion; add it to the current
 	     list of matches.  */
-	  add_filename_to_list (cdata, s->filename, text, word, &list);
+	  add_filename_to_list (cdata, s->filename, text, word);
 	}
       else
 	{
@@ -5852,7 +5801,7 @@ make_source_files_completion_list (struct completer_data *cdata,
 	  if (base_name != s->filename
 	      && !filename_seen (filename_seen_cache, base_name, 1)
 	      && filename_ncmp (base_name, text, text_len) == 0)
-	    add_filename_to_list (cdata, base_name, text, word, &list);
+	    add_filename_to_list (cdata, base_name, text, word);
 	}
     }
 
@@ -5860,15 +5809,11 @@ make_source_files_completion_list (struct completer_data *cdata,
   datum.text = text;
   datum.word = word;
   datum.text_len = text_len;
-  datum.list = &list;
   datum.completer_data = cdata;
   map_symbol_filenames (maybe_add_partial_symtab_filename, &datum,
 			0 /*need_fullname*/);
 
   do_cleanups (cache_cleanup);
-  discard_cleanups (back_to);
-
-  return list;
 }
 \f
 /* Track MAIN */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 508a7e5..eb6e8f2 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1445,30 +1445,31 @@ extern void forget_cached_source_info (void);
 
 extern void select_source_symtab (struct symtab *);
 
-extern VEC (char_ptr) *default_make_symbol_completion_list_break_on
+extern void default_make_symbol_completion_list_break_on
  (struct completer_data *cdata, const char *text, const char *word,
   const char *break_on, enum type_code code);
-extern VEC (char_ptr) *
+
+extern void
  default_make_symbol_completion_list (struct completer_data *,
 				      const char *, const char *,
 				      enum type_code);
-extern VEC (char_ptr) *
+extern void
   make_symbol_completion_list (struct completer_data *, const char *,
 			       const char *);
-extern VEC (char_ptr) *
+extern void
   make_symbol_completion_type (struct completer_data *,
 			       const char *, const char *,
 			       enum type_code);
-extern VEC (char_ptr) *
+extern void
   make_symbol_completion_list_fn (struct completer_data *,
 				  struct cmd_list_element *,
 				  const char *, const char *);
 
-extern VEC (char_ptr) *
+extern void
   make_file_symbol_completion_list (struct completer_data *,
 				    const char *, const char *, const char *);
 
-extern VEC (char_ptr) *
+extern void
  make_source_files_completion_list (struct completer_data *, const char *,
 				    const char *);
 
diff --git a/gdb/value.c b/gdb/value.c
index 8519b91..b5b82c5 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2029,10 +2029,9 @@ lookup_only_internalvar (const char *name)
    Returns a vector of newly allocated strings, or NULL if no matches
    were found.  */
 
-VEC (char_ptr) *
+void
 complete_internalvar (struct completer_data *cdata, const char *name)
 {
-  VEC (char_ptr) *result = NULL;
   struct internalvar *var;
   int len;
 
@@ -2048,21 +2047,17 @@ complete_internalvar (struct completer_data *cdata, const char *name)
 	switch (add_status)
 	  {
 	  case MAYBE_ADD_COMPLETION_OK:
-	    VEC_safe_push (char_ptr, result, r);
 	    break;
 	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	    VEC_safe_push (char_ptr, result, r);
-	    return result;
+	    return;
 	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
 	    xfree (r);
-	    return result;
+	    return;
 	  case MAYBE_ADD_COMPLETION_DUPLICATE:
 	    xfree (r);
 	    break;
 	  }
       }
-
-  return result;
 }
 
 /* Create an internal variable with name NAME and with a void value.
diff --git a/gdb/value.h b/gdb/value.h
index 41aec1b..f44e5a1 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -872,8 +872,8 @@ extern struct internalvar *lookup_only_internalvar (const char *name);
 
 extern struct internalvar *create_internalvar (const char *name);
 
-extern VEC (char_ptr) *complete_internalvar (struct completer_data *cdata,
-					     const char *name);
+extern void complete_internalvar (struct completer_data *cdata,
+				  const char *name);
 
 /* An internalvar can be dynamically computed by supplying a vector of
    function pointers to perform various operations.  */

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

* [PATCH 04/18] Implement completion limiting for add_filename_to_list.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (12 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 18/18] Remove the vector return result from the completion API Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 02/18] Remove completion_tracker_t from the public completion API Keith Seitz
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts add_filename_to_list to implement completion limiting.
This function is used by both make_source_files_completion_list
and maybe_add_partial_symbol.  Tests have been added for both code
pathways.

gdb/ChangeLog

	* symtab.c (add_filename_to_list): Add completer_data argument.
	Use maybe_add_completion.
	Update all callers.

gdb/testsuite/ChangeLog

	* gdb.base/filesym.c (filesym2): Add external declaration.
	* gdb.base/filesym.exp: Compile newly added files.
	Add tests for completion limiting.
	* gdb.base/filesym2.c, gdb.base/filesym3.c, gdb.base/filesym4.c,
	gdb.base/filesym5.c: New files.
---
 gdb/symtab.c                       |   38 ++++++++++++++--
 gdb/testsuite/gdb.base/filesym.c   |    4 +-
 gdb/testsuite/gdb.base/filesym.exp |   84 +++++++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.base/filesym2.c  |   24 ++++++++++
 gdb/testsuite/gdb.base/filesym3.c  |   24 ++++++++++
 gdb/testsuite/gdb.base/filesym4.c  |   24 ++++++++++
 gdb/testsuite/gdb.base/filesym5.c  |   22 +++++++++
 7 files changed, 210 insertions(+), 10 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/filesym2.c
 create mode 100644 gdb/testsuite/gdb.base/filesym3.c
 create mode 100644 gdb/testsuite/gdb.base/filesym4.c
 create mode 100644 gdb/testsuite/gdb.base/filesym5.c

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0736582..d997a33 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5689,12 +5689,16 @@ make_file_symbol_completion_list (struct completer_data *cdata,
    list as necessary.  */
 
 static void
-add_filename_to_list (const char *fname, const char *text, const char *word,
+add_filename_to_list (struct completer_data *cdata, const char *fname,
+		      const char *text, const char *word,
 		      VEC (char_ptr) **list)
 {
   char *newobj;
   size_t fnlen = strlen (fname);
+  enum maybe_add_completion_enum add_status;
+  struct cleanup *cleanup;
 
+  cleanup = make_cleanup (free_current_contents, &newobj);
   if (word == text)
     {
       /* Return exactly fname.  */
@@ -5715,7 +5719,25 @@ add_filename_to_list (const char *fname, const char *text, const char *word,
       newobj[text - word] = '\0';
       strcat (newobj, fname);
     }
-  VEC_safe_push (char_ptr, *list, newobj);
+
+  add_status = maybe_add_completion (cdata, newobj);
+  switch (add_status)
+    {
+    case MAYBE_ADD_COMPLETION_OK:
+      VEC_safe_push (char_ptr, *list, newobj);
+      discard_cleanups (cleanup);
+      break;
+    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+      VEC_safe_push (char_ptr, *list, newobj);
+      discard_cleanups (cleanup);
+      break;
+    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+      do_cleanups (cleanup);
+      break;
+    case MAYBE_ADD_COMPLETION_DUPLICATE:
+      do_cleanups (cleanup);
+      break;
+    }
 }
 
 static int
@@ -5764,7 +5786,8 @@ maybe_add_partial_symtab_filename (const char *filename, const char *fullname,
     {
       /* This file matches for a completion; add it to the
 	 current list of matches.  */
-      add_filename_to_list (filename, data->text, data->word, data->list);
+      add_filename_to_list (data->completer_data, filename, data->text,
+			    data->word, data->list);
     }
   else
     {
@@ -5773,7 +5796,10 @@ maybe_add_partial_symtab_filename (const char *filename, const char *fullname,
       if (base_name != filename
 	  && !filename_seen (data->filename_seen_cache, base_name, 1)
 	  && filename_ncmp (base_name, data->text, data->text_len) == 0)
-	add_filename_to_list (base_name, data->text, data->word, data->list);
+	{
+	  add_filename_to_list (data->completer_data, base_name, data->text,
+				data->word, data->list);
+	}
     }
 }
 
@@ -5814,7 +5840,7 @@ make_source_files_completion_list (struct completer_data *cdata,
 	{
 	  /* This file matches for a completion; add it to the current
 	     list of matches.  */
-	  add_filename_to_list (s->filename, text, word, &list);
+	  add_filename_to_list (cdata, s->filename, text, word, &list);
 	}
       else
 	{
@@ -5826,7 +5852,7 @@ make_source_files_completion_list (struct completer_data *cdata,
 	  if (base_name != s->filename
 	      && !filename_seen (filename_seen_cache, base_name, 1)
 	      && filename_ncmp (base_name, text, text_len) == 0)
-	    add_filename_to_list (base_name, text, word, &list);
+	    add_filename_to_list (cdata, base_name, text, word, &list);
 	}
     }
 
diff --git a/gdb/testsuite/gdb.base/filesym.c b/gdb/testsuite/gdb.base/filesym.c
index 59093a4..39bf9af 100644
--- a/gdb/testsuite/gdb.base/filesym.c
+++ b/gdb/testsuite/gdb.base/filesym.c
@@ -15,10 +15,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+extern int filesym2 (int arg);
+
 int
 filesym (int arg)
 {
-  return arg;
+  return filesym2 (arg);
 }
 
 int
diff --git a/gdb/testsuite/gdb.base/filesym.exp b/gdb/testsuite/gdb.base/filesym.exp
index 8084710..3b0ede4 100644
--- a/gdb/testsuite/gdb.base/filesym.exp
+++ b/gdb/testsuite/gdb.base/filesym.exp
@@ -16,9 +16,10 @@
 # This series of completion tests checks the completion output
 # on a name which is potentially both a symbol name and a file name.
 
-standard_testfile
+standard_testfile filesym.c filesym2.c filesym3.c filesym4.c filesym5.c
 
-if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+if {[prepare_for_testing $testfile.exp $testfile \
+	 [list $srcfile $srcfile2 $srcfile3 $srcfile4 $srcfile5] debug]} {
     return -1
 }
 
@@ -40,9 +41,20 @@ gdb_test_multiple "" $tst {
 
 	# Now ask for the completion list
 	set tst "completion list for \"filesym\""
+
+	set expected {\x07\r\n}
+	for {set i 1} {$i <= 5} {incr i} {
+	    if {$i == 1} {
+		append expected "filesym\[ \t\]+"
+		append expected "filesym\\\.c\[ \t\]+"
+	    } else {
+		append expected "filesym$i\[ \t\]+"
+		append expected "filesym${i}\\\.c\[ \t\]+"
+	    }
+	}
 	send_gdb "\t\t"
 	gdb_test_multiple "" $tst {
-	    -re "\\\x07\r\nfilesym\[ \t\]+filesym.c\[ \t\]+\r\n$gdb_prompt " {
+	    -re "$expected\r\n$gdb_prompt " {
 		pass $tst
 
 		# Flush the rest of the output by creating the breakpoint.
@@ -54,4 +66,70 @@ gdb_test_multiple "" $tst {
     }
 }
 
+# Test completion limiting.
+#
+# We cannot assume any order when completion
+# limiting is in effect.  All we know is symbol names are completed
+# first and file names next.  If max-completions is set greater than
+# the number of symbols, we will have all of the symbols and
+# (max-completions - 5) file names in the completion list.
+# We also cannot know which file names will be mentioned -- only the number
+# of source files that should be mentioned.
+
+# The number of symbols/files defined in this test.
+set number_of_symbols 5
+
+# The number of completions to attempt
+set max_completions 7;  # This will give all (5) files and 2 file names
+
+# List of seen files
+set files_seen {}
+
+# List of seen symbols
+set syms_seen {}
+
+# Set the maximum completions.
+gdb_test_no_output "set max-completions $max_completions"
+
+# The terminal at the end of the complete command
+set end "\\\*\\\*\\\* List may be truncated, "
+append end "max-completions reached\\\. \\\*\\\*\\\*"
+
+with_test_prefix "completion limit on files" {
+    gdb_test_multiple "complete break filesy" "" {
+	"complete break filesy" { exp_continue }
+
+	-re "break filesym(\[2-$number_of_symbols\])?(\\\.c)?\r\n" {
+	    set name [string trim $expect_out(0,string) \r\n]
+	    #send_log "seen: $name -- "
+	    if {[string match "*.c" $name]} {
+		#send_log "is file\n"
+		lappend files_seen $name
+	    } else {
+		#send_log "is symbol\n"
+		lappend syms_seen $name
+	    }
+	    exp_continue
+	}
+
+	-re "break filesy $end\r\n$gdb_prompt $" {
+	    set n [llength $syms_seen]
+	    if {$n == $number_of_symbols} {
+		pass "symbols seen"
+	    } else {
+		fail "symbols seen ($n/$number_of_symbols)"
+	    }
+
+	    if {[llength $files_seen] \
+		    == [expr {$max_completions - $number_of_symbols}]} {
+		pass "files seen"
+	    } else {
+		set n [llength $files_seen]
+		set m [expr {$max_completions - $number_of_symbols}]
+		fail "files seen ($n/$m)"
+	    }
+	}
+    }
+}
+
 unset -nocomplain tst
diff --git a/gdb/testsuite/gdb.base/filesym2.c b/gdb/testsuite/gdb.base/filesym2.c
new file mode 100644
index 0000000..e16bb53
--- /dev/null
+++ b/gdb/testsuite/gdb.base/filesym2.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int filesym3 (int arg);
+
+int
+filesym2 (int arg)
+{
+  return filesym3 (arg);
+}
diff --git a/gdb/testsuite/gdb.base/filesym3.c b/gdb/testsuite/gdb.base/filesym3.c
new file mode 100644
index 0000000..846a488
--- /dev/null
+++ b/gdb/testsuite/gdb.base/filesym3.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int filesym4 (int arg);
+
+int
+filesym3 (int arg)
+{
+  return filesym4 (arg);
+}
diff --git a/gdb/testsuite/gdb.base/filesym4.c b/gdb/testsuite/gdb.base/filesym4.c
new file mode 100644
index 0000000..2d6e506
--- /dev/null
+++ b/gdb/testsuite/gdb.base/filesym4.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int filesym5 (int arg);
+
+int
+filesym4 (int arg)
+{
+  return filesym5 (arg);
+}
diff --git a/gdb/testsuite/gdb.base/filesym5.c b/gdb/testsuite/gdb.base/filesym5.c
new file mode 100644
index 0000000..e192da8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/filesym5.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+filesym5 (int arg)
+{
+  return arg;
+}

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

* [PATCH 06/18] Implement completion limiting for condition_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
  2015-04-13 19:23 ` [PATCH 14/18] Implement completion limiting in add_struct_fields Keith Seitz
  2015-04-13 19:23 ` [PATCH 09/18] Implement completion limiting for interpreter_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 10/18] Implement completion limiting for cmdpy_completer Keith Seitz
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts the condition completer to use maybe_add_completion.
A side-effect of this is the similar conversion of complete_internalvar.

Tests have been added to exercise this new behavior.

gdb/ChangeLog

	* breakpoint.c (condition_completer): Pass completer_data to
	complete_internalvar.
	Use maybe_add_completion.
	* value.c: Include completer.h.
	(complete_internalvar): Add completer_data argument.
	Use maybe_add_completion.
	* value.h (complete_internalvar): Add completer_data argument.

gdb/testsuite/ChangeLog

	* gdb.base/condbreak.exp (test_completion): New procedure.
	Add more completion tests, with and without limiting.
---
 gdb/breakpoint.c                     |   24 +++++++++++-
 gdb/testsuite/gdb.base/condbreak.exp |   70 ++++++++++++++++++++++++++++++++++
 gdb/value.c                          |   21 +++++++++-
 gdb/value.h                          |    3 +
 4 files changed, 113 insertions(+), 5 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 095fdf2..04dc734 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1052,7 +1052,7 @@ condition_completer (struct completer_data *cdata,
 	  /* We don't support completion of history indices.  */
 	  if (isdigit (text[1]))
 	    return NULL;
-	  return complete_internalvar (&text[1]);
+	  return complete_internalvar (cdata, &text[1]);
 	}
 
       /* We're completing the breakpoint number.  */
@@ -1065,7 +1065,27 @@ condition_completer (struct completer_data *cdata,
 	  xsnprintf (number, sizeof (number), "%d", b->number);
 
 	  if (strncmp (number, text, len) == 0)
-	    VEC_safe_push (char_ptr, result, xstrdup (number));
+	    {
+	      enum maybe_add_completion_enum add_status;
+	      char *completion = xstrdup (number);
+
+	      add_status = maybe_add_completion (cdata, completion);
+	      switch (add_status)
+		{
+		case MAYBE_ADD_COMPLETION_OK:
+		  VEC_safe_push (char_ptr, result, completion);
+		  break;
+		case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+		  VEC_safe_push (char_ptr, result, completion);
+		  return result;
+		case MAYBE_ADD_COMPLETION_MAX_REACHED:
+		  xfree (completion);
+		  return result;
+		case MAYBE_ADD_COMPLETION_DUPLICATE:
+		  xfree (completion);
+		  break;
+		}
+	    }
 	}
 
       return result;
diff --git a/gdb/testsuite/gdb.base/condbreak.exp b/gdb/testsuite/gdb.base/condbreak.exp
index fa40a5f..5630ede 100644
--- a/gdb/testsuite/gdb.base/condbreak.exp
+++ b/gdb/testsuite/gdb.base/condbreak.exp
@@ -246,3 +246,73 @@ gdb_test "complete cond 1" "cond 1"
 gdb_test "set variable \$var = 1"
 gdb_test "complete cond \$v" "cond \\\$var"
 gdb_test "complete cond 1 values\[0\].a" "cond 1 values.0..a_field"
+
+# Test non-trivial completion and completion-limiting
+
+# Delete all breakpoints and create a bunch of new ones.
+delete_breakpoints
+for {set i 0} {$i < 20} {incr i} {
+    with_test_prefix "set breakpoint $i" {
+	gdb_breakpoint "factorial"
+    }
+}
+
+# While the completer function does traverse breakpoints in the order
+# they were created, don't assume that is required for the test.
+# We only count the number of completions found.  In this case,
+# this test will create breakpoints 9-19, giving "complete cond 1"
+# ten total completion possibilities.
+
+# A convenience procedure to automate test completion lists.
+proc test_completion {cmd exp total {limit 0}} {
+    global gdb_prompt
+
+    if {$limit} {
+	set end "\\\*\\\*\\\* List may be truncated, "
+	append end "max-completions reached\\\. \\\*\\\*\\\*\r\n"
+	set testname "limit '$cmd'"
+    } else {
+	set end ""
+	set testname $cmd
+    }
+
+    set seen 0
+    gdb_test_multiple $cmd $testname {
+	"$cmd\r\n" { exp_continue }
+
+	-re "cond $exp\[0-9\]+\r\n" {
+	    incr seen
+	    exp_continue
+	}
+
+	-re ".*$end$gdb_prompt $" {
+	    if {$seen == $total} {
+		pass $testname
+	    } else {
+		fail "$testname ($seen/$total)"
+	    }
+	}
+    }
+}
+
+# Test completion of breakpoint number.
+with_test_prefix "completion test:" {
+    test_completion "complete cond 1" "1" 10
+}
+
+# Test completion of breakpoint number using internal variable.
+for {set i 0} {$i < 10} {incr i} {
+    gdb_test_no_output "set variable \$var_bp_$i = $i"
+}
+
+test_completion "complete cond \$var_bp" "\\\$var_bp_" 10
+
+# Run the above tests with completion limiting.
+set max_completions 4
+gdb_test_no_output "set max-completions $max_completions"
+
+with_test_prefix "completion test:" {
+    test_completion "complete cond 1" "1" $max_completions 1
+}
+
+test_completion "complete cond \$var_bp" "\\\$var_bp_" $max_completions 1
diff --git a/gdb/value.c b/gdb/value.c
index cb56849..8519b91 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -40,6 +40,7 @@
 #include "tracepoint.h"
 #include "cp-abi.h"
 #include "user-regs.h"
+#include "completer.h"
 
 /* Prototypes for exported functions.  */
 
@@ -2029,7 +2030,7 @@ lookup_only_internalvar (const char *name)
    were found.  */
 
 VEC (char_ptr) *
-complete_internalvar (const char *name)
+complete_internalvar (struct completer_data *cdata, const char *name)
 {
   VEC (char_ptr) *result = NULL;
   struct internalvar *var;
@@ -2041,8 +2042,24 @@ complete_internalvar (const char *name)
     if (strncmp (var->name, name, len) == 0)
       {
 	char *r = xstrdup (var->name);
+	enum maybe_add_completion_enum add_status;
 
-	VEC_safe_push (char_ptr, result, r);
+	add_status = maybe_add_completion (cdata, r);
+	switch (add_status)
+	  {
+	  case MAYBE_ADD_COMPLETION_OK:
+	    VEC_safe_push (char_ptr, result, r);
+	    break;
+	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	    VEC_safe_push (char_ptr, result, r);
+	    return result;
+	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	    xfree (r);
+	    return result;
+	  case MAYBE_ADD_COMPLETION_DUPLICATE:
+	    xfree (r);
+	    break;
+	  }
       }
 
   return result;
diff --git a/gdb/value.h b/gdb/value.h
index 21baa32..41aec1b 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -872,7 +872,8 @@ extern struct internalvar *lookup_only_internalvar (const char *name);
 
 extern struct internalvar *create_internalvar (const char *name);
 
-extern VEC (char_ptr) *complete_internalvar (const char *name);
+extern VEC (char_ptr) *complete_internalvar (struct completer_data *cdata,
+					     const char *name);
 
 /* An internalvar can be dynamically computed by supplying a vector of
    function pointers to perform various operations.  */

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

* [PATCH 01/18] Add struct completer_data to the completion API.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (5 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 16/18] Make the completion API completely opaque Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 03/18] Implement completion limiting for complete_on_cmdlist Keith Seitz
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch is largely mechanical.  It modifies the completion API so
that all completion functions take a new (pointer to a) structure,
which will (eventually) be used internally by the completer to perform
completion limiting.

gdb/ChangeLog

	* completer.c (struct completer_data): Define.
	* language.h (struct language_defn)
	<la_make_symbol_completion_list>: Add struct completer_data
	to argument list.
	All users updated.
	* symtab.c (COMPLETION_LIST_ADD_SYMBOL): Add CDATA argument.
	Update all callers.
	(MCOMPLETION_LIST_ADD_SYMBOL): Likewise.
	(struct add_name_data) <completer_data>: New field.
	(default_make_symbol_completion_list_break_on_1): Initialize
	the above new field.
	(struct add_partial_filename_data) <completer_data>: New field.
	(make_source_files_completion_list): Initialize the above new
	field.
---
 gdb/ada-lang.c            |    3 +
 gdb/break-catch-syscall.c |    5 +-
 gdb/breakpoint.c          |    5 +-
 gdb/cli/cli-decode.c      |   11 ++--
 gdb/command.h             |   11 +++-
 gdb/completer.c           |   85 ++++++++++++++++++-----------
 gdb/completer.h           |   22 +++++---
 gdb/corefile.c            |    5 +-
 gdb/cp-abi.c              |    5 +-
 gdb/f-lang.c              |    6 +-
 gdb/guile/scm-cmd.c       |    3 +
 gdb/infrun.c              |    7 +-
 gdb/interps.c             |    3 +
 gdb/language.h            |    8 ++-
 gdb/python/py-cmd.c       |   12 ++--
 gdb/remote-sim.c          |    3 +
 gdb/symtab.c              |  130 ++++++++++++++++++++++++++++-----------------
 gdb/symtab.h              |   44 +++++++++------
 18 files changed, 229 insertions(+), 139 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 124e370..5ca4b5b 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -6190,7 +6190,8 @@ ada_complete_symbol_matcher (const char *name, void *user_data)
    the entire command on which completion is made.  */
 
 static VEC (char_ptr) *
-ada_make_symbol_completion_list (const char *text0, const char *word,
+ada_make_symbol_completion_list (struct completer_data *cdata,
+				 const char *text0, const char *word,
 				 enum type_code code)
 {
   char *text;
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 1718f49..4677132 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -592,12 +592,13 @@ catching_syscall_number (int syscall_number)
 
 /* Complete syscall names.  Used by "catch syscall".  */
 static VEC (char_ptr) *
-catch_syscall_completer (struct cmd_list_element *cmd,
+catch_syscall_completer (struct completer_data *cdata,
+			 struct cmd_list_element *cmd,
                          const char *text, const char *word)
 {
   const char **list = get_syscall_names (get_current_arch ());
   VEC (char_ptr) *retlist
-    = (list == NULL) ? NULL : complete_on_enum (list, word, word);
+    = (list == NULL) ? NULL : complete_on_enum (cdata, list, word, word);
 
   xfree (list);
   return retlist;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 31869f1..095fdf2 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1033,7 +1033,8 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
 /* Completion for the "condition" command.  */
 
 static VEC (char_ptr) *
-condition_completer (struct cmd_list_element *cmd,
+condition_completer (struct completer_data *cdata,
+		     struct cmd_list_element *cmd,
 		     const char *text, const char *word)
 {
   const char *space;
@@ -1072,7 +1073,7 @@ condition_completer (struct cmd_list_element *cmd,
 
   /* We're completing the expression part.  */
   text = skip_spaces_const (space);
-  return expression_completer (cmd, text, word);
+  return expression_completer (cdata, cmd, text, word);
 }
 
 /* condition N EXP -- set break condition of breakpoint N to EXP.  */
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 4d0d5a9..286fc61 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -648,7 +648,8 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass
    support a special "unlimited" value.  */
 
 static VEC (char_ptr) *
-integer_unlimited_completer (struct cmd_list_element *ignore,
+integer_unlimited_completer (struct completer_data *cdata,
+			     struct cmd_list_element *ignore,
 			     const char *text, const char *word)
 {
   static const char * const keywords[] =
@@ -657,7 +658,7 @@ integer_unlimited_completer (struct cmd_list_element *ignore,
       NULL,
     };
 
-  return complete_on_enum (keywords, text, word);
+  return complete_on_enum (cdata, keywords, text, word);
 }
 
 /* Add element named NAME to both the set and show command LISTs (the
@@ -1765,7 +1766,8 @@ lookup_cmd_composition (const char *text,
    "oobar"; if WORD is "baz/foo", return "baz/foobar".  */
 
 VEC (char_ptr) *
-complete_on_cmdlist (struct cmd_list_element *list,
+complete_on_cmdlist (struct completer_data *cdata,
+		     struct cmd_list_element *list,
 		     const char *text, const char *word,
 		     int ignore_help_classes)
 {
@@ -1835,7 +1837,8 @@ complete_on_cmdlist (struct cmd_list_element *list,
    "oobar"; if WORD is "baz/foo", return "baz/foobar".  */
 
 VEC (char_ptr) *
-complete_on_enum (const char *const *enumlist,
+complete_on_enum (struct completer_data *cdata,
+		  const char *const *enumlist,
 		  const char *text, const char *word)
 {
   VEC (char_ptr) *matchlist = NULL;
diff --git a/gdb/command.h b/gdb/command.h
index bdf625b..b2aeb30 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -23,6 +23,8 @@
 /* This file defines the public interface for any code wanting to
    create commands.  */
 
+struct completer_data;
+
 /* Command classes are top-level categories into which commands are
    broken down for "help" purposes.
 
@@ -156,7 +158,8 @@ typedef void cmd_sfunc_ftype (char *args, int from_tty,
 extern void set_cmd_sfunc (struct cmd_list_element *cmd,
 			   cmd_sfunc_ftype *sfunc);
 
-typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *,
+typedef VEC (char_ptr) *completer_ftype (struct completer_data *,
+					 struct cmd_list_element *,
 					 const char *, const char *);
 
 typedef void completer_ftype_void (struct cmd_list_element *,
@@ -225,10 +228,12 @@ extern struct cmd_list_element *add_info (const char *,
 extern struct cmd_list_element *add_info_alias (const char *, const char *,
 						int);
 
-extern VEC (char_ptr) *complete_on_cmdlist (struct cmd_list_element *,
+extern VEC (char_ptr) *complete_on_cmdlist (struct completer_data *,
+					    struct cmd_list_element *,
 					    const char *, const char *, int);
 
-extern VEC (char_ptr) *complete_on_enum (const char *const *enumlist,
+extern VEC (char_ptr) *complete_on_enum (struct completer_data *,
+					 const char *const *enumlist,
 					 const char *, const char *);
 
 /* Functions that implement commands about CLI commands.  */
diff --git a/gdb/completer.c b/gdb/completer.c
index c8c0e4c..f2b31e9 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -86,6 +86,15 @@ static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
    we can't include '"' because the gdb C parser treats such quoted
    sequences as strings.  */
 static char *gdb_completer_quote_characters = "'";
+
+/* A structure holding completion-specific calldata.  */
+
+struct completer_data
+{
+  /* The completion tracker being used by the completer.  */
+  completion_tracker_t tracker;
+};
+
 \f
 /* Accessor for some completer data that may interest other files.  */
 
@@ -107,7 +116,8 @@ readline_line_completion_function (const char *text, int matches)
 /* This can be used for functions which don't want to complete on
    symbols but don't want to complete on anything else either.  */
 VEC (char_ptr) *
-noop_completer (struct cmd_list_element *ignore, 
+noop_completer (struct completer_data *cdata,
+		struct cmd_list_element *ignore,
 		const char *text, const char *prefix)
 {
   return NULL;
@@ -115,7 +125,8 @@ noop_completer (struct cmd_list_element *ignore,
 
 /* Complete on filenames.  */
 VEC (char_ptr) *
-filename_completer (struct cmd_list_element *ignore, 
+filename_completer (struct completer_data *cdata,
+		    struct cmd_list_element *ignore,
 		    const char *text, const char *word)
 {
   int subsequent_name;
@@ -184,7 +195,8 @@ filename_completer (struct cmd_list_element *ignore,
    etc.  */
 
 VEC (char_ptr) *
-location_completer (struct cmd_list_element *ignore, 
+location_completer (struct completer_data *cdata,
+		    struct cmd_list_element *ignore,
 		    const char *text, const char *word)
 {
   int n_syms, n_files, ix;
@@ -260,18 +272,18 @@ location_completer (struct cmd_list_element *ignore,
      symbols as well as on files.  */
   if (colon)
     {
-      list = make_file_symbol_completion_list (symbol_start, word,
+      list = make_file_symbol_completion_list (cdata, symbol_start, word,
 					       file_to_match);
       xfree (file_to_match);
     }
   else
     {
-      list = make_symbol_completion_list (symbol_start, word);
+      list = make_symbol_completion_list (cdata, symbol_start, word);
       /* If text includes characters which cannot appear in a file
 	 name, they cannot be asking for completion on files.  */
       if (strcspn (text, 
 		   gdb_completer_file_name_break_characters) == text_len)
-	fn_list = make_source_files_completion_list (text, text);
+	fn_list = make_source_files_completion_list (cdata, text, text);
     }
 
   n_syms = VEC_length (char_ptr, list);
@@ -326,7 +338,7 @@ location_completer (struct cmd_list_element *ignore,
     {
       /* No completions at all.  As the final resort, try completing
 	 on the entire text as a symbol.  */
-      list = make_symbol_completion_list (orig_text, word);
+      list = make_symbol_completion_list (cdata, orig_text, word);
     }
 
   return list;
@@ -336,7 +348,8 @@ location_completer (struct cmd_list_element *ignore,
    method names from TYPE, a struct or union type, to the array
    OUTPUT.  */
 static void
-add_struct_fields (struct type *type, VEC (char_ptr) **output,
+add_struct_fields (struct completer_data *cdata,
+		   struct type *type, VEC (char_ptr) **output,
 		   char *fieldname, int namelen)
 {
   int i;
@@ -347,7 +360,7 @@ add_struct_fields (struct type *type, VEC (char_ptr) **output,
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
     {
       if (i < TYPE_N_BASECLASSES (type))
-	add_struct_fields (TYPE_BASECLASS (type, i),
+	add_struct_fields (cdata, TYPE_BASECLASS (type, i),
 			   output, fieldname, namelen);
       else if (TYPE_FIELD_NAME (type, i))
 	{
@@ -361,7 +374,7 @@ add_struct_fields (struct type *type, VEC (char_ptr) **output,
 	  else if (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION)
 	    {
 	      /* Recurse into anonymous unions.  */
-	      add_struct_fields (TYPE_FIELD_TYPE (type, i),
+	      add_struct_fields (cdata, TYPE_FIELD_TYPE (type, i),
 				 output, fieldname, namelen);
 	    }
 	}
@@ -389,7 +402,8 @@ add_struct_fields (struct type *type, VEC (char_ptr) **output,
    names, but some language parsers also have support for completing
    field names.  */
 VEC (char_ptr) *
-expression_completer (struct cmd_list_element *ignore, 
+expression_completer (struct completer_data *cdata,
+		      struct cmd_list_element *ignore,
 		      const char *text, const char *word)
 {
   struct type *type = NULL;
@@ -427,7 +441,7 @@ expression_completer (struct cmd_list_element *ignore,
 	  int flen = strlen (fieldname);
 	  VEC (char_ptr) *result = NULL;
 
-	  add_struct_fields (type, &result, fieldname, flen);
+	  add_struct_fields (cdata, type, &result, fieldname, flen);
 	  xfree (fieldname);
 	  return result;
 	}
@@ -437,7 +451,8 @@ expression_completer (struct cmd_list_element *ignore,
       VEC (char_ptr) *result;
       struct cleanup *cleanup = make_cleanup (xfree, fieldname);
 
-      result = make_symbol_completion_type (fieldname, fieldname, code);
+      result
+	= make_symbol_completion_type (cdata, fieldname, fieldname, code);
       do_cleanups (cleanup);
       return result;
     }
@@ -451,7 +466,7 @@ expression_completer (struct cmd_list_element *ignore,
     ;
 
   /* Not ideal but it is what we used to do before...  */
-  return location_completer (ignore, p, word);
+  return location_completer (cdata, ignore, p, word);
 }
 
 /* See definition in completer.h.  */
@@ -529,8 +544,8 @@ complete_line_internal_reason;
  */
 
 static VEC (char_ptr) *
-complete_line_internal (const char *text, 
-			const char *line_buffer, int point,
+complete_line_internal (struct completer_data *cdata,
+			const char *text, const char *line_buffer, int point,
 			complete_line_internal_reason reason)
 {
   VEC (char_ptr) *list = NULL;
@@ -615,13 +630,13 @@ complete_line_internal (const char *text,
 	  if (result_list)
 	    {
 	      if (reason != handle_brkchars)
-		list = complete_on_cmdlist (*result_list->prefixlist, p,
-					    word, ignore_help_classes);
+		list = complete_on_cmdlist (cdata, *result_list->prefixlist,
+					    p, word, ignore_help_classes);
 	    }
 	  else
 	    {
 	      if (reason != handle_brkchars)
-		list = complete_on_cmdlist (cmdlist, p, word,
+		list = complete_on_cmdlist (cdata, cmdlist, p, word,
 					    ignore_help_classes);
 	    }
 	  /* Ensure that readline does the right thing with respect to
@@ -648,8 +663,8 @@ complete_line_internal (const char *text,
 		  /* It is a prefix command; what comes after it is
 		     a subcommand (e.g. "info ").  */
 		  if (reason != handle_brkchars)
-		    list = complete_on_cmdlist (*c->prefixlist, p, word,
-						ignore_help_classes);
+		    list = complete_on_cmdlist (cdata, *c->prefixlist, p,
+						word, ignore_help_classes);
 
 		  /* Ensure that readline does the right thing
 		     with respect to inserting quotes.  */
@@ -661,7 +676,7 @@ complete_line_internal (const char *text,
 	      else if (c->enums)
 		{
 		  if (reason != handle_brkchars)
-		    list = complete_on_enum (c->enums, p, word);
+		    list = complete_on_enum (cdata, c->enums, p, word);
 		  rl_completer_word_break_characters =
 		    gdb_completer_command_word_break_characters;
 		}
@@ -701,7 +716,7 @@ complete_line_internal (const char *text,
 		      && c->completer_handle_brkchars != NULL)
 		    (*c->completer_handle_brkchars) (c, p, word);
 		  if (reason != handle_brkchars && c->completer != NULL)
-		    list = (*c->completer) (c, p, word);
+		    list = (*c->completer) (cdata, c, p, word);
 		}
 	    }
 	  else
@@ -723,7 +738,7 @@ complete_line_internal (const char *text,
 		}
 
 	      if (reason != handle_brkchars)
-		list = complete_on_cmdlist (result_list, q, word,
+		list = complete_on_cmdlist (cdata, result_list, q, word,
 					    ignore_help_classes);
 
 	      /* Ensure that readline does the right thing
@@ -747,7 +762,7 @@ complete_line_internal (const char *text,
 	  else if (c->enums)
 	    {
 	      if (reason != handle_brkchars)
-		list = complete_on_enum (c->enums, p, word);
+		list = complete_on_enum (cdata, c->enums, p, word);
 	    }
 	  else
 	    {
@@ -777,7 +792,7 @@ complete_line_internal (const char *text,
 		  && c->completer_handle_brkchars != NULL)
 		(*c->completer_handle_brkchars) (c, p, word);
 	      if (reason != handle_brkchars && c->completer != NULL)
-		list = (*c->completer) (c, p, word);
+		list = (*c->completer) (cdata, c, p, word);
 	    }
 	}
     }
@@ -804,7 +819,6 @@ new_completion_tracker (void)
 
 /* Cleanup routine to free a completion tracker and reset the pointer
    to NULL.  */
-
 static void
 free_completion_tracker (void *p)
 {
@@ -885,7 +899,7 @@ complete_line (const char *text, const char *line_buffer, int point)
 
   if (max_completions == 0)
     return NULL;
-  list = complete_line_internal (text, line_buffer, point,
+  list = complete_line_internal (NULL, text, line_buffer, point,
 				 handle_completions);
   if (max_completions < 0)
     return list;
@@ -932,17 +946,19 @@ complete_line (const char *text, const char *line_buffer, int point)
 
 /* Complete on command names.  Used by "help".  */
 VEC (char_ptr) *
-command_completer (struct cmd_list_element *ignore, 
+command_completer (struct completer_data *cdata,
+		   struct cmd_list_element *ignore,
 		   const char *text, const char *word)
 {
-  return complete_line_internal (word, text, 
+  return complete_line_internal (cdata, word, text,
 				 strlen (text), handle_help);
 }
 
 /* Complete on signals.  */
 
 VEC (char_ptr) *
-signal_completer (struct cmd_list_element *ignore,
+signal_completer (struct completer_data *cdata,
+		  struct cmd_list_element *ignore,
 		  const char *text, const char *word)
 {
   VEC (char_ptr) *return_val = NULL;
@@ -972,7 +988,8 @@ signal_completer (struct cmd_list_element *ignore,
 /* Complete on a register or reggroup.  */
 
 VEC (char_ptr) *
-reg_or_group_completer (struct cmd_list_element *ignore,
+reg_or_group_completer (struct completer_data *cdata,
+			struct cmd_list_element *ignore,
 			const char *text, const char *word)
 {
   VEC (char_ptr) *result = NULL;
@@ -1016,8 +1033,8 @@ gdb_completion_word_break_characters (void)
 {
   VEC (char_ptr) *list;
 
-  list = complete_line_internal (rl_line_buffer, rl_line_buffer, rl_point,
-				 handle_brkchars);
+  list = complete_line_internal (NULL, rl_line_buffer, rl_line_buffer,
+				 rl_point, handle_brkchars);
   gdb_assert (list == NULL);
   return rl_completer_word_break_characters;
 }
diff --git a/gdb/completer.h b/gdb/completer.h
index 56e1a2b..f32c696 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -23,6 +23,7 @@
 /* Types of functions in struct match_list_displayer.  */
 
 struct match_list_displayer;
+struct completer_data;
 
 typedef void mld_crlf_ftype (const struct match_list_displayer *);
 typedef void mld_putch_ftype (const struct match_list_displayer *, int);
@@ -75,25 +76,32 @@ extern VEC (char_ptr) *complete_line (const char *text,
 extern char *readline_line_completion_function (const char *text,
 						int matches);
 
-extern VEC (char_ptr) *noop_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *noop_completer (struct completer_data *,
+				       struct cmd_list_element *,
 				       const char *, const char *);
 
-extern VEC (char_ptr) *filename_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *filename_completer (struct completer_data *,
+					   struct cmd_list_element *,
 					   const char *, const char *);
 
-extern VEC (char_ptr) *expression_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *expression_completer (struct completer_data *,
+					     struct cmd_list_element *,
 					     const char *, const char *);
 
-extern VEC (char_ptr) *location_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *location_completer (struct completer_data *,
+					   struct cmd_list_element *,
 					   const char *, const char *);
 
-extern VEC (char_ptr) *command_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *command_completer (struct completer_data *,
+					  struct cmd_list_element *,
 					  const char *, const char *);
 
-extern VEC (char_ptr) *signal_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *signal_completer (struct completer_data *,
+					 struct cmd_list_element *,
 					 const char *, const char *);
 
-extern VEC (char_ptr) *reg_or_group_completer (struct cmd_list_element *,
+extern VEC (char_ptr) *reg_or_group_completer (struct completer_data *,
+					       struct cmd_list_element *,
 					       const char *, const char *);
 
 extern char *get_gdb_completer_quote_characters (void);
diff --git a/gdb/corefile.c b/gdb/corefile.c
index a042e6d..b5ec0e0 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -468,7 +468,8 @@ set_gnutarget_command (char *ignore, int from_tty,
 /* A completion function for "set gnutarget".  */
 
 static VEC (char_ptr) *
-complete_set_gnutarget (struct cmd_list_element *cmd,
+complete_set_gnutarget (struct completer_data *cdata,
+			struct cmd_list_element *cmd,
 			const char *text, const char *word)
 {
   static const char **bfd_targets;
@@ -486,7 +487,7 @@ complete_set_gnutarget (struct cmd_list_element *cmd,
       bfd_targets[last + 1] = NULL;
     }
 
-  return complete_on_enum (bfd_targets, text, word);
+  return complete_on_enum (cdata, bfd_targets, text, word);
 }
 
 /* Set the gnutarget.  */
diff --git a/gdb/cp-abi.c b/gdb/cp-abi.c
index b8af8f0..504bb20 100644
--- a/gdb/cp-abi.c
+++ b/gdb/cp-abi.c
@@ -359,7 +359,8 @@ set_cp_abi_cmd (char *args, int from_tty)
 /* A completion function for "set cp-abi".  */
 
 static VEC (char_ptr) *
-cp_abi_completer (struct cmd_list_element *ignore,
+cp_abi_completer (struct completer_data *cdata,
+		  struct cmd_list_element *ignore,
 		  const char *text, const char *word)
 {
   static const char **cp_abi_names;
@@ -374,7 +375,7 @@ cp_abi_completer (struct cmd_list_element *ignore,
       cp_abi_names[i] = NULL;
     }
 
-  return complete_on_enum (cp_abi_names, text, word);
+  return complete_on_enum (cdata, cp_abi_names, text, word);
 }
 
 /* Show the currently selected C++ ABI.  */
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 8b61028..6852b08 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -229,10 +229,12 @@ f_word_break_characters (void)
    class.  */
 
 static VEC (char_ptr) *
-f_make_symbol_completion_list (const char *text, const char *word,
+f_make_symbol_completion_list (struct completer_data *cdata,
+			       const char *text, const char *word,
 			       enum type_code code)
 {
-  return default_make_symbol_completion_list_break_on (text, word, ":", code);
+  return default_make_symbol_completion_list_break_on (cdata, text, word,
+						       ":", code);
 }
 
 const struct language_defn f_language_defn =
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index a693df2..2c57d17 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -378,7 +378,8 @@ cmdscm_add_completion (SCM completion, VEC (char_ptr) **result)
 /* Called by gdb for command completion.  */
 
 static VEC (char_ptr) *
-cmdscm_completer (struct cmd_list_element *command,
+cmdscm_completer (struct completer_data *cdata,
+		  struct cmd_list_element *command,
 		  const char *text, const char *word)
 {
   command_smob *c_smob/*obj*/ = (command_smob *) get_cmd_context (command);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4164a00..23f193f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7072,7 +7072,8 @@ Are you sure you want to change it? "),
 /* Complete the "handle" command.  */
 
 static VEC (char_ptr) *
-handle_completer (struct cmd_list_element *ignore,
+handle_completer (struct completer_data *cdata,
+		  struct cmd_list_element *ignore,
 		  const char *text, const char *word)
 {
   VEC (char_ptr) *vec_signals, *vec_keywords, *return_val;
@@ -7090,8 +7091,8 @@ handle_completer (struct cmd_list_element *ignore,
       NULL,
     };
 
-  vec_signals = signal_completer (ignore, text, word);
-  vec_keywords = complete_on_enum (keywords, word, word);
+  vec_signals = signal_completer (cdata, ignore, text, word);
+  vec_keywords = complete_on_enum (cdata, keywords, word, word);
 
   return_val = VEC_merge (char_ptr, vec_signals, vec_keywords);
   VEC_free (char_ptr, vec_signals);
diff --git a/gdb/interps.c b/gdb/interps.c
index 90b5b2d..ac1b512 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -437,7 +437,8 @@ interpreter_exec_cmd (char *args, int from_tty)
 
 /* List the possible interpreters which could complete the given text.  */
 static VEC (char_ptr) *
-interpreter_completer (struct cmd_list_element *ignore,
+interpreter_completer (struct completer_data *cdata,
+		       struct cmd_list_element *ignore,
 		       const char *text, const char *word)
 {
   int textlen;
diff --git a/gdb/language.h b/gdb/language.h
index 436fd6e..17b670e 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -301,9 +301,11 @@ struct language_defn
        completion is being made.  If CODE is TYPE_CODE_UNDEF, then all
        symbols should be examined; otherwise, only STRUCT_DOMAIN
        symbols whose type has a code of CODE should be matched.  */
-    VEC (char_ptr) *(*la_make_symbol_completion_list) (const char *text,
-						       const char *word,
-						       enum type_code code);
+    VEC (char_ptr) *
+      (*la_make_symbol_completion_list) (struct completer_data *,
+					 const char *text,
+					 const char *word,
+					 enum type_code code);
 
     /* The per-architecture (OS/ABI) language information.  */
     void (*la_language_arch_info) (struct gdbarch *,
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index b11fc32..21d842e 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -237,7 +237,8 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
    call.  */
 
 static PyObject *
-cmdpy_completer_helper (struct cmd_list_element *command,
+cmdpy_completer_helper (struct completer_data *cdata,
+			struct cmd_list_element *command,
 			const char *text, const char *word)
 {
   cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
@@ -293,7 +294,7 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
-  resultobj = cmdpy_completer_helper (command, text, word);
+  resultobj = cmdpy_completer_helper (NULL, command, text, word);
 
   /* Check if there was an error.  */
   if (resultobj == NULL)
@@ -330,7 +331,8 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 /* Called by gdb for command completion.  */
 
 static VEC (char_ptr) *
-cmdpy_completer (struct cmd_list_element *command,
+cmdpy_completer (struct completer_data *cdata,
+		 struct cmd_list_element *command,
 		 const char *text, const char *word)
 {
   PyObject *resultobj = NULL;
@@ -341,7 +343,7 @@ cmdpy_completer (struct cmd_list_element *command,
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
-  resultobj = cmdpy_completer_helper (command, text, word);
+  resultobj = cmdpy_completer_helper (cdata, command, text, word);
 
   /* If the result object of calling the Python function is NULL, it
      means that there was an error.  In this case, just give up and
@@ -362,7 +364,7 @@ cmdpy_completer (struct cmd_list_element *command,
 	  PyErr_Clear ();
 	}
       else if (value >= 0 && value < (long) N_COMPLETERS)
-	result = completers[value].completer (command, text, word);
+	result = completers[value].completer (cdata, command, text, word);
     }
   else
     {
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index fd2fd58..2bcb19e 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -1217,7 +1217,8 @@ simulator_command (char *args, int from_tty)
 }
 
 static VEC (char_ptr) *
-sim_command_completer (struct cmd_list_element *ignore, const char *text,
+sim_command_completer (struct completer_data *cdata,
+		       struct cmd_list_element *ignore, const char *text,
 		       const char *word)
 {
   struct sim_inferior_data *sim_data;
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 72df872..c0562e1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5007,13 +5007,15 @@ do_free_completion_list (void *list)
 
 static VEC (char_ptr) *return_val;
 
-#define COMPLETION_LIST_ADD_SYMBOL(symbol, sym_text, len, text, word) \
-      completion_list_add_name \
-	(SYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word))
+#define COMPLETION_LIST_ADD_SYMBOL(cdata, symbol, sym_text, len,	\
+				   text, word)				\
+  completion_list_add_name						\
+  ((cdata), SYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word))
 
-#define MCOMPLETION_LIST_ADD_SYMBOL(symbol, sym_text, len, text, word) \
-      completion_list_add_name \
-	(MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word))
+#define MCOMPLETION_LIST_ADD_SYMBOL(cdata, symbol, sym_text, len,	\
+				    text, word)				\
+  completion_list_add_name						\
+  (cdata, MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word))
 
 /* Tracker for how many unique completions have been generated.  Used
    to terminate completion list generation early if the list has grown
@@ -5029,7 +5031,8 @@ static completion_tracker_t completion_tracker;
    characters.  If so, add it to the current completion list.  */
 
 static void
-completion_list_add_name (const char *symname,
+completion_list_add_name (struct completer_data *cdata,
+			  const char *symname,
 			  const char *sym_text, int sym_text_len,
 			  const char *text, const char *word)
 {
@@ -5088,7 +5091,8 @@ completion_list_add_name (const char *symname,
    again and feed all the selectors into the mill.  */
 
 static void
-completion_list_objc_symbol (struct minimal_symbol *msymbol,
+completion_list_objc_symbol (struct completer_data *cdata,
+			     struct minimal_symbol *msymbol,
 			     const char *sym_text, int sym_text_len,
 			     const char *text, const char *word)
 {
@@ -5106,7 +5110,8 @@ completion_list_objc_symbol (struct minimal_symbol *msymbol,
 
   if (sym_text[0] == '[')
     /* Complete on shortened method method.  */
-    completion_list_add_name (method + 1, sym_text, sym_text_len, text, word);
+    completion_list_add_name (cdata, method + 1, sym_text, sym_text_len,
+			      text, word);
 
   while ((strlen (method) + 1) >= tmplen)
     {
@@ -5127,9 +5132,11 @@ completion_list_objc_symbol (struct minimal_symbol *msymbol,
       memcpy (tmp, method, (category - method));
       tmp[category - method] = ' ';
       memcpy (tmp + (category - method) + 1, selector, strlen (selector) + 1);
-      completion_list_add_name (tmp, sym_text, sym_text_len, text, word);
+      completion_list_add_name (cdata, tmp, sym_text, sym_text_len,
+				text, word);
       if (sym_text[0] == '[')
-	completion_list_add_name (tmp + 1, sym_text, sym_text_len, text, word);
+	completion_list_add_name (cdata, tmp + 1, sym_text, sym_text_len,
+				  text, word);
     }
 
   if (selector != NULL)
@@ -5140,7 +5147,8 @@ completion_list_objc_symbol (struct minimal_symbol *msymbol,
       if (tmp2 != NULL)
 	*tmp2 = '\0';
 
-      completion_list_add_name (tmp, sym_text, sym_text_len, text, word);
+      completion_list_add_name (cdata, tmp, sym_text, sym_text_len,
+				text, word);
     }
 }
 
@@ -5191,7 +5199,8 @@ language_search_unquoted_string (const char *text, const char *p)
 }
 
 static void
-completion_list_add_fields (struct symbol *sym, const char *sym_text,
+completion_list_add_fields (struct completer_data *cdata,
+			    struct symbol *sym, const char *sym_text,
 			    int sym_text_len, const char *text,
 			    const char *word)
 {
@@ -5204,7 +5213,7 @@ completion_list_add_fields (struct symbol *sym, const char *sym_text,
       if (c == TYPE_CODE_UNION || c == TYPE_CODE_STRUCT)
 	for (j = TYPE_N_BASECLASSES (t); j < TYPE_NFIELDS (t); j++)
 	  if (TYPE_FIELD_NAME (t, j))
-	    completion_list_add_name (TYPE_FIELD_NAME (t, j),
+	    completion_list_add_name (cdata, TYPE_FIELD_NAME (t, j),
 				      sym_text, sym_text_len, text, word);
     }
 }
@@ -5220,6 +5229,9 @@ struct add_name_data
   const char *text;
   const char *word;
 
+  /* Completion data used by the completer function.  */
+  struct completer_data *completer_data;
+
   /* Extra argument required for add_symtab_completions.  */
   enum type_code code;
 };
@@ -5234,7 +5246,7 @@ add_macro_name (const char *name, const struct macro_definition *ignore,
 {
   struct add_name_data *datum = (struct add_name_data *) user_data;
 
-  completion_list_add_name (name,
+  completion_list_add_name (datum->completer_data, name,
 			    datum->sym_text, datum->sym_text_len,
 			    datum->text, datum->word);
 }
@@ -5252,7 +5264,8 @@ symbol_completion_matcher (const char *name, void *user_data)
 /* Add matching symbols from SYMTAB to the current completion list.  */
 
 static void
-add_symtab_completions (struct compunit_symtab *cust,
+add_symtab_completions (struct completer_data *cdata,
+			struct compunit_symtab *cust,
 			const char *sym_text, int sym_text_len,
 			const char *text, const char *word,
 			enum type_code code)
@@ -5271,7 +5284,7 @@ add_symtab_completions (struct compunit_symtab *cust,
 	  if (code == TYPE_CODE_UNDEF
 	      || (SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN
 		  && TYPE_CODE (SYMBOL_TYPE (sym)) == code))
-	    COMPLETION_LIST_ADD_SYMBOL (sym,
+	    COMPLETION_LIST_ADD_SYMBOL (cdata, sym,
 					sym_text, sym_text_len,
 					text, word);
 	}
@@ -5287,14 +5300,15 @@ symtab_expansion_callback (struct compunit_symtab *symtab,
 {
   struct add_name_data *datum = (struct add_name_data *) user_data;
 
-  add_symtab_completions (symtab,
+  add_symtab_completions (datum->completer_data, symtab,
 			  datum->sym_text, datum->sym_text_len,
 			  datum->text, datum->word,
 			  datum->code);
 }
 
 static void
-default_make_symbol_completion_list_break_on_1 (const char *text,
+default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata,
+						const char *text,
 						const char *word,
 						const char *break_on,
 						enum type_code code)
@@ -5394,6 +5408,7 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
   datum.text = text;
   datum.word = word;
   datum.code = code;
+  datum.completer_data = cdata;
 
   /* At this point scan through the misc symbol vectors and add each
      symbol you find to the list.  Eventually we want to ignore
@@ -5405,17 +5420,17 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
       ALL_MSYMBOLS (objfile, msymbol)
 	{
 	  QUIT;
-	  MCOMPLETION_LIST_ADD_SYMBOL (msymbol, sym_text, sym_text_len, text,
-				       word);
+	  MCOMPLETION_LIST_ADD_SYMBOL (cdata, msymbol, sym_text, sym_text_len,
+				       text, word);
 
-	  completion_list_objc_symbol (msymbol, sym_text, sym_text_len, text,
-				       word);
+	  completion_list_objc_symbol (cdata, msymbol, sym_text, sym_text_len,
+				       text, word);
 	}
     }
 
   /* Add completions for all currently loaded symbol tables.  */
   ALL_COMPUNITS (objfile, cust)
-    add_symtab_completions (cust, sym_text, sym_text_len, text, word,
+    add_symtab_completions (cdata, cust, sym_text, sym_text_len, text, word,
 			    code);
 
   /* Look through the partial symtabs for all symbols which begin
@@ -5443,15 +5458,15 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
 	  {
 	    if (code == TYPE_CODE_UNDEF)
 	      {
-		COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text,
-					    word);
-		completion_list_add_fields (sym, sym_text, sym_text_len, text,
-					    word);
+		COMPLETION_LIST_ADD_SYMBOL (cdata, sym, sym_text,
+					    sym_text_len, text, word);
+		completion_list_add_fields (cdata, sym, sym_text,
+					    sym_text_len, text, word);
 	      }
 	    else if (SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN
 		     && TYPE_CODE (SYMBOL_TYPE (sym)) == code)
-	      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text,
-					  word);
+	      COMPLETION_LIST_ADD_SYMBOL (cdata, sym, sym_text, sym_text_len,
+					  text, word);
 	  }
 
 	/* Stop when we encounter an enclosing function.  Do not stop for
@@ -5468,11 +5483,16 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
     {
       if (surrounding_static_block != NULL)
 	ALL_BLOCK_SYMBOLS (surrounding_static_block, iter, sym)
-	  completion_list_add_fields (sym, sym_text, sym_text_len, text, word);
-
+	  {
+	    completion_list_add_fields (cdata, sym, sym_text, sym_text_len,
+					text, word);
+	  }
       if (surrounding_global_block != NULL)
 	ALL_BLOCK_SYMBOLS (surrounding_global_block, iter, sym)
-	  completion_list_add_fields (sym, sym_text, sym_text_len, text, word);
+	  {
+	    completion_list_add_fields (cdata, sym, sym_text, sym_text_len,
+					text, word);
+	  }
     }
 
   /* Skip macros if we are completing a struct tag -- arguable but
@@ -5505,7 +5525,8 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
 }
 
 VEC (char_ptr) *
-default_make_symbol_completion_list_break_on (const char *text,
+default_make_symbol_completion_list_break_on (struct completer_data *cdata,
+					      const char *text,
 					      const char *word,
 					      const char *break_on,
 					      enum type_code code)
@@ -5517,7 +5538,7 @@ default_make_symbol_completion_list_break_on (const char *text,
 
   TRY
     {
-      default_make_symbol_completion_list_break_on_1 (text, word,
+      default_make_symbol_completion_list_break_on_1 (cdata, text, word,
 						      break_on, code);
     }
   CATCH (except, RETURN_MASK_ERROR)
@@ -5532,10 +5553,12 @@ default_make_symbol_completion_list_break_on (const char *text,
 }
 
 VEC (char_ptr) *
-default_make_symbol_completion_list (const char *text, const char *word,
+default_make_symbol_completion_list (struct completer_data *cdata,
+				     const char *text, const char *word,
 				     enum type_code code)
 {
-  return default_make_symbol_completion_list_break_on (text, word, "", code);
+  return default_make_symbol_completion_list_break_on (cdata, text, word, "",
+						       code);
 }
 
 /* Return a vector of all symbols (regardless of class) which begin by
@@ -5543,9 +5566,10 @@ default_make_symbol_completion_list (const char *text, const char *word,
    is NULL.  */
 
 VEC (char_ptr) *
-make_symbol_completion_list (const char *text, const char *word)
+make_symbol_completion_list (struct completer_data *cdata,
+			     const char *text, const char *word)
 {
-  return current_language->la_make_symbol_completion_list (text, word,
+  return current_language->la_make_symbol_completion_list (cdata, text, word,
 							   TYPE_CODE_UNDEF);
 }
 
@@ -5553,30 +5577,33 @@ make_symbol_completion_list (const char *text, const char *word)
    symbols whose type code is CODE.  */
 
 VEC (char_ptr) *
-make_symbol_completion_type (const char *text, const char *word,
-			     enum type_code code)
+make_symbol_completion_type (struct completer_data *cdata, const char *text,
+			     const char *word, enum type_code code)
 {
   gdb_assert (code == TYPE_CODE_UNION
 	      || code == TYPE_CODE_STRUCT
 	      || code == TYPE_CODE_ENUM);
-  return current_language->la_make_symbol_completion_list (text, word, code);
+  return current_language->la_make_symbol_completion_list (cdata, text, word,
+							   code);
 }
 
 /* Like make_symbol_completion_list, but suitable for use as a
    completion function.  */
 
 VEC (char_ptr) *
-make_symbol_completion_list_fn (struct cmd_list_element *ignore,
+make_symbol_completion_list_fn (struct completer_data *cdata,
+				struct cmd_list_element *ignore,
 				const char *text, const char *word)
 {
-  return make_symbol_completion_list (text, word);
+  return make_symbol_completion_list (cdata, text, word);
 }
 
 /* Like make_symbol_completion_list, but returns a list of symbols
    defined in a source file FILE.  */
 
 VEC (char_ptr) *
-make_file_symbol_completion_list (const char *text, const char *word,
+make_file_symbol_completion_list (struct completer_data *cdata,
+				  const char *text, const char *word,
 				  const char *srcfile)
 {
   struct symbol *sym;
@@ -5658,13 +5685,15 @@ make_file_symbol_completion_list (const char *text, const char *word,
   b = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (s), GLOBAL_BLOCK);
   ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
+      COMPLETION_LIST_ADD_SYMBOL (cdata, sym, sym_text, sym_text_len,
+				  text, word);
     }
 
   b = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (s), STATIC_BLOCK);
   ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
+      COMPLETION_LIST_ADD_SYMBOL (cdata, sym, sym_text, sym_text_len,
+				  text, word);
     }
 
   return (return_val);
@@ -5730,6 +5759,9 @@ struct add_partial_filename_data
   const char *word;
   int text_len;
   VEC (char_ptr) **list;
+
+  /* Completion data used by the completer function.  */
+  struct completer_data *completer_data;
 };
 
 /* A callback for map_partial_symbol_filenames.  */
@@ -5766,7 +5798,8 @@ maybe_add_partial_symtab_filename (const char *filename, const char *fullname,
    NULL.  */
 
 VEC (char_ptr) *
-make_source_files_completion_list (const char *text, const char *word)
+make_source_files_completion_list (struct completer_data *cdata,
+				   const char *text, const char *word)
 {
   struct compunit_symtab *cu;
   struct symtab *s;
@@ -5817,6 +5850,7 @@ make_source_files_completion_list (const char *text, const char *word)
   datum.word = word;
   datum.text_len = text_len;
   datum.list = &list;
+  datum.completer_data = cdata;
   map_symbol_filenames (maybe_add_partial_symtab_filename, &datum,
 			0 /*need_fullname*/);
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 6a0b8da..508a7e5 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -38,6 +38,7 @@ struct program_space;
 struct language_defn;
 struct probe;
 struct common_block;
+struct completer_data;
 
 /* Some of the structures in this file are space critical.
    The space-critical structures are:
@@ -1445,24 +1446,31 @@ extern void forget_cached_source_info (void);
 extern void select_source_symtab (struct symtab *);
 
 extern VEC (char_ptr) *default_make_symbol_completion_list_break_on
-  (const char *text, const char *word, const char *break_on,
-   enum type_code code);
-extern VEC (char_ptr) *default_make_symbol_completion_list (const char *,
-							    const char *,
-							    enum type_code);
-extern VEC (char_ptr) *make_symbol_completion_list (const char *, const char *);
-extern VEC (char_ptr) *make_symbol_completion_type (const char *, const char *,
-						    enum type_code);
-extern VEC (char_ptr) *make_symbol_completion_list_fn (struct cmd_list_element *,
-						       const char *,
-						       const char *);
-
-extern VEC (char_ptr) *make_file_symbol_completion_list (const char *,
-							 const char *,
-							 const char *);
-
-extern VEC (char_ptr) *make_source_files_completion_list (const char *,
-							  const char *);
+ (struct completer_data *cdata, const char *text, const char *word,
+  const char *break_on, enum type_code code);
+extern VEC (char_ptr) *
+ default_make_symbol_completion_list (struct completer_data *,
+				      const char *, const char *,
+				      enum type_code);
+extern VEC (char_ptr) *
+  make_symbol_completion_list (struct completer_data *, const char *,
+			       const char *);
+extern VEC (char_ptr) *
+  make_symbol_completion_type (struct completer_data *,
+			       const char *, const char *,
+			       enum type_code);
+extern VEC (char_ptr) *
+  make_symbol_completion_list_fn (struct completer_data *,
+				  struct cmd_list_element *,
+				  const char *, const char *);
+
+extern VEC (char_ptr) *
+  make_file_symbol_completion_list (struct completer_data *,
+				    const char *, const char *, const char *);
+
+extern VEC (char_ptr) *
+ make_source_files_completion_list (struct completer_data *, const char *,
+				    const char *);
 
 /* symtab.c */
 

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

* [PATCH 08/18] Implement completion limiting for signal_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (9 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 17/18] Use the hashtable to accumulate completion results Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 11/18] Implement completion limiting for reg_or_group_completer Keith Seitz
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts signal_completer to use maybe_add_completion
and adds some tests to cover this new behavior.

gdb/ChangeLog

	* completer.c (signal_completer): Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/completion.exp: Add completion limiting tests
	for "handle signal".
---
 gdb/completer.c                       |   22 +++++++++++++++++++++-
 gdb/testsuite/gdb.base/completion.exp |   27 +++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 291ba73..3022d7e 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1009,7 +1009,27 @@ signal_completer (struct completer_data *cdata,
 	continue;
 
       if (strncasecmp (signame, word, len) == 0)
-	VEC_safe_push (char_ptr, return_val, xstrdup (signame));
+	{
+	  char *completion = xstrdup (signame);
+	  enum maybe_add_completion_enum add_status;
+
+	  add_status = maybe_add_completion (cdata, completion);
+	  switch (add_status)
+	    {
+	    case MAYBE_ADD_COMPLETION_OK:
+	      VEC_safe_push (char_ptr, return_val, completion);
+	      break;
+	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	      VEC_safe_push (char_ptr, return_val, completion);
+	      return return_val;
+	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	      xfree (completion);
+	      return return_val;
+	    case MAYBE_ADD_COMPLETION_DUPLICATE:
+	      xfree (completion);
+	      break;
+	    }
+	}
     }
 
   return return_val;
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 78ba216..90cdb36 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -994,3 +994,30 @@ test_completion_limit "file ./gdb.base/jit-s" \
 # same as above but completing on directory names.
 test_completion_limit "file ./gdb.a" "file \\\./gdb\\\.a(da|rch|sm)" \
     $max_completions
+
+# Test completion limiting in signal_completer.
+with_test_prefix "signal_completer" {
+    gdb_test_no_output "set max-completions unlimited"
+    set num_signals 0
+    gdb_test_multiple "complete handle signal SIG10" "" {
+	"complete handle signal SIG10" { exp_continue }
+	-re "handle signal SIG10\[0-9\]\r\n" {
+	    incr num_signals
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    pass "count available signals"
+	}
+    }
+
+    gdb_test_no_output "set max-completions $max_completions"
+}
+
+if {$num_signals > $max_completions} {
+    test_completion_limit "handle signal SIG10" \
+	"handle signal SIG10\[0-9\]" $max_completions
+} else {
+    set msg "insufficient signals to test completion "
+    append msg "limiting in signal_handler ($num_signals)"
+    untested $msg
+}

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

* [PATCH 02/18] Remove completion_tracker_t from the public completion API.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (13 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 04/18] Implement completion limiting for add_filename_to_list Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 07/18] Implement completion limiting for filename_completer Keith Seitz
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch removes the recently introduced "completion tracker" concept
from the public completion API.  In the previous patch, the completion
tracker was added to the (new) completer_data structure that is now
used by all completion functions.

Since completion_tracker_t is now used entirely inside completer.c, there
is no need to make its type opaque, so the typedef has been removed.

This patch also fixes gdb/17960, which is easily demonstrated:

$ gdb -nx -q gdb
(gdb) b gdb.c:ma<TAB>
./../src/gdb/completer.c:837: internal-error: maybe_add_completion:
Assertion `tracker != NULL' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)

This occurs because the code path in this case is:
complete_line
- complete_line_internal
-- location_completer
--- make_file_symbol_completion_list
(...)
---- completion_list_add_name
----- maybe_add_completion
----> gdb_assert (tracker != NULL);

The tracker in maybe_add_completion is a static global in symtab.c called
completion_tracker.  It is initialized by
default_make_symbol_completion_list_break_on_1 (which is a defaulted
language vector method).  In this case, this function is never called
and completion_tracker is left NULL/uninitialized.

gdb/ChangeLog

	PR gdb/17960
	* completer.c (struct completer_data) <tracker>: Change type
	to htab_t.
	(DEFAULT_MAX_COMPLETIONS): Define.
	(max_completions): Initialize global to above value.
	(new_completion_tracker): Delete.
	(new_completer_data): New function.
	(make_cleanup_free_completion_tracker): Delete.
	(free_completer_data): New function.
	(maybe_add_completion): Change argument `tracker' to struct
	completer_data and use the tracker in that structure to
	do completion limiting.
	(complete_line): Remove completion_tracker code.
	Allocate a completer_data and pass that to complete_line_internal.
	Remove now unused cleanup.
	When complete_line_internal returns more completions than are
	in the tracker, reset the tracker and use maybe_add_completion
	to implement completion tracking.
	(gdb_completion_word_break_characters): Use completer_data.
	* completer.h (completion_tracker_t): Remove.
	(new_completion_tracker): Remove.
	(make_cleanup_free_completion_tracker): Remove.
	(maybe_add_completion): Replace `completion_tracker_t tracker'
	with `struct completer_data *cdata'.
	* symtab.c (completion_tracker): Remove global variable.
	(completion_list_add_name): Pass completer_data to
	maybe_add_completion instead of completion_tracker_t.
	(default_make_symbol_completion_list_break_on_1): Remove now
	unused cleanup.
	Do not allocate a completion tracker.  Use the supplied
	completer_data instead.

gdb/testsuite/ChangeLog

	PR gdb/17960
	* gdb.base/completion.exp: Add some basic tests for the
	location completer, including a regression test for
	the gdb/17960 assertion failure.
---
 gdb/completer.c                       |  157 ++++++++++++++++++---------------
 gdb/completer.h                       |   20 ----
 gdb/symtab.c                          |   17 ----
 gdb/testsuite/gdb.base/completion.exp |   78 ++++++++++++++++
 4 files changed, 168 insertions(+), 104 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index f2b31e9..6708bb1 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -91,8 +91,8 @@ static char *gdb_completer_quote_characters = "'";
 
 struct completer_data
 {
-  /* The completion tracker being used by the completer.  */
-  completion_tracker_t tracker;
+  /* A hashtable used to track completions.  */
+  htab_t tracker;
 };
 
 \f
@@ -802,47 +802,38 @@ complete_line_internal (struct completer_data *cdata,
 
 /* See completer.h.  */
 
-int max_completions = 200;
+#define DEFAULT_MAX_COMPLETIONS 200
+int max_completions = DEFAULT_MAX_COMPLETIONS;
 
-/* See completer.h.  */
-
-completion_tracker_t
-new_completion_tracker (void)
-{
-  if (max_completions <= 0)
-    return NULL;
+/* Allocate a new completer data structure.  */
 
-  return htab_create_alloc (max_completions,
-			    htab_hash_string, (htab_eq) streq,
-			    NULL, xcalloc, xfree);
-}
-
-/* Cleanup routine to free a completion tracker and reset the pointer
-   to NULL.  */
-static void
-free_completion_tracker (void *p)
+static struct completer_data *
+new_completer_data (int size)
 {
-  completion_tracker_t *tracker_ptr = p;
+  struct completer_data *cdata = XCNEW (struct completer_data);
 
-  htab_delete (*tracker_ptr);
-  *tracker_ptr = NULL;
+  cdata->tracker
+    = htab_create_alloc ((size < 0 ? DEFAULT_MAX_COMPLETIONS : size),
+			 htab_hash_string, (htab_eq) streq, NULL,
+			 xcalloc, xfree);
+  return cdata;
 }
 
-/* See completer.h.  */
+/* Free the completion data represented by P.  */
 
-struct cleanup *
-make_cleanup_free_completion_tracker (completion_tracker_t *tracker_ptr)
+static void
+free_completer_data (void *p)
 {
-  if (*tracker_ptr == NULL)
-    return make_cleanup (null_cleanup, NULL);
+  struct completer_data *cdata = p;
 
-  return make_cleanup (free_completion_tracker, tracker_ptr);
+  htab_delete (cdata->tracker);
+  xfree (cdata);
 }
 
 /* See completer.h.  */
 
 enum maybe_add_completion_enum
-maybe_add_completion (completion_tracker_t tracker, char *name)
+maybe_add_completion (struct completer_data *cdata, char *name)
 {
   void **slot;
 
@@ -851,19 +842,17 @@ maybe_add_completion (completion_tracker_t tracker, char *name)
   if (max_completions == 0)
     return MAYBE_ADD_COMPLETION_MAX_REACHED;
 
-  gdb_assert (tracker != NULL);
-
-  if (htab_elements (tracker) >= max_completions)
+  if (htab_elements (cdata->tracker) >= max_completions)
     return MAYBE_ADD_COMPLETION_MAX_REACHED;
 
-  slot = htab_find_slot (tracker, name, INSERT);
+  slot = htab_find_slot (cdata->tracker, name, INSERT);
 
   if (*slot != HTAB_EMPTY_ENTRY)
     return MAYBE_ADD_COMPLETION_DUPLICATE;
 
   *slot = name;
 
-  return (htab_elements (tracker) < max_completions
+  return (htab_elements (cdata->tracker) < max_completions
 	  ? MAYBE_ADD_COMPLETION_OK
 	  : MAYBE_ADD_COMPLETION_OK_MAX_REACHED);
 }
@@ -892,54 +881,79 @@ complete_line (const char *text, const char *line_buffer, int point)
 {
   VEC (char_ptr) *list;
   VEC (char_ptr) *result = NULL;
-  struct cleanup *cleanups;
-  completion_tracker_t tracker;
+  struct cleanup *cdata_cleanup, *list_cleanup;
   char *candidate;
   int ix, max_reached;
+  struct completer_data *cdata;
 
   if (max_completions == 0)
     return NULL;
-  list = complete_line_internal (NULL, text, line_buffer, point,
+
+  cdata = new_completer_data (max_completions);
+  cdata_cleanup = make_cleanup (free_completer_data, cdata);
+  list = complete_line_internal (cdata, text, line_buffer, point,
 				 handle_completions);
   if (max_completions < 0)
-    return list;
-
-  tracker = new_completion_tracker ();
-  cleanups = make_cleanup_free_completion_tracker (&tracker);
-  make_cleanup_free_char_ptr_vec (list);
-
-  /* Do a final test for too many completions.  Individual completers may
-     do some of this, but are not required to.  Duplicates are also removed
-     here.  Otherwise the user is left scratching his/her head: readline and
-     complete_command will remove duplicates, and if removal of duplicates
-     there brings the total under max_completions the user may think gdb quit
-     searching too early.  */
-
-  for (ix = 0, max_reached = 0;
-       !max_reached && VEC_iterate (char_ptr, list, ix, candidate);
-       ++ix)
     {
-      enum maybe_add_completion_enum add_status;
+      do_cleanups (cdata_cleanup);
+      return list;
+    }
+
+  list_cleanup = make_cleanup_free_char_ptr_vec (list);
+
+  /* If complete_line_internal returned more completions than were
+     recorded by the completion tracker, then the completer function that
+     was run does not support completion tracking.  In this case,
+     do a final test for too many completions.
+
+     Duplicates are also removed here.  Otherwise the user is left
+     scratching his/her head: readline and complete_command will remove
+     duplicates, and if removal of duplicates there brings the total under
+     max_completions the user may think gdb quit searching too early.  */
 
-      add_status = maybe_add_completion (tracker, candidate);
+  if (VEC_length (char_ptr, list) > htab_elements (cdata->tracker))
+    {
+      /* Clear the tracker so that we can re-use it to count the number
+	 of returned completions.  */
+      htab_empty (cdata->tracker);
 
-      switch (add_status)
+      for (ix = 0, max_reached = 0;
+	   !max_reached && VEC_iterate (char_ptr, list, ix, candidate);
+	   ++ix)
 	{
-	  case MAYBE_ADD_COMPLETION_OK:
-	    VEC_safe_push (char_ptr, result, xstrdup (candidate));
-	    break;
-	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	    VEC_safe_push (char_ptr, result, xstrdup (candidate));
-	    max_reached = 1;
-	    break;
-      	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
-	    gdb_assert_not_reached ("more than max completions reached");
-	  case MAYBE_ADD_COMPLETION_DUPLICATE:
-	    break;
+	  enum maybe_add_completion_enum add_status;
+
+	  add_status = maybe_add_completion (cdata, candidate);
+
+	  switch (add_status)
+	    {
+	    case MAYBE_ADD_COMPLETION_OK:
+	      VEC_safe_push (char_ptr, result, xstrdup (candidate));
+	      break;
+	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	      VEC_safe_push (char_ptr, result, xstrdup (candidate));
+	      max_reached = 1;
+	      break;
+	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	      gdb_assert_not_reached ("more than max completions reached");
+	    case MAYBE_ADD_COMPLETION_DUPLICATE:
+	      break;
+	    }
 	}
+
+      /* The return result has been assembled and the original list from
+	 complete_line_internal is no longer needed.  Free it.  */
+      do_cleanups (list_cleanup);
+    }
+  else
+    {
+      /* There is a valid tracker for the completion -- simply return
+	 the completed list.  */
+      discard_cleanups (list_cleanup);
+      result = list;
     }
 
-  do_cleanups (cleanups);
+  do_cleanups (cdata_cleanup);
 
   return result;
 }
@@ -1032,9 +1046,14 @@ char *
 gdb_completion_word_break_characters (void)
 {
   VEC (char_ptr) *list;
+  struct completer_data *cdata;
+  struct cleanup *cleanup;
 
-  list = complete_line_internal (NULL, rl_line_buffer, rl_line_buffer,
+  cdata = new_completer_data (max_completions);
+  cleanup = make_cleanup (free_completer_data, cdata);
+  list = complete_line_internal (cdata, rl_line_buffer, rl_line_buffer,
 				 rl_point, handle_brkchars);
+  do_cleanups (cleanup);
   gdb_assert (list == NULL);
   return rl_completer_word_break_characters;
 }
diff --git a/gdb/completer.h b/gdb/completer.h
index f32c696..003c66c 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -128,24 +128,6 @@ extern const char *skip_quoted (const char *);
 
 extern int max_completions;
 
-/* Object to track how many unique completions have been generated.
-   Used to limit the size of generated completion lists.  */
-
-typedef htab_t completion_tracker_t;
-
-/* Create a new completion tracker.
-   The result is a hash table to track added completions, or NULL
-   if max_completions <= 0.  If max_completions < 0, tracking is disabled.
-   If max_completions == 0, the max is indeed zero.  */
-
-extern completion_tracker_t new_completion_tracker (void);
-
-/* Make a cleanup to free a completion tracker, and reset its pointer
-   to NULL.  */
-
-extern struct cleanup *make_cleanup_free_completion_tracker
-		      (completion_tracker_t *tracker_ptr);
-
 /* Return values for maybe_add_completion.  */
 
 enum maybe_add_completion_enum
@@ -180,7 +162,7 @@ enum maybe_add_completion_enum
    found.  */
 
 extern enum maybe_add_completion_enum
-  maybe_add_completion (completion_tracker_t tracker, char *name);
+  maybe_add_completion (struct completer_data *cdata, char *name);
 
 /* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR.  */ 
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index c0562e1..0736582 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5017,15 +5017,6 @@ static VEC (char_ptr) *return_val;
   completion_list_add_name						\
   (cdata, MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word))
 
-/* Tracker for how many unique completions have been generated.  Used
-   to terminate completion list generation early if the list has grown
-   to a size so large as to be useless.  This helps avoid GDB seeming
-   to lock up in the event the user requests to complete on something
-   vague that necessitates the time consuming expansion of many symbol
-   tables.  */
-
-static completion_tracker_t completion_tracker;
-
 /*  Test to see if the symbol specified by SYMNAME (which is already
    demangled for C++ symbols) matches SYM_TEXT in the first SYM_TEXT_LEN
    characters.  If so, add it to the current completion list.  */
@@ -5067,7 +5058,7 @@ completion_list_add_name (struct completer_data *cdata,
 	strcat (newobj, symname);
       }
 
-    add_status = maybe_add_completion (completion_tracker, newobj);
+    add_status = maybe_add_completion (cdata, newobj);
 
     switch (add_status)
       {
@@ -5329,7 +5320,6 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata,
   /* Length of sym_text.  */
   int sym_text_len;
   struct add_name_data datum;
-  struct cleanup *cleanups;
 
   /* Now look for the symbol we are supposed to complete on.  */
   {
@@ -5400,9 +5390,6 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata,
     }
   gdb_assert (sym_text[sym_text_len] == '\0' || sym_text[sym_text_len] == '(');
 
-  completion_tracker = new_completion_tracker ();
-  cleanups = make_cleanup_free_completion_tracker (&completion_tracker);
-
   datum.sym_text = sym_text;
   datum.sym_text_len = sym_text_len;
   datum.text = text;
@@ -5520,8 +5507,6 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata,
       /* User-defined macros are always visible.  */
       macro_for_each (macro_user_macros, add_macro_name, &datum);
     }
-
-  do_cleanups (cleanups);
 }
 
 VEC (char_ptr) *
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index f77bfe2..5afd851 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -777,6 +777,84 @@ gdb_test_multiple "" "$test" {
 }
 
 #
+# Tests for the location completer
+#
+
+# Turn off pending breakpoint support so that we don't get queried
+# all the time.
+gdb_test_no_output "set breakpoint pending off"
+
+set subsrc [string range $srcfile 0 [expr {[string length $srcfile] - 3}]]
+set test "tab complete break $subsrc"
+send_gdb "break $subsrc\t\t"
+gdb_test_multiple "" $test {
+    -re "break\.c.*break1\.c.*$gdb_prompt " {
+	send_gdb "1\t\n"
+	gdb_test_multiple "" $test {
+	    -re ".*Function \"$srcfile2\" not defined\..*$gdb_prompt " {
+		pass $test
+	    }
+	    -re "$gdb_prompt p$" {
+		fail $test
+	    }
+	}
+    }
+
+    -re "$gdb_prompt p$" {
+	fail $test
+    }
+}
+
+gdb_test "complete break $subsrc" "break\.c.*break1\.c"
+
+# gdb/17960
+set test "tab complete break $srcfile:ma"
+send_gdb "break $srcfile:ma\t"
+gdb_test_multiple "" $test {
+    -re "break $srcfile:main " {
+	send_gdb "\n"
+	gdb_test_multiple "" $test {
+	    -re ".*Breakpoint.*at .*/$srcfile, line .*$gdb_prompt " {
+		pass $test
+		gdb_test_no_output "delete breakpoint \$bpnum" \
+		    "delete breakpoint for $test"
+	    }
+	    -re "$gdb_prompt p$" {
+		fail $test
+	    }
+	}
+    }
+    -re "$gdb_prompt p$" {
+	fail $test
+    }
+}
+
+gdb_test "complete break $srcfile:ma" "break\.c:main"
+
+set test "tab complete break need"
+send_gdb "break need\t"
+gdb_test_multiple "" $test {
+    -re "break need_malloc " {
+	send_gdb "\n"
+	gdb_test_multiple "" $test {
+	    -re ".*Breakpoint.*at .*/$srcfile, line .*$gdb_prompt " {
+		pass $test
+		gdb_test_no_output "delete breakpoint \$bpnum" \
+		    "delete breakpoint for $test"
+	    }
+	    -re "$gdb_prompt p$" {
+		fail $test
+	    }
+	}
+    }
+    -re "$gdb_prompt p$" {
+	fail $test
+    }
+}
+
+gdb_test "complete break need" "need_malloc"
+
+#
 # Completion limiting.
 #
 

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

* [PATCH 17/18] Use the hashtable to accumulate completion results.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (8 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 13/18] Implement completion limiting for complete_on_enum Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 08/18] Implement completion limiting for signal_completer Keith Seitz
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

The completion API now uses a VEC (char_ptr) to collect results of
completion.  The completion tracker is a hashtable whose elements are
the completions.  Both hold essentially the same data, so there is no
need to keep both around.

This patch introduces some API support for removing the vector of
completions altogether.  While it does not remove the vector or
the vector return result from the completer functions, this patch
does not use the results of the vector at all.  Only the results of
the hashtable inside the completer's private data is used.

The vector will be removed in the next patch.

gdb/ChangeLog

	* cli/cli-decode.c (complete_on_cmdlist): Use get_completion_count
	to ascertain if there are any completion results.
	* completer.c (remove_leading_fn_component): New function.
	(location_completer): Use get_completion_count to figure out
	how many symbols and/or file names were found searching for
	possible completions.
	Traverse the completion tracker hashtable to strip leading
	file name components.  Contents moved to remove_leading_fn_component.
	(free_completer_data): Change argument to proper type.
	(free_entry_callback): New function.
	(free_all_completer_data): New function.
	(vectorize_htab): New function.
	(get_completion_list): New function.
	(get_completion_count): New function.
	(maybe_add_completion): Accumulate completions when not limiting
	the number of completions.
	(complete_line): Ignore the return list from complete_line_internal
	and get the completion results from the tracker.
	Do not count/limit the results at all -- it is no longer necessary.
	Use free_all_completer_data to free any allocated memory during
	completion in the case of an exception.
	Use free_completer_data after get_completion_list to free
	completer data structures.
	(gdb_completion_word_break_characters): Ignore the return list
	from complete_line_internal and get the completion results from
	the tracker.
	Use free_completer_data after get_completion_list to free
	completer data structures.
	* completer.h (get_completion_list): Declare.
	(get_completion_count): Declare.
	* python/py-cmd.c (cmdpy_completer): Use get_completion_count
	to ascertain if there are any completion results.
---
 gdb/cli/cli-decode.c |    2 -
 gdb/completer.c      |  189 ++++++++++++++++++++++++++------------------------
 gdb/completer.h      |    9 ++
 gdb/python/py-cmd.c  |    2 -
 4 files changed, 110 insertions(+), 92 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 04eb437..4e0fbf7 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1781,7 +1781,7 @@ complete_on_cmdlist (struct completer_data *cdata,
      commands.  If we see no matching commands in the first pass, and
      if we did happen to see a matching deprecated command, we do
      another loop to collect those.  */
-  for (pass = 0; matchlist == 0 && pass < 2; ++pass)
+  for (pass = 0; get_completion_count (cdata) == 0 && pass < 2; ++pass)
     {
       for (ptr = list; ptr; ptr = ptr->next)
 	if (!strncmp (ptr->name, text, textlen)
diff --git a/gdb/completer.c b/gdb/completer.c
index 5d06784..425b5aa 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -208,6 +208,20 @@ filename_completer (struct completer_data *cdata,
   return return_val;
 }
 
+/* A hashtable traversal function to remove leading file name
+   components, as required by rl_complete.  See more detailed explanation
+   in location_completer for more.  */
+
+static int
+remove_leading_fn_component (void **slot, void *calldata)
+{
+  char *fn = *slot;
+  int offset = *(int *) calldata;
+
+  memmove (fn, fn + offset, strlen (fn) + 1 - offset);
+  return 1;
+}
+
 /* Complete on locations, which might be of two possible forms:
 
        file:line
@@ -297,21 +311,25 @@ location_completer (struct completer_data *cdata,
     {
       list = make_file_symbol_completion_list (cdata, symbol_start, word,
 					       file_to_match);
+      n_syms = get_completion_count (cdata);
+      n_files = 0;
       xfree (file_to_match);
     }
   else
     {
       list = make_symbol_completion_list (cdata, symbol_start, word);
+      n_syms = get_completion_count (cdata);
+      n_files = 0;
       /* If text includes characters which cannot appear in a file
 	 name, they cannot be asking for completion on files.  */
       if (strcspn (text, 
 		   gdb_completer_file_name_break_characters) == text_len)
-	fn_list = make_source_files_completion_list (cdata, text, text);
+	{
+	  fn_list = make_source_files_completion_list (cdata, text, text);
+	  n_files = get_completion_count (cdata) - n_syms;
+	}
     }
 
-  n_syms = VEC_length (char_ptr, list);
-  n_files = VEC_length (char_ptr, fn_list);
-
   /* Catenate fn_list[] onto the end of list[].  */
   if (!n_syms)
     {
@@ -334,7 +352,7 @@ location_completer (struct completer_data *cdata,
     }
   else if (n_files)
     {
-      char *fn;
+      int offset = word - text;
 
       /* If we only have file names as possible completion, we should
 	 bring them in sync with what rl_complete expects.  The
@@ -349,13 +367,10 @@ location_completer (struct completer_data *cdata,
 	 the full "/foo/bar" and "/foo/baz" strings.  This produces
 	 wrong results when, e.g., there's only one possible
 	 completion, because rl_complete will prepend "/foo/" to each
-	 candidate completion.  The loop below removes that leading
+	 candidate completion.  The callback below removes that leading
 	 part.  */
-      for (ix = 0; VEC_iterate (char_ptr, list, ix, fn); ++ix)
-	{
-	  memmove (fn, fn + (word - text),
-		   strlen (fn) + 1 - (word - text));
-	}
+      htab_traverse (cdata->tracker, remove_leading_fn_component,
+		     &offset);
     }
   else if (!n_syms)
     {
@@ -879,14 +894,68 @@ new_completer_data (int size)
 /* Free the completion data represented by P.  */
 
 static void
-free_completer_data (void *p)
+free_completer_data (struct completer_data *cdata)
 {
-  struct completer_data *cdata = p;
-
   htab_delete (cdata->tracker);
   xfree (cdata);
 }
 
+/* A hashtable traversal function to free the elements of the table.  */
+
+static int
+free_entry_callback (void **slot, void *calldata)
+{
+  char *element = *slot;
+
+  xfree (element);
+  return 1;
+}
+
+/* A cleanup function to free all data associated with the completer_data
+   given by P.  */
+
+static void
+free_all_completer_data (void *p)
+{
+  struct completer_data *cdata = p;
+
+  htab_traverse (cdata->tracker, free_entry_callback, NULL);
+  free_completer_data (cdata);
+}
+
+/* A hashtable traversal function to turn the hashtable keys
+   into a vector.  */
+
+static int
+vectorize_htab (void **slot, void *calldata)
+{
+  char *element = *slot;
+  VEC (char_ptr) **vector = calldata;
+
+  VEC_safe_push (char_ptr, *vector, element);
+  return 1;
+}
+
+/* See completer.h.  */
+
+VEC (char_ptr) *
+get_completion_list (const struct completer_data *cdata)
+{
+  VEC (char_ptr) *result = NULL;
+
+  htab_traverse (cdata->tracker, vectorize_htab, &result);
+  return result;
+}
+
+/* See completer.h.  */
+
+size_t
+get_completion_count (const struct completer_data *cdata)
+{
+  return htab_elements (cdata->tracker);
+}
+
+
 /* See completer.h.  */
 
 int
@@ -902,12 +971,11 @@ maybe_add_completion (struct completer_data *cdata, char *name)
 {
   void **slot;
 
-  if (max_completions < 0)
-    return MAYBE_ADD_COMPLETION_OK;
   if (max_completions == 0)
     return MAYBE_ADD_COMPLETION_MAX_REACHED;
 
-  if (htab_elements (cdata->tracker) >= max_completions)
+  if (max_completions > 0
+      && htab_elements (cdata->tracker) >= max_completions)
     return MAYBE_ADD_COMPLETION_MAX_REACHED;
 
   slot = htab_find_slot (cdata->tracker, name, INSERT);
@@ -917,7 +985,8 @@ maybe_add_completion (struct completer_data *cdata, char *name)
 
   *slot = name;
 
-  return (htab_elements (cdata->tracker) < max_completions
+  return ((max_completions < 0
+	   || htab_elements (cdata->tracker) < max_completions)
 	  ? MAYBE_ADD_COMPLETION_OK
 	  : MAYBE_ADD_COMPLETION_OK_MAX_REACHED);
 }
@@ -946,82 +1015,20 @@ throw_max_completions_reached_error (void)
 VEC (char_ptr) *
 complete_line (const char *text, const char *line_buffer, int point)
 {
-  VEC (char_ptr) *list;
   VEC (char_ptr) *result = NULL;
   struct cleanup *cdata_cleanup, *list_cleanup;
-  char *candidate;
-  int ix, max_reached;
   struct completer_data *cdata;
 
   if (max_completions == 0)
     return NULL;
 
   cdata = new_completer_data (max_completions);
-  cdata_cleanup = make_cleanup (free_completer_data, cdata);
-  list = complete_line_internal (cdata, text, line_buffer, point,
-				 handle_completions);
-  if (max_completions < 0)
-    {
-      do_cleanups (cdata_cleanup);
-      return list;
-    }
-
-  list_cleanup = make_cleanup_free_char_ptr_vec (list);
-
-  /* If complete_line_internal returned more completions than were
-     recorded by the completion tracker, then the completer function that
-     was run does not support completion tracking.  In this case,
-     do a final test for too many completions.
-
-     Duplicates are also removed here.  Otherwise the user is left
-     scratching his/her head: readline and complete_command will remove
-     duplicates, and if removal of duplicates there brings the total under
-     max_completions the user may think gdb quit searching too early.  */
-
-  if (VEC_length (char_ptr, list) > htab_elements (cdata->tracker))
-    {
-      /* Clear the tracker so that we can re-use it to count the number
-	 of returned completions.  */
-      htab_empty (cdata->tracker);
-
-      for (ix = 0, max_reached = 0;
-	   !max_reached && VEC_iterate (char_ptr, list, ix, candidate);
-	   ++ix)
-	{
-	  enum maybe_add_completion_enum add_status;
-
-	  add_status = maybe_add_completion (cdata, candidate);
-
-	  switch (add_status)
-	    {
-	    case MAYBE_ADD_COMPLETION_OK:
-	      VEC_safe_push (char_ptr, result, xstrdup (candidate));
-	      break;
-	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
-	      VEC_safe_push (char_ptr, result, xstrdup (candidate));
-	      max_reached = 1;
-	      break;
-	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
-	      gdb_assert_not_reached ("more than max completions reached");
-	    case MAYBE_ADD_COMPLETION_DUPLICATE:
-	      break;
-	    }
-	}
-
-      /* The return result has been assembled and the original list from
-	 complete_line_internal is no longer needed.  Free it.  */
-      do_cleanups (list_cleanup);
-    }
-  else
-    {
-      /* There is a valid tracker for the completion -- simply return
-	 the completed list.  */
-      discard_cleanups (list_cleanup);
-      result = list;
-    }
-
-  do_cleanups (cdata_cleanup);
-
+  cdata_cleanup = make_cleanup (free_all_completer_data, cdata);
+  complete_line_internal (cdata, text, line_buffer, point,
+			  handle_completions);
+  result = get_completion_list (cdata);
+  discard_cleanups (cdata_cleanup);
+  free_completer_data (cdata);
   return result;
 }
 
@@ -1175,10 +1182,12 @@ gdb_completion_word_break_characters (void)
   struct cleanup *cleanup;
 
   cdata = new_completer_data (max_completions);
-  cleanup = make_cleanup (free_completer_data, cdata);
-  list = complete_line_internal (cdata, rl_line_buffer, rl_line_buffer,
-				 rl_point, handle_brkchars);
-  do_cleanups (cleanup);
+  cleanup = make_cleanup (free_all_completer_data, cdata);
+  complete_line_internal (cdata, rl_line_buffer, rl_line_buffer,
+			  rl_point, handle_brkchars);
+  list = get_completion_list (cdata);
+  discard_cleanups (cleanup);
+  free_completer_data (cdata);
   gdb_assert (list == NULL);
   return rl_completer_word_break_characters;
 }
diff --git a/gdb/completer.h b/gdb/completer.h
index 67fdcad..0724590 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -128,6 +128,15 @@ extern const char *skip_quoted (const char *);
 
 extern int get_maximum_completions (void);
 
+/* Get the list of completions.  */
+
+extern VEC (char_ptr) *
+  get_completion_list (const struct completer_data *cdata);
+
+/* Get the number of completions in CDATA.  */
+
+extern size_t get_completion_count (const struct completer_data *cdata);
+
 /* Return values for maybe_add_completion.  */
 
 enum maybe_add_completion_enum
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index ed97ce5..7926157 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -419,7 +419,7 @@ cmdpy_completer (struct completer_data *cdata,
 
       /* If we got some results, ignore problems.  Otherwise, report
 	 the problem.  */
-      if (result != NULL && PyErr_Occurred ())
+      if (get_completion_count (cdata) > 0 && PyErr_Occurred ())
 	PyErr_Clear ();
     }
 

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

* [PATCH 14/18] Implement completion limiting in add_struct_fields.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 09/18] Implement completion limiting for interpreter_completer Keith Seitz
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts add_struct_fields (used by expression_completer) to use
maybe_add_completion and adds (coverage) tests for this new behavior.

break.exp contains two overly greedy regexp patterns which required
tweaking, too.

gdb/ChangeLog

	* completer.c (add_struct_fields): Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/break.c (struct field_test): New structure.
	(field_test_global): New global variable.
	* gdb.base/break.exp: Add srcfile to otherwise too greedy regexp
	for tests "check disable with history value" and "check disable
	with convenience values".
	* gdb.base/completion.exp: Add completion limiting tests for
	add_struct_fields using the print command.
	* gdb.cp/cpcompletion.exp (test_completion_limit): New procedure.
	Add completion limiting tests for C++-specific parts of
	add_struct_fields using the print command.
	* gdb.cp/pr9694.cc (class Foo) <repeated1, repeated2, repeated3,
	repeated4, repeated5, repeated6, repeated7, repeated8>: New members.
	(Foo::Foo): Add initializer for new members.
	(main): Add new methods method1, method2, method3, method4, method5,
	method6, method7, and method8 to anonymous structure and make
	sure they are not optimized away.
---
 gdb/completer.c                       |   45 +++++++++++++++++++++++++--
 gdb/testsuite/gdb.base/break.c        |   14 ++++++++
 gdb/testsuite/gdb.base/break.exp      |    4 +-
 gdb/testsuite/gdb.base/completion.exp |    4 ++
 gdb/testsuite/gdb.cp/cpcompletion.exp |   55 +++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/pr9594.cc        |   28 ++++++++++++++++-
 6 files changed, 144 insertions(+), 6 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index b112c2c..45170e3 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -384,8 +384,27 @@ add_struct_fields (struct completer_data *cdata,
 	    {
 	      if (! strncmp (TYPE_FIELD_NAME (type, i), 
 			     fieldname, namelen))
-		VEC_safe_push (char_ptr, *output,
-			       xstrdup (TYPE_FIELD_NAME (type, i)));
+		{
+		  char *match = xstrdup (TYPE_FIELD_NAME (type, i));
+		  enum maybe_add_completion_enum add_status;
+
+		  add_status = maybe_add_completion (cdata, match);
+		  switch (add_status)
+		    {
+		    case MAYBE_ADD_COMPLETION_OK:
+		      VEC_safe_push (char_ptr, *output, match);
+		      break;
+		    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+		      VEC_safe_push (char_ptr, *output, match);
+		      return;
+		    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+		      xfree (match);
+		      return;
+		    case MAYBE_ADD_COMPLETION_DUPLICATE:
+		      xfree (match);
+		      break;
+		    }
+		}
 	    }
 	  else if (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION)
 	    {
@@ -409,7 +428,27 @@ add_struct_fields (struct completer_data *cdata,
 	    }
 	  /* Omit constructors from the completion list.  */
 	  if (!type_name || strcmp (type_name, name))
-	    VEC_safe_push (char_ptr, *output, xstrdup (name));
+	    {
+	      char *match = xstrdup (name);
+	      enum maybe_add_completion_enum add_status;
+
+	      add_status = maybe_add_completion (cdata, match);
+	      switch (add_status)
+		{
+		case MAYBE_ADD_COMPLETION_OK:
+		  VEC_safe_push (char_ptr, *output, match);
+		  break;
+		case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+		  VEC_safe_push (char_ptr, *output, match);
+		  return;
+		case MAYBE_ADD_COMPLETION_MAX_REACHED:
+		  xfree (match);
+		  return;
+		case MAYBE_ADD_COMPLETION_DUPLICATE:
+		  xfree (match);
+		  break;
+		}
+	    }
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.base/break.c b/gdb/testsuite/gdb.base/break.c
index d017a91..8ee11b1 100644
--- a/gdb/testsuite/gdb.base/break.c
+++ b/gdb/testsuite/gdb.base/break.c
@@ -30,6 +30,20 @@ void *need_malloc ()
   return malloc (1);
 }
 
+/* A structure for testing add_struct_fields.  */
+struct field_test
+{
+  int a1_field;
+  int a2_field;
+  int a3_field;
+  int a4_field;
+  int a5_field;
+  int a6_field;
+  int a7_field;
+};
+
+struct field_test field_test_global = {1, 2, 3, 4, 5, 6, 7};
+
 /*
  *	This simple classical example of recursion is useful for
  *	testing stack backtraces and such.
diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
index f879bc8..5344533 100644
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -249,7 +249,7 @@ set see5 0
 set see6 0
 
 gdb_test_multiple "info break" "check disable with history values" {
-    -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
+    -re "1\[\t \]+breakpoint *keep y.* at .*$srcfile:$main_line\[\r\n\]*" {
 	set see1 1
 	exp_continue
     }
@@ -295,7 +295,7 @@ set see5 0
 set see6 0
 
 gdb_test_multiple "info break" "check disable with convenience values" {
-    -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
+    -re "1\[\t \]+breakpoint *keep y.* in main at .*$srcfile:$main_line\[^\r\n\]*" {
 	set see1 1
 	exp_continue
     }
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 516975c..0466dfb 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -1053,3 +1053,7 @@ if {$signal_to_use != ""} {
 	"handle signal $signal_to_use n\[a-z\]+" \
 	$max_completions
 }
+
+# Test add_struct_fields.
+test_completion_limit "print field_test_global.a" \
+    "print field_test_global\\\.a\[1-9\]_field" $max_completions
diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
index 023c261..a1bc6d8 100644
--- a/gdb/testsuite/gdb.cp/cpcompletion.exp
+++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
@@ -85,3 +85,58 @@ gdb_test "complete p foo1.Fo" "p foo1\\.Foofoo"
 
 # Test completion with an anonymous struct.
 gdb_test "complete p a.g" "p a\\.get"
+
+# Test completion limiting (add_struct_fields)
+set max_completions 3
+gdb_test_no_output "set max-completions $max_completions"
+
+# A convenience function for testing completion limiting.
+# CMD is a GDB command to to run with "complete".
+# PATTERN is a regexp pattern matching the expected output
+#   of completion items "seen" in the output.
+# NUM is the number of maximum completions expected.
+#
+# The test will use the test name "limit complete CMD"
+# and will only count the number of completion items matching
+# PATTERN.  No assumptions are made on the order of the items
+# seen in GDB's output.
+#
+# If NUM items are seen before the truncation message, the test
+# passes, otherwise it fails.  The test can also fail if no
+# truncation message is seen at all, in which case the test
+# failure message will say "(unlimited)".
+
+proc test_completion_limit {cmd pattern num} {
+    global gdb_prompt
+
+    # The terminal at the end of the complete command
+    set end "\\\*\\\*\\\* List may be truncated, "
+    append end "max-completions reached\\\. \\\*\\\*\\\*"
+
+    set cmdr [string_to_regexp $cmd]
+    set seen 0
+    gdb_test_multiple "complete $cmd" "limit complete $cmd" {
+	"complete $cmdr" { exp_continue }
+	-re "$pattern\r\n" {
+	    incr seen
+	    exp_continue
+	}
+	-re ".*$end\r\n$gdb_prompt $" {
+	    if {$seen == $num} {
+		pass "limit complete $cmd"
+	    } else {
+		fail "limit complete $cmd ($seen/$num)"
+	    }
+	}
+	-re ".*$gdb_prompt $" {
+	    fail "limit complete $cmd (unlimited)"
+	}
+    }
+}
+
+# Completion limiting tests for add_struct_fields.
+test_completion_limit "print a.meth" "print a\\\.method\[1-8\]" \
+    $max_completions
+
+test_completion_limit "print foo1.repeat" \
+    "print foo1\\\.repeated\[1-8\]" $max_completions
diff --git a/gdb/testsuite/gdb.cp/pr9594.cc b/gdb/testsuite/gdb.cp/pr9594.cc
index 8fdee84..e4b49ce 100644
--- a/gdb/testsuite/gdb.cp/pr9594.cc
+++ b/gdb/testsuite/gdb.cp/pr9594.cc
@@ -11,9 +11,19 @@ class Foo : public Base
 
 private:
   int foo_value;
+  int repeated1;
+  int repeated2;
+  int repeated3;
+  int repeated4;
+  int repeated5;
+  int repeated6;
+  int repeated7;
+  int repeated8;
 
 public:
-  Foo () { foo_value = 0;}
+  Foo () : foo_value (0), repeated1 (1), repeated2 (2), repeated3 (3),
+	   repeated4 (4), repeated5 (5), repeated6 (6), repeated7 (7),
+	   repeated8 (8) {}
   Foo (int i) { foo_value = i;}
   ~Foo () { }
   void set_foo (int value);
@@ -44,9 +54,25 @@ int main ()
   // Anonymous struct with method.
   struct {
     int get() { return 5; }
+    int method1 (void) { return 1; }
+    int method2 (void) { return 2; }
+    int method3 (void) { return 3; }
+    int method4 (void) { return 4; }
+    int method5 (void) { return 5; }
+    int method6 (void) { return 6; }
+    int method7 (void) { return 7; }
+    int method8 (void) { return 8; }
   } a;
   Foo foo1;
   foo1.set_foo (42);		// Set breakpoint here.
   a.get();			// Prevent compiler from throwing 'a' away.
+  a.method1 ();
+  a.method2 ();
+  a.method3 ();
+  a.method4 ();
+  a.method5 ();
+  a.method6 ();
+  a.method7 ();
+  a.method8 ();
   return 0;
 }

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

* [PATCH 10/18] Implement completion limiting for cmdpy_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (2 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 06/18] Implement completion limiting for condition_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 12/18] Implement completion limiting for sim_command_completer Keith Seitz
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts cmdpy_completer, used by commands written in
python.  It also adds tests for some untested python functionality
related to completion.

gdb/ChangeLog

	* python/py-cmd.c (cmdpy_completer) Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.python/py-completion.exp: Test completion functions,
	with and without completion limiting.
---
 gdb/python/py-cmd.c                        |   24 ++++++++++++++-
 gdb/testsuite/gdb.python/py-completion.exp |   45 ++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 21d842e..ed97ce5 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -370,13 +370,15 @@ cmdpy_completer (struct completer_data *cdata,
     {
       PyObject *iter = PyObject_GetIter (resultobj);
       PyObject *elt;
+      int stop = 0;
 
       if (iter == NULL)
 	goto done;
 
-      while ((elt = PyIter_Next (iter)) != NULL)
+      while (!stop && (elt = PyIter_Next (iter)) != NULL)
 	{
 	  char *item;
+	  enum maybe_add_completion_enum add_status;
 
 	  if (! gdbpy_is_string (elt))
 	    {
@@ -392,7 +394,25 @@ cmdpy_completer (struct completer_data *cdata,
 	      PyErr_Clear ();
 	      continue;
 	    }
-	  VEC_safe_push (char_ptr, result, item);
+
+	  add_status = maybe_add_completion (cdata, item);
+	  switch (add_status)
+	    {
+	    case MAYBE_ADD_COMPLETION_OK:
+	      VEC_safe_push (char_ptr, result, item);
+	      break;
+	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	      VEC_safe_push (char_ptr, result, item);
+	      stop = 1;
+	      break;
+	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	      xfree (item);
+	      stop = 1;
+	      break;
+	    case MAYBE_ADD_COMPLETION_DUPLICATE:
+	      xfree (item);
+	      break;
+	    }
 	}
 
       Py_DECREF (iter);
diff --git a/gdb/testsuite/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp
index 5e45087..f7f23a3 100644
--- a/gdb/testsuite/gdb.python/py-completion.exp
+++ b/gdb/testsuite/gdb.python/py-completion.exp
@@ -128,3 +128,48 @@ if {[readline_is_used]} {
 	    "completelimit2 cl29"
 	}
 }
+
+# The terminal at the end of the complete command
+set end "\\\*\\\*\\\* List may be truncated, "
+append end "max-completions reached\\\. \\\*\\\*\\\*"
+
+set max_completions 3
+gdb_test_no_output "set max-completions $max_completions"
+set seen 0
+
+set testname "limit completions of 'complete completel'"
+gdb_test_multiple "complete completel" $testname {
+    "complete completel" { exp_continue }
+
+    -re "completelimit\[1-9\]+\r\n" {
+	incr seen
+	exp_continue
+    }
+
+    -re "completel $end\r\n$gdb_prompt $" {
+	if {$seen == $max_completions} {
+	    pass $testname
+	} else {
+	    fail "$testname ($seen/$max_completions)"
+	}
+    }
+}
+
+set testname "limit completions of 'complete completelimit1 c'"
+set seen 0
+gdb_test_multiple "complete completelimit1 c" $testname {
+    "complete completelimit1 c" { exp_continue }
+
+    -re "completelimit1 cl1\[1-9\]+\r\n" {
+	incr seen
+	exp_continue
+    }
+
+    -re "completelimit1 c $end\r\n$gdb_prompt $" {
+	if {$seen == $max_completions} {
+	    pass $testname
+	} else {
+	    fail "$testname ($seen/$max_completions)"
+	}
+    }
+}

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

* [PATCH 12/18] Implement completion limiting for sim_command_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (3 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 10/18] Implement completion limiting for cmdpy_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-14 15:28   ` Mike Frysinger
  2015-04-13 19:23 ` [PATCH 16/18] Make the completion API completely opaque Keith Seitz
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts sim_command_completer to use maybe_add_completion.
It does not add any tests, since the `sim' command is highly
target-dependent and unimplemented for the majority of simulators.

gdb/ChangeLog

	* remote-sim.c: Include completer.h.
	(sim_command_completer): Use maybe_add_completion.
---
 gdb/remote-sim.c |   28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 2bcb19e..5056a13 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -39,6 +39,7 @@
 #include "arch-utils.h"
 #include "readline/readline.h"
 #include "gdbthread.h"
+#include "completer.h"
 
 /* Prototypes */
 
@@ -1223,7 +1224,7 @@ sim_command_completer (struct completer_data *cdata,
 {
   struct sim_inferior_data *sim_data;
   char **tmp;
-  int i;
+  int i, stop;
   VEC (char_ptr) *result = NULL;
 
   sim_data = inferior_data (current_inferior (), sim_inferior_data_key);
@@ -1235,8 +1236,29 @@ sim_command_completer (struct completer_data *cdata,
     return NULL;
 
   /* Transform the array into a VEC, and then free the array.  */
-  for (i = 0; tmp[i] != NULL; i++)
-    VEC_safe_push (char_ptr, result, tmp[i]);
+  for (i = 0, stop = 0; !stop && tmp[i] != NULL; i++)
+    {
+      enum maybe_add_completion_enum add_status;
+
+      add_status = maybe_add_completion (cdata, tmp[i]);
+      switch (add_status)
+	{
+	case MAYBE_ADD_COMPLETION_OK:
+	  VEC_safe_push (char_ptr, result, tmp[i]);
+	  break;
+	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	  VEC_safe_push (char_ptr, result, tmp[i]);
+	  stop = 1;
+	  break;
+	case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	  xfree (tmp[i]);
+	  stop = 1;
+	  break;
+	case MAYBE_ADD_COMPLETION_DUPLICATE:
+	  xfree (tmp[i]);
+	  break;
+	}
+    }
   xfree (tmp);
 
   return result;

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

* [PATCH 03/18] Implement completion limiting for complete_on_cmdlist.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (6 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 01/18] Add struct completer_data to the completion API Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 13/18] Implement completion limiting for complete_on_enum Keith Seitz
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This is the first of a series of smaller patches to switch over
all completion functions to using maybe_add_completion to add
completions to the completion list to be presented to the user.

Note that in order to verify that this patch works as intended,
one must override the backup completion counting in complete_line.
[This backup code will be permanently removed in a later patch.]
During testing, I have verified all patches with this planned code
removal to verify that it works.

First up is complete_on_cmdlist.  Completion limiting is already tested
in gdb.base/completion.exp, so there are no new tests.

gdb/ChangeLog

	* cli/cli-decode.c (complete_on_cmdlist): Use maybe_add_completion
	to determine whether to continue looking for completions.
---
 gdb/cli/cli-decode.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 286fc61..21f4c45 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1790,6 +1790,7 @@ complete_on_cmdlist (struct completer_data *cdata,
 		|| ptr->prefixlist))
 	  {
 	    char *match;
+	    enum maybe_add_completion_enum add_status;
 
 	    if (pass == 0)
 	      {
@@ -1815,7 +1816,22 @@ complete_on_cmdlist (struct completer_data *cdata,
 		match[text - word] = '\0';
 		strcat (match, ptr->name);
 	      }
-	    VEC_safe_push (char_ptr, matchlist, match);
+	    add_status = maybe_add_completion (cdata, match);
+	    switch (add_status)
+	      {
+	      case MAYBE_ADD_COMPLETION_OK:
+		VEC_safe_push (char_ptr, matchlist, match);
+		break;
+	      case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+		VEC_safe_push (char_ptr, matchlist, match);
+		return matchlist;
+	      case MAYBE_ADD_COMPLETION_MAX_REACHED:
+		xfree (match);
+		return matchlist;
+	      case MAYBE_ADD_COMPLETION_DUPLICATE:
+		xfree (match);
+		break;
+	      }
 	  }
       /* If we saw no matching deprecated commands in the first pass,
 	 just bail out.  */

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

* [PATCH 09/18] Implement completion limiting for interpreter_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
  2015-04-13 19:23 ` [PATCH 14/18] Implement completion limiting in add_struct_fields Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 06/18] Implement completion limiting for condition_completer Keith Seitz
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This is another simple patch which converts interpreter_completer
to use maybe_add_completion and adds some tests to make sure
that everything is working properly.

gdb/ChangeLog

	* interps.c (interpreter_completer): Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/completion.exp: Test completion limiting on
	interpreter name with "interpreter-exec".
---
 gdb/interps.c                         |   19 ++++++++++++++++++-
 gdb/testsuite/gdb.base/completion.exp |   16 ++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/gdb/interps.c b/gdb/interps.c
index ac1b512..085cac5 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -451,6 +451,7 @@ interpreter_completer (struct completer_data *cdata,
       if (strncmp (interp->name, text, textlen) == 0)
 	{
 	  char *match;
+	  enum maybe_add_completion_enum add_status;
 
 	  match = (char *) xmalloc (strlen (word) + strlen (interp->name) + 1);
 	  if (word == text)
@@ -467,7 +468,23 @@ interpreter_completer (struct completer_data *cdata,
 	      match[text - word] = '\0';
 	      strcat (match, interp->name);
 	    }
-	  VEC_safe_push (char_ptr, matches, match);
+
+	  add_status = maybe_add_completion (cdata, match);
+	  switch (add_status)
+	    {
+	    case MAYBE_ADD_COMPLETION_OK:
+	      VEC_safe_push (char_ptr, matches, match);
+	      break;
+	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	      VEC_safe_push (char_ptr, matches, match);
+	      return matches;
+	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	      xfree (match);
+	      return matches;
+	    case MAYBE_ADD_COMPLETION_DUPLICATE:
+	      xfree (match);
+	      break;
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 90cdb36..d07ca86 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -1021,3 +1021,19 @@ if {$num_signals > $max_completions} {
     append msg "limiting in signal_handler ($num_signals)"
     untested $msg
 }
+
+# Test interpreter_completer.  There are only four completions
+# available for this, so temporarily set max-completions to 3.
+with_test_prefix "interpreter_completer" {
+    set old_max $max_completions
+    set max_completions 3
+    gdb_test_no_output "set max-completions $max_completions"
+}
+
+test_completion_limit "interpreter-exec m" \
+    "interpreter-exec mi\[1-3\]?" $max_completions
+
+with_test_prefix "interpreter_completer reset" {
+    set max_completions $old_max
+    gdb_test_no_output "set max-completions $max_completions"
+}

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

* [PATCH 11/18] Implement completion limiting for reg_or_group_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (10 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 08/18] Implement completion limiting for signal_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 18/18] Remove the vector return result from the completion API Keith Seitz
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts reg_or_group_completer to use maybe_add_completion
and adds tests for this new behavior.

gdb/ChangeLog

	* completer.c (reg_or_group_completer): Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/completion.exp: Add tests for limit completing
	"info reigsters".
---
 gdb/completer.c                       |   46 ++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.base/completion.exp |    4 +++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 3022d7e..b112c2c 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1048,6 +1048,7 @@ reg_or_group_completer (struct completer_data *cdata,
   struct reggroup *group;
   const char *name;
   int i;
+  enum maybe_add_completion_enum add_status;
 
   if (!target_has_registers)
     return result;
@@ -1059,16 +1060,53 @@ reg_or_group_completer (struct completer_data *cdata,
        i++)
     {
       if (*name != '\0' && strncmp (word, name, len) == 0)
-	VEC_safe_push (char_ptr, result, xstrdup (name));
+	{
+	  char *match = xstrdup (name);
+
+	  add_status = maybe_add_completion (cdata, match);
+	  switch (add_status)
+	    {
+	    case MAYBE_ADD_COMPLETION_OK:
+	      VEC_safe_push (char_ptr, result, match);
+	      break;
+	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	      VEC_safe_push (char_ptr, result, match);
+	      return result;
+	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	      xfree (match);
+	      return result;
+	    case MAYBE_ADD_COMPLETION_DUPLICATE:
+	      xfree (match);
+	      break;
+	    }
+	}
     }
 
   for (group = reggroup_next (gdbarch, NULL);
        group != NULL;
        group = reggroup_next (gdbarch, group))
     {
-      name = reggroup_name (group);
-      if (strncmp (word, name, len) == 0)
-	VEC_safe_push (char_ptr, result, xstrdup (name));
+      char *match = xstrdup (reggroup_name (group));
+
+      if (strncmp (word, match, len) == 0)
+	{
+	  add_status = maybe_add_completion (cdata, match);
+	  switch (add_status)
+	    {
+	    case MAYBE_ADD_COMPLETION_OK:
+	      VEC_safe_push (char_ptr, result, match);
+	      break;
+	    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	      VEC_safe_push (char_ptr, result, match);
+	      return result;
+	    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	      xfree (match);
+	      return result;
+	    case MAYBE_ADD_COMPLETION_DUPLICATE:
+	      xfree (match);
+	      break;
+	    }
+	}
     }
 
   return result;
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index d07ca86..4a35cd1 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -1037,3 +1037,7 @@ with_test_prefix "interpreter_completer reset" {
     set max_completions $old_max
     gdb_test_no_output "set max-completions $max_completions"
 }
+
+# Test reg_or_group_completer.
+test_completion_limit "info registers " \
+    "info registers \[a-zA-Z0-9\]+" $max_completions

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

* [PATCH 00/18] Implement full completer limiting
@ 2015-04-13 19:23 Keith Seitz
  2015-04-13 19:23 ` [PATCH 14/18] Implement completion limiting in add_struct_fields Keith Seitz
                   ` (18 more replies)
  0 siblings, 19 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This series of patches is essentially a rewrite of the completion API
to enable better/more consistent completion-limiting.

Currently completer functions are not required to implement completion-
limiting.  These functions will compute all possible completions and then
rely on complete_line to limit the result.

The main goal of this patchset is to require completer functions to
implement proper completion-limiting using maybe_add_completion.  This
actually cleans up the completer API significantly and fixes at least one
serious bug (an assertion failure, gdb/17960).

The new API requires all completions to be added to the completion
list using maybe_add_completion:

void
my_completer_function (struct completer_data *cdata,
                       struct cmd_list_element *cmd,
                       const char *text, const char *prefix)
{
    while (/* there are more completions to look for */)
    {
      char *match = xstrdup (a_completion_match);
      enum maybe_add_completion_enum add_status;

      add_status = maybe_add_completion (cdata, match);
      switch (add_status)
      {
        case MAYBE_ADD_COMPLETION_OK:
          /* Completion was added -- keep looking for more.  */
          break;
        case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
          /* Completion was added, but now at maximum permitted completions.
             Stop looking for more matches.  */
          return;
        case MAYBE_ADD_COMPLETION_MAX_REACHED:
          /* Completion was not added;  maximum permitted completions
             already reached.  Stop looking for more matches.  */
          xfree (match);
          return;
        case MAYBE_ADD_COMPLETION_DUPLICATE:
          /* This completion is already in the list.  Keep looking for
             more matches.  */
          xfree (match);
          break;
      }
    }
}

Each patch of the set has been tested regression-free against x86_64
linux, native and native-gdbserver.


---

Keith Seitz (18):
      Add struct completer_data to the completion API.
      Remove completion_tracker_t from the public completion API.
      Implement completion limiting for complete_on_cmdlist.
      Implement completion limiting for add_filename_to_list.
      Implement completion limiting for ada_make_symbol_completion_list.
      Implement completion limiting for condition_completer.
      Implement completion limiting for filename_completer.
      Implement completion limiting for signal_completer.
      Implement completion limiting for interpreter_completer.
      Implement completion limiting for cmdpy_completer.
      Implement completion limiting for reg_or_group_completer.
      Implement completion limiting for sim_command_completer.
      Implement completion limiting for complete_on_enum.
      Implement completion limiting in add_struct_fields.
      Implement completion limiting for scmcmd_add_completion.
      Make the completion API completely opaque.
      Use the hashtable to accumulate completion results.
      Remove the vector return result from the completion API.


 gdb/ada-lang.c                             |  102 +++++-
 gdb/break-catch-syscall.c                  |   11 -
 gdb/breakpoint.c                           |   36 ++
 gdb/cli/cli-cmds.c                         |    4 
 gdb/cli/cli-decode.c                       |   66 +++-
 gdb/command.h                              |   17 +
 gdb/completer.c                            |  492 ++++++++++++++++++----------
 gdb/completer.h                            |   73 ++--
 gdb/corefile.c                             |    7 
 gdb/cp-abi.c                               |    7 
 gdb/f-lang.c                               |    8 
 gdb/guile/scm-cmd.c                        |   43 +-
 gdb/infrun.c                               |   15 -
 gdb/interps.c                              |   25 +
 gdb/language.h                             |    8 
 gdb/python/py-cmd.c                        |   42 ++
 gdb/remote-sim.c                           |   31 ++
 gdb/symtab.c                               |  254 +++++++-------
 gdb/symtab.h                               |   47 ++-
 gdb/testsuite/gdb.ada/complete.exp         |  144 ++++++++
 gdb/testsuite/gdb.ada/complete/foo.adb     |    4 
 gdb/testsuite/gdb.ada/complete/pck.ads     |   12 +
 gdb/testsuite/gdb.base/break.c             |   14 +
 gdb/testsuite/gdb.base/break.exp           |    4 
 gdb/testsuite/gdb.base/completion.exp      |  206 +++++++++++-
 gdb/testsuite/gdb.base/condbreak.exp       |   70 ++++
 gdb/testsuite/gdb.base/filesym.c           |    4 
 gdb/testsuite/gdb.base/filesym.exp         |   84 +++++
 gdb/testsuite/gdb.base/filesym2.c          |   24 +
 gdb/testsuite/gdb.base/filesym3.c          |   24 +
 gdb/testsuite/gdb.base/filesym4.c          |   24 +
 gdb/testsuite/gdb.base/filesym5.c          |   22 +
 gdb/testsuite/gdb.cp/cpcompletion.exp      |   55 +++
 gdb/testsuite/gdb.cp/pr9594.cc             |   28 ++
 gdb/testsuite/gdb.guile/scm-cmd.exp        |   24 +
 gdb/testsuite/gdb.python/py-completion.exp |   45 +++
 gdb/value.c                                |   24 +
 gdb/value.h                                |    3 
 38 files changed, 1582 insertions(+), 521 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/filesym2.c
 create mode 100644 gdb/testsuite/gdb.base/filesym3.c
 create mode 100644 gdb/testsuite/gdb.base/filesym4.c
 create mode 100644 gdb/testsuite/gdb.base/filesym5.c

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

* [PATCH 15/18] Implement completion limiting for scmcmd_add_completion.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (15 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 07/18] Implement completion limiting for filename_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 05/18] Implement completion limiting for ada_make_symbol_completion_list Keith Seitz
  2015-04-20  3:21 ` [PATCH 00/18] Implement full completer limiting Doug Evans
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts scmcmd_add_completion to use maybe_add_completion and
adds some tests for this new behavior.

gdb/ChangeLog

	* guile/scm-cmd.c (cmdscm_add_completion): Add completer_data
	argument.  All callers updated.
	Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.guile/scm-cmd.exp: Add completion limiting tests.
---
 gdb/guile/scm-cmd.c                 |   21 +++++++++++++++++----
 gdb/testsuite/gdb.guile/scm-cmd.exp |   24 ++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 2c57d17..1680c27 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -348,10 +348,12 @@ cmdscm_bad_completion_result (const char *msg, SCM completion)
    The result is a boolean indicating success.  */
 
 static int
-cmdscm_add_completion (SCM completion, VEC (char_ptr) **result)
+cmdscm_add_completion (SCM completion, struct completer_data *cdata,
+		       VEC (char_ptr) **result)
 {
   char *item;
   SCM except_scm;
+  enum maybe_add_completion_enum add_status;
 
   if (!scm_is_string (completion))
     {
@@ -370,7 +372,18 @@ cmdscm_add_completion (SCM completion, VEC (char_ptr) **result)
       return 0;
     }
 
-  VEC_safe_push (char_ptr, *result, item);
+  add_status = maybe_add_completion (cdata, item);
+  switch (add_status)
+    {
+    case MAYBE_ADD_COMPLETION_OK:
+    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+      VEC_safe_push (char_ptr, *result, item);
+      break;
+    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+    case MAYBE_ADD_COMPLETION_DUPLICATE:
+      xfree (item);
+      break;
+    }
 
   return 1;
 }
@@ -418,7 +431,7 @@ cmdscm_completer (struct completer_data *cdata,
 	{
 	  SCM next = scm_car (list);
 
-	  if (!cmdscm_add_completion (next, &result))
+	  if (!cmdscm_add_completion (next, cdata, &result))
 	    {
 	      VEC_free (char_ptr, result);
 	      goto done;
@@ -442,7 +455,7 @@ cmdscm_completer (struct completer_data *cdata,
 	      goto done;
 	    }
 
-	  if (!cmdscm_add_completion (next, &result))
+	  if (!cmdscm_add_completion (next, cdata, &result))
 	    {
 	      VEC_free (char_ptr, result);
 	      goto done;
diff --git a/gdb/testsuite/gdb.guile/scm-cmd.exp b/gdb/testsuite/gdb.guile/scm-cmd.exp
index 53c0fdf..ac721c3 100644
--- a/gdb/testsuite/gdb.guile/scm-cmd.exp
+++ b/gdb/testsuite/gdb.guile/scm-cmd.exp
@@ -196,6 +196,30 @@ gdb_test "test-scheme-error-cmd ugh" \
     "Error occurred in Scheme-implemented GDB command." \
     "call scheme-error command"
 
+# Test completion limiting.
+set max_completions 2
+gdb_test_no_output "set max-completions $max_completions"
+set end "\\\*\\\*\\\* List may be truncated, "
+append end "max-completions reached\\\. \\\*\\\*\\\*"
+set test "limit complete completer-as-function 42\."
+gdb_test_multiple "complete completer-as-function 42\." $test {
+    "complete completer-as-function 42\\\." { exp_continue }
+    -re "completer-as-function 42\\\.\[1-3\]\r\n" {
+	incr seen
+	exp_continue
+    }
+    -re ".*$end\r\n$gdb_prompt $" {
+	if {$seen == $max_completions} {
+	    pass $test
+	} else {
+	    fail "$test ($seen/$max_completions)"
+	}
+    }
+    -re ".*$gdb_prompt $" {
+	fail "$test (unlimited)"
+    }
+}
+
 # If there is a problem with object management, this can often trigger it.
 # It is useful to do this last, after we've created a bunch of command objects.
 

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

* [PATCH 07/18] Implement completion limiting for filename_completer.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (14 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 02/18] Remove completion_tracker_t from the public completion API Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 15/18] Implement completion limiting for scmcmd_add_completion Keith Seitz
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts filename_completer to using maybe_add_completion
and adds some tests to exercise this new behavior.

gdb/ChangeLog

	* completer.c (filename_completer): Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/completion.exp: Put completion truncation message
	into a variable for use by other tests.  Update existing tests.
	(test_completion_limit): New procedure.
	Add tests for completion limiting on file names.
---
 gdb/completer.c                       |   18 +++++++++
 gdb/testsuite/gdb.base/completion.exp |   65 +++++++++++++++++++++++++++++++--
 2 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 6708bb1..291ba73 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -136,6 +136,7 @@ filename_completer (struct completer_data *cdata,
   while (1)
     {
       char *p, *q;
+      enum maybe_add_completion_enum add_status;
 
       p = rl_filename_completion_function (text, subsequent_name);
       if (p == NULL)
@@ -172,7 +173,22 @@ filename_completer (struct completer_data *cdata,
 	  strcat (q, p);
 	  xfree (p);
 	}
-      VEC_safe_push (char_ptr, return_val, q);
+      add_status = maybe_add_completion (cdata, q);
+      switch (add_status)
+	{
+	case MAYBE_ADD_COMPLETION_OK:
+	  VEC_safe_push (char_ptr, return_val, q);
+	  break;
+	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	  VEC_safe_push (char_ptr, return_val, q);
+	  return return_val;
+	case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	  xfree (q);
+	  return return_val;
+	case MAYBE_ADD_COMPLETION_DUPLICATE:
+	  xfree (q);
+	  break;
+	}
     }
 #if 0
   /* There is no way to do this just long enough to affect quote
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 5afd851..78ba216 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -858,6 +858,10 @@ gdb_test "complete break need" "need_malloc"
 # Completion limiting.
 #
 
+# The terminal at the end of the complete command
+set end "\\\*\\\*\\\* List may be truncated, "
+append end "max-completions reached\\\. \\\*\\\*\\\*"
+
 gdb_test_no_output "set max-completions 5"
 
 set test "command-name completion limiting using tab character"
@@ -866,7 +870,7 @@ gdb_test_multiple "" "$test" {
     -re "^p\\\x07$" {
 	send_gdb "\t"
 	gdb_test_multiple "" "$test" {
-	    -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p$" {
+	    -re "$end\r\n$gdb_prompt p$" {
 		# Complete the command and ignore the output to resync
 		# gdb for the next test.
 		send_gdb "\n"
@@ -898,7 +902,8 @@ gdb_test_multiple "" "$test" {
     }
 }
 
-gdb_test_no_output "set max-completions 3"
+set max_completions 3
+gdb_test_no_output "set max-completions $max_completions"
 
 set test "symbol-name completion limiting using tab character"
 send_gdb "p marker\t"
@@ -906,7 +911,7 @@ gdb_test_multiple "" "$test" {
     -re "^p marker\\\x07$" {
 	send_gdb "\t"
 	gdb_test_multiple "" "$test" {
-	    -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p marker$" {
+	    -re "$end\r\n$gdb_prompt p marker$" {
 		# Complete the command and ignore the output to resync
 		# gdb for the next test.
 		send_gdb "\n"
@@ -933,7 +938,59 @@ gdb_test_multiple "" "$test" {
 set test "symbol-name completion limiting using complete command"
 send_gdb "complete p mark\n"
 gdb_test_multiple "" "$test" {
-    -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" {
+    -re "$end\r\n$gdb_prompt $" {
 	pass "$test"
     }
 }
+
+# A convenience function for testing completion limiting.
+# CMD is a GDB command to to run with "complete".
+# PATTERN is a regexp pattern matching the expected output
+#   of completion items "seen" in the output.
+# NUM is the number of maximum completions expected.
+#
+# The test will use the test name "limit complete CMD"
+# and will only count the number of completion items matching
+# PATTERN.  No assumptions are made on the order of the items
+# seen in GDB's output.
+#
+# If NUM items are seen before the truncation message, the test
+# passes, otherwise it fails.  The test can also fail if no
+# truncation message is seen at all, in which case the test
+# failure message will say "(unlimited)".
+
+proc test_completion_limit {cmd pattern num} {
+    global gdb_prompt
+
+    # The terminal at the end of the complete command
+    set end "\\\*\\\*\\\* List may be truncated, "
+    append end "max-completions reached\\\. \\\*\\\*\\\*"
+
+    set cmdr [string_to_regexp $cmd]
+    set seen 0
+    gdb_test_multiple "complete $cmd" "limit complete $cmd" {
+	"complete $cmdr" { exp_continue }
+	-re "$pattern\r\n" {
+	    incr seen
+	    exp_continue
+	}
+	-re ".*$end\r\n$gdb_prompt $" {
+	    if {$seen == $num} {
+		pass "limit complete $cmd"
+	    } else {
+		fail "limit complete $cmd ($seen/$num)"
+	    }
+	}
+	-re ".*$gdb_prompt $" {
+	    fail "limit complete $cmd (unlimited)"
+	}
+    }
+}
+
+test_completion_limit "file ./gdb.base/jit-s" \
+    "file \\\./gdb\\\.base/jit-s(imple|olib|o)(\\\.c|\\\.exp)?" \
+    $max_completions
+
+# same as above but completing on directory names.
+test_completion_limit "file ./gdb.a" "file \\\./gdb\\\.a(da|rch|sm)" \
+    $max_completions

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

* [PATCH 16/18] Make the completion API completely opaque.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (4 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 12/18] Implement completion limiting for sim_command_completer Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 01/18] Add struct completer_data to the completion API Keith Seitz
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

Now that the completion API is passing around a structure holding
its internal data, there is no need to expose any of this data and
other settings to other modules.

This patch removes global access to max_completions and
introduces a new API function,  get_maximum_completions (void), which
other modules may use to query the current completion maximum.

The API will be expanded as necessary in subsequent patches.

gdb/ChangeLog

	* cli/cli-cmds.c (complete_command): Use get_maximum_completions
	instead of accessing the global max_completions.
	* completer.c (max_completions): Move definition earlier and
	make static.
	(get_maximum_completions): New function.
	(throw_max_completions_reached_error): Add comment.
	* completer.h (max_completions): Remove declaration.
	(get_maximum_completions): New declaration.
---
 gdb/cli/cli-cmds.c |    4 ++--
 gdb/completer.c    |   22 +++++++++++++++++-----
 gdb/completer.h    |    8 ++++----
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 2ec2dd3..a73a655 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -248,7 +248,7 @@ complete_command (char *arg, int from_tty)
 
   dont_repeat ();
 
-  if (max_completions == 0)
+  if (get_maximum_completions () == 0)
     {
       /* Only print this for non-mi frontends.  An MI frontend may not
 	 be able to handle this.  */
@@ -307,7 +307,7 @@ complete_command (char *arg, int from_tty)
       xfree (prev);
       VEC_free (char_ptr, completions);
 
-      if (size == max_completions)
+      if (size == get_maximum_completions ())
 	{
 	  /* ARG_PREFIX and POINT are included in the output so that emacs
 	     will include the message in the output.  */
diff --git a/gdb/completer.c b/gdb/completer.c
index 45170e3..5d06784 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -42,6 +42,13 @@
 
 #include "completer.h"
 
+/* Maximum number of candidates to consider before the completer
+   bails by throwing MAX_COMPLETIONS_REACHED_ERROR.  Negative values
+   disable limiting.  */
+
+#define DEFAULT_MAX_COMPLETIONS 200
+static int max_completions = DEFAULT_MAX_COMPLETIONS;
+
 /* Prototypes for local functions.  */
 static
 char *line_completion_function (const char *text, int matches, 
@@ -855,11 +862,6 @@ complete_line_internal (struct completer_data *cdata,
   return list;
 }
 
-/* See completer.h.  */
-
-#define DEFAULT_MAX_COMPLETIONS 200
-int max_completions = DEFAULT_MAX_COMPLETIONS;
-
 /* Allocate a new completer data structure.  */
 
 static struct completer_data *
@@ -887,6 +889,14 @@ free_completer_data (void *p)
 
 /* See completer.h.  */
 
+int
+get_maximum_completions (void)
+{
+  return max_completions;
+}
+
+/* See completer.h.  */
+
 enum maybe_add_completion_enum
 maybe_add_completion (struct completer_data *cdata, char *name)
 {
@@ -912,6 +922,8 @@ maybe_add_completion (struct completer_data *cdata, char *name)
 	  : MAYBE_ADD_COMPLETION_OK_MAX_REACHED);
 }
 
+/* See completer.h.  */
+
 void
 throw_max_completions_reached_error (void)
 {
diff --git a/gdb/completer.h b/gdb/completer.h
index 003c66c..67fdcad 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -122,11 +122,11 @@ extern const char *skip_quoted_chars (const char *, const char *,
 
 extern const char *skip_quoted (const char *);
 
-/* Maximum number of candidates to consider before the completer
-   bails by throwing MAX_COMPLETIONS_REACHED_ERROR.  Negative values
-   disable limiting.  */
+/* Get the maximum number of completions that are permitted before the
+   completer throws a MAX_COMPLETIONS_REACHED_ERROR.  Negative values
+   mean completion limiting is entirely disabled.  */
 
-extern int max_completions;
+extern int get_maximum_completions (void);
 
 /* Return values for maybe_add_completion.  */
 

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

* [PATCH 05/18] Implement completion limiting for ada_make_symbol_completion_list.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (16 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 15/18] Implement completion limiting for scmcmd_add_completion Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-20  3:21 ` [PATCH 00/18] Implement full completer limiting Doug Evans
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This patch converts the one Ada completion(-related) function,
symbol_completion_add, to use maybe_add_completion, and tests have
been added to exercise this newly implemented behavior.

gdb/ChangeLog

	* ada-lang.c (symbol_completion_add): Return
	maybe_add_completion_enum instead of void.
	Use maybe_add_completion and return the status of this function.
	(ada_make_symbol_completion_list): If symbol_completion_add
	returns that maximum completions have been reached, stop
	looking for completions and return the list.

gdb/testsuite/ChangeLog

	* gdb.ada/complete.exp (limit_multi_line): New procedure.
	Update existing tests for source changes.
	Add additional tests for new types.
	Add tests for completion limiting.
	* gdb.ada/complete/foo.adb (Repeat_Variable_1, Repeat_Variable_2,
	Repeat_Variable_3, Repeat_Variable_4): Define.
	* gdb.ada/complete/pck.ads (Repeat_Variable_1, Repeat_Variable_2)
	(Repeat_Variable_3, Repeat_Variable_4): Declare.
	(Repeated_1, Repeated_2, Repeated_3, Repeated_4): Define.
---
 gdb/ada-lang.c                         |   93 +++++++++++++++++----
 gdb/testsuite/gdb.ada/complete.exp     |  144 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/complete/foo.adb |    4 +
 gdb/testsuite/gdb.ada/complete/pck.ads |   12 +++
 4 files changed, 236 insertions(+), 17 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 5ca4b5b..4b9b3d3 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -6122,8 +6122,9 @@ symbol_completion_match (const char *sym_name,
    encoded formed (in which case the completion should also be
    encoded).  */
 
-static void
+static enum maybe_add_completion_enum
 symbol_completion_add (VEC(char_ptr) **sv,
+		       struct completer_data *cdata,
                        const char *sym_name,
                        const char *text, int text_len,
                        const char *orig_text, const char *word,
@@ -6132,9 +6133,10 @@ symbol_completion_add (VEC(char_ptr) **sv,
   const char *match = symbol_completion_match (sym_name, text, text_len,
                                                wild_match_p, encoded_p);
   char *completion;
+  enum maybe_add_completion_enum add_status;
 
   if (match == NULL)
-    return;
+    return MAYBE_ADD_COMPLETION_OK;
 
   /* We found a match, so add the appropriate completion to the given
      string vector.  */
@@ -6159,7 +6161,19 @@ symbol_completion_add (VEC(char_ptr) **sv,
       strcat (completion, match);
     }
 
-  VEC_safe_push (char_ptr, *sv, completion);
+  add_status = maybe_add_completion (cdata, completion);
+  switch (add_status)
+    {
+    case MAYBE_ADD_COMPLETION_OK:
+    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+      VEC_safe_push (char_ptr, *sv, completion);
+      break;
+    case MAYBE_ADD_COMPLETION_MAX_REACHED:
+    case MAYBE_ADD_COMPLETION_DUPLICATE:
+      xfree (completion);
+      break;
+    }
+  return add_status;
 }
 
 /* An object of this type is passed as the user_data argument to the
@@ -6207,6 +6221,7 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
   int i;
   struct block_iterator iter;
   struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+  enum maybe_add_completion_enum add_status;
 
   gdb_assert (code == TYPE_CODE_UNDEF);
 
@@ -6257,9 +6272,21 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
   ALL_MSYMBOLS (objfile, msymbol)
   {
     QUIT;
-    symbol_completion_add (&completions, MSYMBOL_LINKAGE_NAME (msymbol),
-			   text, text_len, text0, word, wild_match_p,
-			   encoded_p);
+    add_status = symbol_completion_add (&completions, cdata,
+					MSYMBOL_LINKAGE_NAME (msymbol),
+					text, text_len, text0, word,
+					wild_match_p, encoded_p);
+    switch (add_status)
+      {
+      case MAYBE_ADD_COMPLETION_OK:
+	break;
+      case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+      case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	do_cleanups (old_chain);
+	return completions;
+      case MAYBE_ADD_COMPLETION_DUPLICATE:
+	break;
+      }
   }
 
   /* Search upwards from currently selected frame (so that we can
@@ -6272,9 +6299,21 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
 
       ALL_BLOCK_SYMBOLS (b, iter, sym)
       {
-        symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
-                               text, text_len, text0, word,
-                               wild_match_p, encoded_p);
+        add_status = symbol_completion_add (&completions, cdata,
+					    SYMBOL_LINKAGE_NAME (sym),
+					    text, text_len, text0, word,
+					    wild_match_p, encoded_p);
+	switch (add_status)
+	  {
+	  case MAYBE_ADD_COMPLETION_OK:
+	    break;
+	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	    do_cleanups (old_chain);
+	    return completions;
+	  case MAYBE_ADD_COMPLETION_DUPLICATE:
+	    break;
+	  }
       }
     }
 
@@ -6287,9 +6326,21 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
     b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (s), GLOBAL_BLOCK);
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
-                             text, text_len, text0, word,
-                             wild_match_p, encoded_p);
+      add_status = symbol_completion_add (&completions, cdata,
+					  SYMBOL_LINKAGE_NAME (sym),
+					  text, text_len, text0, word,
+					  wild_match_p, encoded_p);
+      switch (add_status)
+	{
+	case MAYBE_ADD_COMPLETION_OK:
+	  break;
+	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	  do_cleanups (old_chain);
+	  return completions;
+	case MAYBE_ADD_COMPLETION_DUPLICATE:
+	  break;
+	}
     }
   }
 
@@ -6302,9 +6353,21 @@ ada_make_symbol_completion_list (struct completer_data *cdata,
       continue;
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
-                             text, text_len, text0, word,
-                             wild_match_p, encoded_p);
+      add_status = symbol_completion_add (&completions, cdata,
+					 SYMBOL_LINKAGE_NAME (sym),
+					 text, text_len, text0, word,
+					 wild_match_p, encoded_p);
+      switch (add_status)
+	{
+	case MAYBE_ADD_COMPLETION_OK:
+	  break;
+	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	  do_cleanups (old_chain);
+	  return completions;
+	case MAYBE_ADD_COMPLETION_DUPLICATE:
+	  break;
+	}
     }
   }
 
diff --git a/gdb/testsuite/gdb.ada/complete.exp b/gdb/testsuite/gdb.ada/complete.exp
index 9919bdf..b4efc68 100644
--- a/gdb/testsuite/gdb.ada/complete.exp
+++ b/gdb/testsuite/gdb.ada/complete.exp
@@ -44,6 +44,29 @@ proc test_gdb_no_completion { expr } {
     gdb_test_no_output "complete p $expr"
 }
 
+# A convenience function that joins all the arguments together,
+# with a regexp that matches zero-or-more end of lines in between
+# each argument.  This function is ideal to write the expected output
+# of a GDB command that generates more than a couple of lines, as
+# this allows us to write each line as a separate string, which is
+# easier to read by a human being.
+
+proc multi_line { args } {
+    return [join $args "\[\r\n\]*"]
+}
+
+# Like multi_line above, but limiting the return result to MAX
+# elements and adding the completer's truncation message.
+
+proc limit_multi_line { max args } {
+    set result [join [lrange $args 0 [expr {$max - 1}]] "\[\r\n\]*"]
+    if {$max <= [llength $args]} {
+	append result ".*\\\*\\\*\\\* List may be truncated, "
+	append result "max-completions reached\\\. \\\*\\\*\\\*"
+    }
+    return $result
+}
+
 # Try a global variable, only one match should be found:
 
 test_gdb_complete "my_glob" \
@@ -145,7 +168,11 @@ test_gdb_complete "pck" \
                               "p pck.local_identical_one" \
                               "p pck.local_identical_two" \
                               "p pck.my_global_variable" \
-                              "p pck.proc" ]
+                              "p pck.proc" \
+                              "p pck.repeat_variable_1" \
+                              "p pck.repeat_variable_2" \
+                              "p pck.repeat_variable_3" \
+                              "p pck.repeat_variable_4" ]
 
 # Complete on the name of a package followed by a dot:
 test_gdb_complete "pck." \
@@ -156,12 +183,125 @@ test_gdb_complete "pck." \
                               "p pck.local_identical_one" \
                               "p pck.local_identical_two" \
                               "p pck.my_global_variable" \
-                              "p pck.proc" ]
+                              "p pck.proc" \
+                              "p pck.repeat_variable_1" \
+                              "p pck.repeat_variable_2" \
+                              "p pck.repeat_variable_3" \
+                              "p pck.repeat_variable_4" ]
+
+# Complete on a repeated global name:
+test_gdb_complete "repeat_" \
+                 [multi_line "p repeat_variable_1" \
+                             "p repeat_variable_2" \
+                             "p repeat_variable_3" \
+                             "p repeat_variable_4" ]
+
+# Complete a mangled symbol name, but using the '<...>' notation:
+test_gdb_complete "<pck__repeat_" \
+                  [multi_line "p <pck__repeat_variable_1>" \
+                              "p <pck__repeat_variable_2>" \
+                              "p <pck__repeat_variable_3>" \
+                              "p <pck__repeat_variable_4>" ]
+
+# Complete on repeated mangled name, using '<...>' notation:
+test_gdb_complete "<Repeated_" \
+                  [multi_line "p <Repeated_1>" \
+                              "p <Repeated_2>" \
+                              "p <Repeated_3>" \
+                              "p <Repeated_4>" ]
 
 # Complete a mangled symbol name, but using the '<...>' notation.
 test_gdb_complete "<pck__my" \
                   "p <pck__my_global_variable>"
 
+# Test completion limiting.
+set max_completions 2
+gdb_test_no_output "set max-completions $max_completions"
+with_test_prefix "limit completion" {
+    # Two matches, from the global scope:
+    test_gdb_complete "local_ident" \
+	[limit_multi_line $max_completions \
+	     "p local_identical_one" \
+	     "p local_identical_two" ]
+
+    # Two matches, from the global scope, but using fully qualified names:
+    test_gdb_complete "pck.local_ident" \
+	[limit_multi_line $max_completions "p pck.local_identical_one" \
+	     "p pck.local_identical_two" ]
+
+    # Two matches, from the global scope, but using mangled fully qualified
+    # names:
+    test_gdb_complete "pck__local_ident" \
+	[limit_multi_line $max_completions \
+	     "p pck__local_identical_one" \
+	     "p pck__local_identical_two" ]
+
+    # Two matches, one from the global scope, the other from the local
+    # scope:
+    test_gdb_complete "external_ident" \
+	[limit_multi_line $max_completions \
+	     "p external_identical_one" \
+	     "p external_identical_two" ]
+
+    # Complete on the name of package.
+    test_gdb_complete "pck" \
+	[limit_multi_line $max_completions \
+	     "(p pck\\.ad\[sb\])?" \
+	     "(p pck\\.ad\[sb\])?" \
+	     "p pck.external_identical_one" \
+	     "p pck.inner.inside_variable" \
+	     "p pck.local_identical_one" \
+	     "p pck.local_identical_two" \
+	     "p pck.my_global_variable" \
+	     "p pck.proc" \
+	     "p pck.repeat_variable_1" \
+	     "p pck.repeat_variable_2" \
+	     "p pck.repeat_variable_3" \
+	     "p pck.repeat_variable_4" ]
+
+    # Complete on the name of a package followed by a dot:
+    test_gdb_complete "pck." \
+	[limit_multi_line $max_completions \
+	     "(p pck\\.ad\[sb\])?" \
+	     "(p pck\\.ad\[sb\])?" \
+	     "p pck.external_identical_one" \
+	     "p pck.inner.inside_variable" \
+	     "p pck.local_identical_one" \
+	     "p pck.local_identical_two" \
+	     "p pck.my_global_variable" \
+	     "p pck.proc" \
+	     "p pck.repeat_variable_1" \
+	     "p pck.repeat_variable_2" \
+	     "p pck.repeat_variable_3" \
+	     "p pck.repeat_variable_4"]
+
+    # Complete on a repeated global name:
+    test_gdb_complete "repeat_" \
+	[limit_multi_line $max_completions \
+	     "p repeat_variable_1" \
+	     "p repeat_variable_2" \
+	     "p repeat_variable_3" \
+	     "p repeat_variable_4" ]
+    # Complete a mangled symbol name, but using the '<...>' notation:
+    test_gdb_complete "<pck__repeat_" \
+	[limit_multi_line $max_completions \
+	     "p <pck__repeat_variable_1>" \
+	     "p <pck__repeat_variable_2>" \
+	     "p <pck__repeat_variable_3>" \
+	     "p <pck__repeat_variable_4>" ]
+
+    # Complete on repeated mangled name, using '<...>' notation:
+    test_gdb_complete "<Repeated_" \
+	[limit_multi_line $max_completions \
+	     "p <Repeated_1>" \
+	     "p <Repeated_2>" \
+	     "p <Repeated_3>" \
+	     "p <Repeated_4>" ]
+}
+
+# Reset completion-limiting to its default.
+gdb_test_no_output "set max-completions 200"
+
 # Very simple completion, but using the interactive form, this time.
 # The verification we are trying to make involves the event loop,
 # and using the "complete" command is not sufficient to reproduce
diff --git a/gdb/testsuite/gdb.ada/complete/foo.adb b/gdb/testsuite/gdb.ada/complete/foo.adb
index 58d0ee3..ad6b5ec 100644
--- a/gdb/testsuite/gdb.ada/complete/foo.adb
+++ b/gdb/testsuite/gdb.ada/complete/foo.adb
@@ -20,6 +20,10 @@ procedure Foo is
    External_Identical_Two : Integer := 74;
 begin
    My_Global_Variable := Some_Local_Variable + 1; -- START
+   Repeat_Variable_1 := My_Global_Variable + 1;
+   Repeat_Variable_2 := Repeat_Variable_1 + 1;
+   Repeat_Variable_3 := Repeat_Variable_2 + 1;
+   Repeat_Variable_4 := Repeat_Variable_3 + 1;
    Proc (External_Identical_Two);
 end Foo;
 
diff --git a/gdb/testsuite/gdb.ada/complete/pck.ads b/gdb/testsuite/gdb.ada/complete/pck.ads
index ab2c47b..5595f7f 100644
--- a/gdb/testsuite/gdb.ada/complete/pck.ads
+++ b/gdb/testsuite/gdb.ada/complete/pck.ads
@@ -16,9 +16,21 @@
 package Pck is
 
    My_Global_Variable : Integer := 1;
+   Repeat_Variable_1 : Integer := 1;
+   Repeat_Variable_2 : Integer := 1;
+   Repeat_Variable_3 : Integer := 1;
+   Repeat_Variable_4 : Integer := 1;
 
    Exported_Capitalized : Integer := 2;
    pragma Export (C, Exported_Capitalized, "Exported_Capitalized");
+   Repeated_1 : Integer := 21;
+   pragma Export (C, Repeated_1, "Repeated_1");
+   Repeated_2 : Integer := 22;
+   pragma Export (C, Repeated_2, "Repeated_2");
+   Repeated_3 : Integer := 23;
+   pragma Export (C, Repeated_3, "Repeated_3");
+   Repeated_4 : Integer := 24;
+   pragma Export (C, Repeated_4, "Repeated_4");
 
    Local_Identical_One : Integer := 4;
    Local_Identical_Two : Integer := 8;

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

* [PATCH 13/18] Implement completion limiting for complete_on_enum.
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (7 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 03/18] Implement completion limiting for complete_on_cmdlist Keith Seitz
@ 2015-04-13 19:23 ` Keith Seitz
  2015-04-13 19:23 ` [PATCH 17/18] Use the hashtable to accumulate completion results Keith Seitz
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-04-13 19:23 UTC (permalink / raw)
  To: gdb-patches

This is another patch to push along the conversion of location_completer
toward using maybe_add_completion.  In this patch, complete_on_enum is
converted.  This function is also used by several other commands, such as
integer_unlimited_completer, handle_completer, cp_abi_completer, etc.

gdb/ChangeLog

	* cli/cli-decode.c (complete_on_enum): Use maybe_add_completion.

gdb/testsuite/ChangeLog

	* gdb.base/completion.exp: Add tests for complete_on_enum
	completion limiting using "handle signal".
---
 gdb/cli/cli-decode.c                  |   19 ++++++++++++++++++-
 gdb/testsuite/gdb.base/completion.exp |   12 ++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 21f4c45..04eb437 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1866,6 +1866,7 @@ complete_on_enum (struct completer_data *cdata,
     if (strncmp (name, text, textlen) == 0)
       {
 	char *match;
+	enum maybe_add_completion_enum add_status;
 
 	match = (char *) xmalloc (strlen (word) + strlen (name) + 1);
 	if (word == text)
@@ -1882,7 +1883,23 @@ complete_on_enum (struct completer_data *cdata,
 	    match[text - word] = '\0';
 	    strcat (match, name);
 	  }
-	VEC_safe_push (char_ptr, matchlist, match);
+
+	add_status = maybe_add_completion (cdata, match);
+	switch (add_status)
+	  {
+	  case MAYBE_ADD_COMPLETION_OK:
+	    VEC_safe_push (char_ptr, matchlist, match);
+	    break;
+	  case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	    VEC_safe_push (char_ptr, matchlist, match);
+	    return matchlist;
+	  case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	    xfree (match);
+	    return matchlist;
+	  case MAYBE_ADD_COMPLETION_DUPLICATE:
+	    xfree (match);
+	    break;
+	  }
       }
 
   return matchlist;
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 4a35cd1..516975c 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -1041,3 +1041,15 @@ with_test_prefix "interpreter_completer reset" {
 # Test reg_or_group_completer.
 test_completion_limit "info registers " \
     "info registers \[a-zA-Z0-9\]+" $max_completions
+
+# Test complete_on_enum.
+set signal_to_use ""
+set signal_list [split \
+		     [capture_command_output "complete handle signal " ""] \
+		     \r\n]
+catch {lindex [split [lindex $signal_list 0]] 2} signal_to_use
+if {$signal_to_use != ""} {
+    test_completion_limit "handle signal $signal_to_use n" \
+	"handle signal $signal_to_use n\[a-z\]+" \
+	$max_completions
+}

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

* Re: [PATCH 12/18] Implement completion limiting for sim_command_completer.
  2015-04-13 19:23 ` [PATCH 12/18] Implement completion limiting for sim_command_completer Keith Seitz
@ 2015-04-14 15:28   ` Mike Frysinger
  2015-05-05 15:03     ` Keith Seitz
  0 siblings, 1 reply; 26+ messages in thread
From: Mike Frysinger @ 2015-04-14 15:28 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On 13 Apr 2015 12:23, Keith Seitz wrote:
> This patch converts sim_command_completer to use maybe_add_completion.

i think you want to push this down into sim/common/sim-options.c ?  otherwise 
you do all of this work only to throw away a good amount of it.

> It does not add any tests, since the `sim' command is highly
> target-dependent and unimplemented for the majority of simulators.

erm, the majority of simulators now have testsuites.  but that doesn't really 
matter because you're updating the gdb part of the API, not the sim side.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 00/18] Implement full completer limiting
  2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
                   ` (17 preceding siblings ...)
  2015-04-13 19:23 ` [PATCH 05/18] Implement completion limiting for ada_make_symbol_completion_list Keith Seitz
@ 2015-04-20  3:21 ` Doug Evans
  2015-04-29 19:19   ` Keith Seitz
  18 siblings, 1 reply; 26+ messages in thread
From: Doug Evans @ 2015-04-20  3:21 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

On Mon, Apr 13, 2015 at 12:23 PM, Keith Seitz <keiths@redhat.com> wrote:
> This series of patches is essentially a rewrite of the completion API
> to enable better/more consistent completion-limiting.
>
> Currently completer functions are not required to implement completion-
> limiting.  These functions will compute all possible completions and then
> rely on complete_line to limit the result.
>
> The main goal of this patchset is to require completer functions to
> implement proper completion-limiting using maybe_add_completion.  This
> actually cleans up the completer API significantly and fixes at least one
> serious bug (an assertion failure, gdb/17960).
>
> The new API requires all completions to be added to the completion
> list using maybe_add_completion:
>
> void
> my_completer_function (struct completer_data *cdata,
>                        struct cmd_list_element *cmd,
>                        const char *text, const char *prefix)
> {
>     while (/* there are more completions to look for */)
>     {
>       char *match = xstrdup (a_completion_match);
>       enum maybe_add_completion_enum add_status;
>
>       add_status = maybe_add_completion (cdata, match);
>       switch (add_status)
>       {
>         case MAYBE_ADD_COMPLETION_OK:
>           /* Completion was added -- keep looking for more.  */
>           break;
>         case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
>           /* Completion was added, but now at maximum permitted completions.
>              Stop looking for more matches.  */
>           return;
>         case MAYBE_ADD_COMPLETION_MAX_REACHED:
>           /* Completion was not added;  maximum permitted completions
>              already reached.  Stop looking for more matches.  */
>           xfree (match);
>           return;
>         case MAYBE_ADD_COMPLETION_DUPLICATE:
>           /* This completion is already in the list.  Keep looking for
>              more matches.  */
>           xfree (match);
>           break;
>       }
>     }
> }
>
> Each patch of the set has been tested regression-free against x86_64
> linux, native and native-gdbserver.

Hi.

I've gone over the entire patch set.  A few things I like, but there's
at least one thing I'm concerned about.  Replicating the above switch
in each completer: IWBN to avoid such duplication.
We should still be able to remove the global state and fix 17960.

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

* Re: [PATCH 00/18] Implement full completer limiting
  2015-04-20  3:21 ` [PATCH 00/18] Implement full completer limiting Doug Evans
@ 2015-04-29 19:19   ` Keith Seitz
  2015-04-30  0:26     ` Doug Evans
  0 siblings, 1 reply; 26+ messages in thread
From: Keith Seitz @ 2015-04-29 19:19 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On 04/19/2015 08:21 PM, Doug Evans wrote:
> I've gone over the entire patch set.  A few things I like, but there's
> at least one thing I'm concerned about.  Replicating the above switch
> in each completer: IWBN to avoid such duplication.
> We should still be able to remove the global state and fix 17960.

The original patch series (necessarily IMO) exposed this
memory-management issue, and a global maintainer approved the change.
Surely the approving maintainer realized that this code would propagate
to other completer functions eventually, no? [I certainly hope the
maintainer did not accept that the complete_line hack would be long-lived!]

I can certainly change the series to add a function which attempts to
hide this detail. For example:

/* Returns whether the maximum number of completions has been
reached/exceeded.  */

int
add_completion (struct completer_data *cdata, const char *match)
{
  char *match = xstrdup (name);
  enum maybe_add_completion_enum add_status;

  add_status = maybe_add_completion (cdata, match);
  switch (add_status)
    {
    case MAYBE_ADD_COMPLETION_OK:
      VEC_safe_push (char_ptr, cdata->results, match);
      break;
    case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
      VEC_safe_push (char_ptr, cdata->results, match);
      return 1;
    case MAYBE_ADD_COMPLETION_MAX_REACHED:
      xfree (match);
      return 1;
    case MAYBE_ADD_COMPLETION_DUPLICATE:
      xfree (match);
      break;
    }

  return 0;
}

Given that completions (like symbol reading/searching) are extremely
time-consuming, one of my design goals was to not introduce anything
that might slow down completion, including multiple allocations and copying.

Unfortunately with this goal in mind, the use of this proposed function
(alone) can only be pushed into four of the fourteen functions dealing
with completions.

The ten remaining functions would require an additional malloc/copy or
yet another API function, e.g.,
add_completion_1/add_completionA/add_completion_Ex [:-)] to deal with
this case. All this to simply forgo checking the return result of
maybe_add_completion. I don't see the benefit.

Developers already have to deal with memory management in exception
handling, which is more complex than simply checking the return result
of a function -- especially in (what I would perceive as) cut-n-paste
code like this.

Or perhaps you had another idea in mind?

In any case, let me know what you'd like me to do, and I'll get right at it.

Keith

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

* Re: [PATCH 00/18] Implement full completer limiting
  2015-04-29 19:19   ` Keith Seitz
@ 2015-04-30  0:26     ` Doug Evans
  0 siblings, 0 replies; 26+ messages in thread
From: Doug Evans @ 2015-04-30  0:26 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Keith Seitz writes:
 > On 04/19/2015 08:21 PM, Doug Evans wrote:
 > > I've gone over the entire patch set.  A few things I like, but there's
 > > at least one thing I'm concerned about.  Replicating the above switch
 > > in each completer: IWBN to avoid such duplication.
 > > We should still be able to remove the global state and fix 17960.
 > 
 > The original patch series (necessarily IMO) exposed this
 > memory-management issue, and a global maintainer approved the change.
 > Surely the approving maintainer realized that this code would propagate
 > to other completer functions eventually, no? [I certainly hope the
 > maintainer did not accept that the complete_line hack would be long-lived!]

Just to make sure we're on the same page,
which complete_line hack?

 > I can certainly change the series to add a function which attempts to
 > hide this detail. For example:
 > 
 > /* Returns whether the maximum number of completions has been
 > reached/exceeded.  */
 > 
 > int
 > add_completion (struct completer_data *cdata, const char *match)
 > {
 >   char *match = xstrdup (name);
 >   enum maybe_add_completion_enum add_status;
 > 
 >   add_status = maybe_add_completion (cdata, match);
 >   switch (add_status)
 >     {
 >     case MAYBE_ADD_COMPLETION_OK:
 >       VEC_safe_push (char_ptr, cdata->results, match);
 >       break;
 >     case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
 >       VEC_safe_push (char_ptr, cdata->results, match);
 >       return 1;
 >     case MAYBE_ADD_COMPLETION_MAX_REACHED:
 >       xfree (match);
 >       return 1;
 >     case MAYBE_ADD_COMPLETION_DUPLICATE:
 >       xfree (match);
 >       break;
 >     }
 > 
 >   return 0;
 > }
 > 
 > Given that completions (like symbol reading/searching) are extremely
 > time-consuming, one of my design goals was to not introduce anything
 > that might slow down completion, including multiple allocations and copying.

Is there any data that says malloc/copying plays a significant role
in the time?  At this point the main culprit is file i/o and(/or) debug
info processing / symbol table processing.

I can imagine a case where the number of completions is large enough
that malloc/copying becomes an issue, but I think the number of
completions would have to be pretty large, far higher than any
reasonable value a user might set for max-completions.
[for some definition of "reasonable"]

So at this point I haven't been worrying about it.
We're already doing a fair bit of malloc/copying in the
code that's there today, e.g., xstrdup calls in complete_line,
and every user I talk to is thrilled with the feature.
[Maybe in a few years when they've forgotten it used to take
minutes will they start to complain about the seconds. :-)]

 > Unfortunately with this goal in mind, the use of this proposed function
 > (alone) can only be pushed into four of the fourteen functions dealing
 > with completions.
 > 
 > The ten remaining functions would require an additional malloc/copy or
 > yet another API function, e.g.,
 > add_completion_1/add_completionA/add_completion_Ex [:-)] to deal with
 > this case. All this to simply forgo checking the return result of
 > maybe_add_completion. I don't see the benefit.
 > 
 > Developers already have to deal with memory management in exception
 > handling, which is more complex than simply checking the return result
 > of a function -- especially in (what I would perceive as) cut-n-paste
 > code like this.

Any time one duplicates code one needs to consider the increased
maintenance burden. Plus less code is easier to read/digest.
I'd like to avoid the duplication where I can.

 > Or perhaps you had another idea in mind?

Some completers don't really need to track the count,
(e.g., even if there were 10000 signals, I doubt it'd matter if
signal_completer tracked them - interesting experiment though),
and the final pass performs its own limiting, so one thing
I have in mind is that individual tracking by every completer
is unnecessary.

Another thought I had was, for individual completers that do need
to track the result, to what extent can they all use the same
API provided by completion tracking (e.g., have an API call
that contains the switch() and have the completers call it, like
you've got above).
That way the quantity of these switches doesn't proliferate.

It may be that this API function can handle all completers,
in which case great, signal_completer et.al. can use it too.
I'm just not big on making it a requirement.
[Though of course I would like the consistency. :-)]

 > In any case, let me know what you'd like me to do, and I'll get right at it.

Does the above make sense?

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

* Re: [PATCH 12/18] Implement completion limiting for sim_command_completer.
  2015-04-14 15:28   ` Mike Frysinger
@ 2015-05-05 15:03     ` Keith Seitz
  2015-05-05 15:34       ` Mike Frysinger
  0 siblings, 1 reply; 26+ messages in thread
From: Keith Seitz @ 2015-05-05 15:03 UTC (permalink / raw)
  To: gdb-patches

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/14/2015 08:28 AM, Mike Frysinger wrote:
> i think you want to push this down into sim/common/sim-options.c ?
> otherwise you do all of this work only to throw away a good amount
> of it.

Eventually, yes, but not right now, if I may. I'd be more than happy
to prepare and submit a follow-up patch for sim/ in the next week or
two (pending acceptance of this patch series).

>> It does not add any tests, since the `sim' command is highly 
>> target-dependent and unimplemented for the majority of
>> simulators.
> 
> erm, the majority of simulators now have testsuites.  but that
> doesn't really matter because you're updating the gdb part of the
> API, not the sim side.

Right, and I can only test it on any sim-enabled target, which I
seldom use anymore.

Keith

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVSNvBAAoJEM4/L8mVMm6zV/oQAKAy+9SyBGEXxI8FVQUyhRlL
y0Hi9ZHa5aTO+pyn0aDNkhJMSyS53F3nNLt2IEvgr/+blN+YISufpU6Tn4AX59gD
1lTzGXMKHRhdZ/uo5x+v0jM/i+05cZV72bfyvWydt+zJlU6mxV1vTwMrxIJRBgE8
ibbzvXNCCll1tkzICAe/fFVOOLgQpbSKj2PQH3f1RhwX7Xa0K0oGfxQhsh8iGVue
SGVdIS0H1EMeegcHT5IxasykqUD1IMKFVCv/pdvFJQU+2F8bqlNPMT0ODabULHLq
i6gLEicLMG4XfFjs7yOttQqezrCWk5fR78IH/fFn/xex8e58KX5F7cyVCo2qf3hC
1zJjw0+N2tpDSBXmos8s064+pFWK4dbFin36ASTpMFI7Qf/QqgQhSRYbgQ3zvXw7
pLzC4tkdCBJ/2IKsLYQZf3zr8S8wKKCG5jJkXeThbhFXMz1ZviQZ/evTkGcbLSi2
K8PBKilWYwPktL6mN1cQB0UDi7g9lvYb5v7Q+oM2boCH7LLNgKiGTrQC6sCh+CLK
emz6EPiSt0tiOaDQo1txcOL3jKEojZyoAwGuIFohtvrHmWYsDr5DAz96ZiWvTirE
IrztQ5toVTKo1rQ8MtKjwcCi0rGDOhJ/FEyvZN7aZ8NPW8IdyaJcp6Ozrjq1ZLl+
yaamOkqLO4CsBsK5rUNr
=Gh9i
-----END PGP SIGNATURE-----

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

* Re: [PATCH 12/18] Implement completion limiting for sim_command_completer.
  2015-05-05 15:03     ` Keith Seitz
@ 2015-05-05 15:34       ` Mike Frysinger
  2015-05-05 15:53         ` Keith Seitz
  0 siblings, 1 reply; 26+ messages in thread
From: Mike Frysinger @ 2015-05-05 15:34 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On 05 May 2015 08:03, Keith Seitz wrote:
> On 04/14/2015 08:28 AM, Mike Frysinger wrote:
> > i think you want to push this down into sim/common/sim-options.c ?
> > otherwise you do all of this work only to throw away a good amount
> > of it.
> 
> Eventually, yes, but not right now, if I may. I'd be more than happy
> to prepare and submit a follow-up patch for sim/ in the next week or
> two (pending acceptance of this patch series).
> 
> >> It does not add any tests, since the `sim' command is highly 
> >> target-dependent and unimplemented for the majority of
> >> simulators.
> > 
> > erm, the majority of simulators now have testsuites.  but that
> > doesn't really matter because you're updating the gdb part of the
> > API, not the sim side.
> 
> Right, and I can only test it on any sim-enabled target, which I
> seldom use anymore.

that's pretty trivial to get though

$ mkdir build
$ cd build
$ ../configure --target=ft32-elf
$ make all-gdb
$ ./gdb/gdb
(gdb) target sim
(gdb) sim <tab>
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 12/18] Implement completion limiting for sim_command_completer.
  2015-05-05 15:34       ` Mike Frysinger
@ 2015-05-05 15:53         ` Keith Seitz
  0 siblings, 0 replies; 26+ messages in thread
From: Keith Seitz @ 2015-05-05 15:53 UTC (permalink / raw)
  To: gdb-patches

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 05/05/2015 08:34 AM, Mike Frysinger wrote:
> that's pretty trivial to get though
> 
> $ mkdir build $ cd build $ ../configure --target=ft32-elf $ make
> all-gdb $ ./gdb/gdb (gdb) target sim (gdb) sim <tab>

Sure, and thanks to you [:-)] I did a nearly trivial amount of digging
and found what *might* be a reliable way to test if the test suite is
using a simulator target. mi-support.exp's mi_gdb_target_load tests
"target_info protocol" for "sim".

So looks like I might actually be able to generically write a test for
this.

Thanks for the prodding!

Keith

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVSOdxAAoJEM4/L8mVMm6z/+sP/RfHfZi3Ftw+jJfjmJri7xM6
i2nS5kVlvCd6tCxLIEIW9d8epHqkMK75UsclV7SLuiwjHwh3n4dZUxCP6LpgGLbM
ww2nL46qk3KAzVLI+phIHqso1lZRVbp2cWrS1ipEbfoaRwig3dfeWJX3WLa0pJSa
+tw6rdTZ6kKFqqGh0kTrvVwG3gQNg9vFJ1t/7y10Eug7uswc7SVKA6oxGEuPPspo
R+6GJOkkfL4CfJh9HqztPCd4FdEnG6nn2FILpcVuXeXdVVMw0ls/AMTIltMkXB/V
4pTrtIBQm3mu5jC4NyL+RXdQ+f8u3rqBv9AOm7WRAAF658ic1qqUPcpVUYOPwvOd
xlFg3Wz7f8QZSn6+I8t2bx7YTbdWTEUoQN7h7HQBoFKyZfrs938Rwf2Y+b0udqyi
R8jnaWLEsOfCeZL6qH29O7UUwDsAIL6URMZaddQRHk4q48sO34+7k2gXQnDBMZgw
zzb093al0xPKKD7Qb4xJ4UqG2YGygOScPJTuupiszFLuL8TdQFKFZjDL46XeuD82
RiGd9+XY6hT8nt3zgDReAAy2EoiXq7UASkz87AI1UFzJeV5fanqJKhKIUUeI7+eh
eCLo5lYnNweHoKm1NnvajH8Hm+KmwRT87QgN5W+YJmV8lddBqRXsXI+H6vGYGXe2
ifz5rM+n49gHe6XqAAz5
=+5N5
-----END PGP SIGNATURE-----

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

end of thread, other threads:[~2015-05-05 15:53 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-13 19:23 [PATCH 00/18] Implement full completer limiting Keith Seitz
2015-04-13 19:23 ` [PATCH 14/18] Implement completion limiting in add_struct_fields Keith Seitz
2015-04-13 19:23 ` [PATCH 09/18] Implement completion limiting for interpreter_completer Keith Seitz
2015-04-13 19:23 ` [PATCH 06/18] Implement completion limiting for condition_completer Keith Seitz
2015-04-13 19:23 ` [PATCH 10/18] Implement completion limiting for cmdpy_completer Keith Seitz
2015-04-13 19:23 ` [PATCH 12/18] Implement completion limiting for sim_command_completer Keith Seitz
2015-04-14 15:28   ` Mike Frysinger
2015-05-05 15:03     ` Keith Seitz
2015-05-05 15:34       ` Mike Frysinger
2015-05-05 15:53         ` Keith Seitz
2015-04-13 19:23 ` [PATCH 16/18] Make the completion API completely opaque Keith Seitz
2015-04-13 19:23 ` [PATCH 01/18] Add struct completer_data to the completion API Keith Seitz
2015-04-13 19:23 ` [PATCH 03/18] Implement completion limiting for complete_on_cmdlist Keith Seitz
2015-04-13 19:23 ` [PATCH 13/18] Implement completion limiting for complete_on_enum Keith Seitz
2015-04-13 19:23 ` [PATCH 17/18] Use the hashtable to accumulate completion results Keith Seitz
2015-04-13 19:23 ` [PATCH 08/18] Implement completion limiting for signal_completer Keith Seitz
2015-04-13 19:23 ` [PATCH 11/18] Implement completion limiting for reg_or_group_completer Keith Seitz
2015-04-13 19:23 ` [PATCH 18/18] Remove the vector return result from the completion API Keith Seitz
2015-04-13 19:23 ` [PATCH 04/18] Implement completion limiting for add_filename_to_list Keith Seitz
2015-04-13 19:23 ` [PATCH 02/18] Remove completion_tracker_t from the public completion API Keith Seitz
2015-04-13 19:23 ` [PATCH 07/18] Implement completion limiting for filename_completer Keith Seitz
2015-04-13 19:23 ` [PATCH 15/18] Implement completion limiting for scmcmd_add_completion Keith Seitz
2015-04-13 19:23 ` [PATCH 05/18] Implement completion limiting for ada_make_symbol_completion_list Keith Seitz
2015-04-20  3:21 ` [PATCH 00/18] Implement full completer limiting Doug Evans
2015-04-29 19:19   ` Keith Seitz
2015-04-30  0:26     ` Doug Evans

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).