From 60a20c190789fd75d1955576160cbbfe94c792fb Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 13 Dec 2017 16:38:49 +0000 Subject: [PATCH] Factor out final completion match string building 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 * 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 &&, 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 &&, 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 | 19 +++++++++ gdb/cli/cli-decode.c | 41 ++------------------ gdb/completer.c | 92 ++++++++++++++++++++++++++++++-------------- gdb/completer.h | 15 ++++++++ gdb/interps.c | 20 +--------- gdb/symtab.c | 50 ++---------------------- 6 files changed, 106 insertions(+), 131 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 835b6887ec9..25ef530a101 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,22 @@ +2017-12-13 Pedro Alves + + * 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 &&, + 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 &&, + 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 * or1k-tdep.c (or1k_analyse_inst): Use _() wrapper for message diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 81df308f473..d657de2dd05 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 (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 (match)); - } + tracker.add_completion (make_completion_match_str (name, text, word)); } diff --git a/gdb/completer.c b/gdb/completer.c index 1cfabcd2516..01951144867 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 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 (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 +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 (newobj); +} + +/* See completer.h. */ + +gdb::unique_xmalloc_ptr +make_completion_match_str (gdb::unique_xmalloc_ptr &&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 (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 9ce70bffffb..73c0f4c7832 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 + 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 + make_completion_match_str (gdb::unique_xmalloc_ptr &&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 b177a8969eb..fa71bb4f74c 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 (match)); + tracker.add_completion + (make_completion_match_str (interp.name, text, word)); } } } diff --git a/gdb/symtab.c b/gdb/symtab.c index 996d52199fc..bb98619a7cb 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 completion (newobj); + gdb::unique_xmalloc_ptr 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.30.2