[PATCH 1/3] Factor out final completion match string building

Pedro Alves palves@redhat.com
Wed Dec 13 13:26:00 GMT 2017


We have several places doing essentially the same thing; factor them
out to a central place.  Some of the places overallocate for no good
reason, or use strcat unnecessarily.  The centralized version is more
precise and to the point.

(I considered making the gdb::unique_xmalloc_ptr overload version of
make_completer_match_str try to realloc (not xrealloc) probably
avoiding an allocation in most cases, but that'd be probably overdoing
it, and also, now that I'm writing this I thought I'd try to see how
could we ever get to filename_completer with "text != word", but I
couldn't figure it out.  Running the testsuite with 'gdb_assert (text
== word);' never tripped on the assertion either.  So post gdb 8.1,
I'll probably propose a patch to simplify filename_completer a bit,
and the gdb::unique_xmalloc_str overload can be removed then.)

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* cli/cli-decode.c (complete_on_cmdlist, complete_on_enum): Use
	make_completion_match_str.
	* completer.c: Use gdb::unique_xmalloc_ptr and
	make_completion_match_str.
	(make_completion_match_str_1): New.
	(make_completion_match_str(const char *, const char *,
	const char *)): New.
	(make_completion_match_str(gdb::unique_xmalloc_ptr<char> &&,
	const char *, const char *)): New.
	* completer.h (make_completion_match_str(const char *,
	const char *, const char *)): New.
	(make_completion_match_str(gdb::unique_xmalloc_ptr<char> &&,
	const char *, const char *)): New.
	* interps.c (interpreter_completer): Use make_completion_match_str.
	* symtab.c (completion_list_add_name, add_filename_to_list): Use
	make_completion_match_str.
---
 gdb/cli/cli-decode.c | 41 ++---------------------
 gdb/completer.c      | 92 ++++++++++++++++++++++++++++++++++++----------------
 gdb/completer.h      | 15 +++++++++
 gdb/interps.c        | 20 ++----------
 gdb/symtab.c         | 50 ++--------------------------
 5 files changed, 87 insertions(+), 131 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 81df308..d657de2 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1816,8 +1816,6 @@ complete_on_cmdlist (struct cmd_list_element *list,
 	    && (!ignore_help_classes || ptr->func
 		|| ptr->prefixlist))
 	  {
-	    char *match;
-
 	    if (pass == 0)
 	      {
 		if (ptr->cmd_deprecated)
@@ -1827,22 +1825,8 @@ complete_on_cmdlist (struct cmd_list_element *list,
 		  }
 	      }
 
-	    match = (char *) xmalloc (strlen (word) + strlen (ptr->name) + 1);
-	    if (word == text)
-	      strcpy (match, ptr->name);
-	    else if (word > text)
-	      {
-		/* Return some portion of ptr->name.  */
-		strcpy (match, ptr->name + (word - text));
-	      }
-	    else
-	      {
-		/* Return some of text plus ptr->name.  */
-		strncpy (match, word, text - word);
-		match[text - word] = '\0';
-		strcat (match, ptr->name);
-	      }
-	    tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
+	    tracker.add_completion
+	      (make_completion_match_str (ptr->name, text, word));
 	    got_matches = true;
 	  }
 
@@ -1876,26 +1860,7 @@ complete_on_enum (completion_tracker &tracker,
 
   for (i = 0; (name = enumlist[i]) != NULL; i++)
     if (strncmp (name, text, textlen) == 0)
-      {
-	char *match;
-
-	match = (char *) xmalloc (strlen (word) + strlen (name) + 1);
-	if (word == text)
-	  strcpy (match, name);
-	else if (word > text)
-	  {
-	    /* Return some portion of name.  */
-	    strcpy (match, name + (word - text));
-	  }
-	else
-	  {
-	    /* Return some of text plus name.  */
-	    strncpy (match, word, text - word);
-	    match[text - word] = '\0';
-	    strcat (match, name);
-	  }
-	tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
-      }
+      tracker.add_completion (make_completion_match_str (name, text, word));
 }
 
 
diff --git a/gdb/completer.c b/gdb/completer.c
index 1cfabcd..0195114 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -158,10 +158,9 @@ filename_completer (struct cmd_list_element *ignore,
   subsequent_name = 0;
   while (1)
     {
-      char *p, *q;
-
-      p = rl_filename_completion_function (text, subsequent_name);
-      if (p == NULL)
+      gdb::unique_xmalloc_ptr<char> p_rl
+	(rl_filename_completion_function (text, subsequent_name));
+      if (p_rl == NULL)
 	break;
       /* We need to set subsequent_name to a non-zero value before the
 	 continue line below, because otherwise, if the first file
@@ -170,32 +169,12 @@ filename_completer (struct cmd_list_element *ignore,
       subsequent_name = 1;
       /* Like emacs, don't complete on old versions.  Especially
          useful in the "source" command.  */
+      const char *p = p_rl.get ();
       if (p[strlen (p) - 1] == '~')
-	{
-	  xfree (p);
-	  continue;
-	}
+	continue;
 
-      if (word == text)
-	/* Return exactly p.  */
-	q = p;
-      else if (word > text)
-	{
-	  /* Return some portion of p.  */
-	  q = (char *) xmalloc (strlen (p) + 5);
-	  strcpy (q, p + (word - text));
-	  xfree (p);
-	}
-      else
-	{
-	  /* Return some of TEXT plus p.  */
-	  q = (char *) xmalloc (strlen (p) + (text - word) + 5);
-	  strncpy (q, word, text - word);
-	  q[text - word] = '\0';
-	  strcat (q, p);
-	  xfree (p);
-	}
-      tracker.add_completion (gdb::unique_xmalloc_ptr<char> (q));
+      tracker.add_completion
+	(make_completion_match_str (std::move (p_rl), text, word));
     }
 #if 0
   /* There is no way to do this just long enough to affect quote
@@ -1580,6 +1559,63 @@ completion_tracker::add_completions (completion_list &&list)
     add_completion (std::move (candidate));
 }
 
+/* Helper for the make_completion_match_str overloads.  Returns NULL
+   as an indication that we want MATCH_NAME exactly.  It is up to the
+   caller to xstrdup that string if desired.  */
+
+static char *
+make_completion_match_str_1 (const char *match_name,
+			     const char *text, const char *word)
+{
+  char *newobj;
+
+  if (word == text)
+    {
+      /* Return NULL as an indication that we want MATCH_NAME
+	 exactly.  */
+      return NULL;
+    }
+  else if (word > text)
+    {
+      /* Return some portion of MATCH_NAME.  */
+      newobj = xstrdup (match_name + (word - text));
+    }
+  else
+    {
+      /* Return some of WORD plus MATCH_NAME.  */
+      size_t len = strlen (match_name);
+      newobj = (char *) xmalloc (text - word + len + 1);
+      memcpy (newobj, word, text - word);
+      memcpy (newobj + (text - word), match_name, len + 1);
+    }
+
+  return newobj;
+}
+
+/* See completer.h.  */
+
+gdb::unique_xmalloc_ptr<char>
+make_completion_match_str (const char *match_name,
+			   const char *text, const char *word)
+{
+  char *newobj = make_completion_match_str_1 (match_name, text, word);
+  if (newobj == NULL)
+    newobj = xstrdup (match_name);
+  return gdb::unique_xmalloc_ptr<char> (newobj);
+}
+
+/* See completer.h.  */
+
+gdb::unique_xmalloc_ptr<char>
+make_completion_match_str (gdb::unique_xmalloc_ptr<char> &&match_name,
+			   const char *text, const char *word)
+{
+  char *newobj = make_completion_match_str_1 (match_name.get (), text, word);
+  if (newobj == NULL)
+    return std::move (match_name);
+  return gdb::unique_xmalloc_ptr<char> (newobj);
+}
+
 /* Generate completions all at once.  Does nothing if max_completions
    is 0.  If max_completions is non-negative, this will collect at
    most max_completions strings.
diff --git a/gdb/completer.h b/gdb/completer.h
index 9ce70bf..73c0f4c 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -482,6 +482,21 @@ private:
   bool m_lowest_common_denominator_unique = false;
 };
 
+/* Return a string to hand off to readline as a completion match
+   candidate, potentially composed of parts of MATCH_NAME and of
+   TEXT/WORD.  For a description of TEXT/WORD see completer_ftype.  */
+
+extern gdb::unique_xmalloc_ptr<char>
+  make_completion_match_str (const char *match_name,
+			     const char *text, const char *word);
+
+/* Like above, but takes ownership of MATCH_NAME (i.e., can
+   reuse/return it).  */
+
+extern gdb::unique_xmalloc_ptr<char>
+  make_completion_match_str (gdb::unique_xmalloc_ptr<char> &&match_name,
+			     const char *text, const char *word);
+
 extern void gdb_display_match_list (char **matches, int len, int max,
 				    const struct match_list_displayer *);
 
diff --git a/gdb/interps.c b/gdb/interps.c
index b177a89..fa71bb4 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -439,24 +439,8 @@ interpreter_completer (struct cmd_list_element *ignore,
     {
       if (strncmp (interp.name, text, textlen) == 0)
 	{
-	  char *match;
-
-	  match = (char *) xmalloc (strlen (word) + strlen (interp.name) + 1);
-	  if (word == text)
-	    strcpy (match, interp.name);
-	  else if (word > text)
-	    {
-	      /* Return some portion of interp->name.  */
-	      strcpy (match, interp.name + (word - text));
-	    }
-	  else
-	    {
-	      /* Return some of text plus interp->name.  */
-	      strncpy (match, word, text - word);
-	      match[text - word] = '\0';
-	      strcat (match, interp.name);
-	    }
-	  tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
+	  tracker.add_completion
+	    (make_completion_match_str (interp.name, text, word));
 	}
     }
 }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 996d521..bb98619 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4725,29 +4725,8 @@ completion_list_add_name (completion_tracker &tracker,
      of matches.  Note that the name is moved to freshly malloc'd space.  */
 
   {
-    char *newobj;
-
-    if (word == text)
-      {
-	newobj = (char *) xmalloc (strlen (symname) + 5);
-	strcpy (newobj, symname);
-      }
-    else if (word > text)
-      {
-	/* Return some portion of symname.  */
-	newobj = (char *) xmalloc (strlen (symname) + 5);
-	strcpy (newobj, symname + (word - text));
-      }
-    else
-      {
-	/* Return some of SYM_TEXT plus symname.  */
-	newobj = (char *) xmalloc (strlen (symname) + (text - word) + 5);
-	strncpy (newobj, word, text - word);
-	newobj[text - word] = '\0';
-	strcat (newobj, symname);
-      }
-
-    gdb::unique_xmalloc_ptr<char> completion (newobj);
+    gdb::unique_xmalloc_ptr<char> completion
+      = make_completion_match_str (symname, text, word);
 
     /* Here we pass the match-for-lcd object to add_completion.  Some
        languages match the user text against substrings of symbol
@@ -5322,30 +5301,7 @@ static void
 add_filename_to_list (const char *fname, const char *text, const char *word,
 		      completion_list *list)
 {
-  char *newobj;
-  size_t fnlen = strlen (fname);
-
-  if (word == text)
-    {
-      /* Return exactly fname.  */
-      newobj = (char *) xmalloc (fnlen + 5);
-      strcpy (newobj, fname);
-    }
-  else if (word > text)
-    {
-      /* Return some portion of fname.  */
-      newobj = (char *) xmalloc (fnlen + 5);
-      strcpy (newobj, fname + (word - text));
-    }
-  else
-    {
-      /* Return some of TEXT plus fname.  */
-      newobj = (char *) xmalloc (fnlen + (text - word) + 5);
-      strncpy (newobj, word, text - word);
-      newobj[text - word] = '\0';
-      strcat (newobj, fname);
-    }
-  list->emplace_back (newobj);
+  list->emplace_back (make_completion_match_str (fname, text, word));
 }
 
 static int
-- 
2.5.5



More information about the Gdb-patches mailing list