Factor out final completion match string building
authorPedro Alves <palves@redhat.com>
Wed, 13 Dec 2017 16:38:49 +0000 (16:38 +0000)
committerPedro Alves <palves@redhat.com>
Wed, 13 Dec 2017 16:38:49 +0000 (16:38 +0000)
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:
2017-12-13  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/ChangeLog
gdb/cli/cli-decode.c
gdb/completer.c
gdb/completer.h
gdb/interps.c
gdb/symtab.c

index 835b688..25ef530 100644 (file)
@@ -1,3 +1,22 @@
+2017-12-13  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.
+
 2017-12-13  Stafford Horne  <shorne@gmail.com>
 
        * or1k-tdep.c (or1k_analyse_inst): Use _() wrapper for message
index 81df308..d657de2 100644 (file)
@@ -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));
 }
 
 
index 1cfabcd..0195114 100644 (file)
@@ -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.
index 9ce70bf..73c0f4c 100644 (file)
@@ -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 *);
 
index b177a89..fa71bb4 100644 (file)
@@ -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));
        }
     }
 }
index 996d521..bb98619 100644 (file)
@@ -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