From patchwork Wed Dec 13 13:26:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 24911 Received: (qmail 16958 invoked by alias); 13 Dec 2017 13:26:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 16906 invoked by uid 89); 13 Dec 2017 13:26:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=TEXT, unnecessarily, composed, 1589 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Dec 2017 13:26:39 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D2A945F7BC for ; Wed, 13 Dec 2017 13:26:38 +0000 (UTC) Received: from cascais.lan (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 284E1614FF for ; Wed, 13 Dec 2017 13:26:37 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 1/3] Factor out final completion match string building Date: Wed, 13 Dec 2017 13:26:34 +0000 Message-Id: <1513171596-10665-2-git-send-email-palves@redhat.com> In-Reply-To: <1513171596-10665-1-git-send-email-palves@redhat.com> References: <1513171596-10665-1-git-send-email-palves@redhat.com> 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 * 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/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 (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 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 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 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 + 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 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 (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 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