From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128656 invoked by alias); 23 Nov 2019 13:17:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 128638 invoked by uid 89); 23 Nov 2019 13:17:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=filenamessize, UD:filenames.size, filenames.size X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 23 Nov 2019 13:17:26 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 35C1E203A5; Sat, 23 Nov 2019 08:17:24 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 228892020A; Sat, 23 Nov 2019 08:17:17 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id E1AA62816F; Sat, 23 Nov 2019 08:17:16 -0500 (EST) X-Gerrit-PatchSet: 5 Date: Sat, 23 Nov 2019 13:17:00 -0000 From: "Simon Marchi (Code Review)" To: Andrew Burgess , gdb-patches@sourceware.org Cc: Joel Brobecker , Christian Biesinger , Tom Tromey Auto-Submitted: auto-generated X-Gerrit-MessageType: comment Subject: [review v5] gdb: Introduce global_symbol_searcher X-Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710 X-Gerrit-Change-Number: 264 X-Gerrit-ChangeURL: X-Gerrit-Commit: 293a303a4ef36bd7999863c2e946fbf73c626ea1 In-Reply-To: References: X-Gerrit-Comment-Date: Sat, 23 Nov 2019 08:17:16 -0500 Reply-To: gnutoolchain-gerrit@osci.io MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Content-Type: text/plain; charset=UTF-8 Message-Id: <20191123131716.E1AA62816F@gnutoolchain-gerrit.osci.io> X-SW-Source: 2019-11/txt/msg00789.txt.bz2 Simon Marchi has posted comments on this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264 ...................................................................... Patch Set 5: Code-Review+2 (6 comments) LGTM, I just noted a few small comments. | --- gdb/symtab.c | +++ gdb/symtab.c | @@ -4457,16 +4437,18 @@ /* See symtab.h. */ | | std::vector | -search_symbols (const char *regexp, enum search_domain kind, | - const char *t_regexp, | - int nfiles, const char *files[], | - bool exclude_minsyms) | -{ | +global_symbol_searcher::search () const | +{ | + const char *regexp = m_symbol_regexp; | + const char *t_regexp = m_type_regexp; | + enum search_domain kind = m_kind; | + bool exclude_minsyms = m_exclude_minsyms; | + int nfiles = filenames.size (); PS3, Line 4445: Done | const struct blockvector *bv; | const struct block *b; | int i = 0; | struct block_iterator iter; | struct symbol *sym; | int found_misc = 0; | static const enum minimal_symbol_type types[] | = {mst_data, mst_text, mst_unknown}; | static const enum minimal_symbol_type types2[] ... | @@ -4539,16 +4521,19 @@ global_symbol_searcher::search () const | } | | /* Search through the partial symtabs *first* for all symbols | matching the regexp. That way we don't have to reproduce all of | the machinery below. */ | expand_symtabs_matching ([&] (const char *filename, bool basenames) | { | - return file_matches (filename, files, nfiles, | - basenames); | + /* EXPAND_SYMTABS_MATCHING expects a callback | + that returns an integer, not a boolean as | + FILE_MATCHES does. */ PS3, Line 4530: Done | + return file_matches (filename, filenames, | + basenames) ? 1 : 0; | }, | lookup_name_info::match_any (), | [&] (const char *symname) | { | return (!preg.has_value () | || preg->exec (symname, | 0, NULL, 0) == 0); | --- gdb/symtab.c | +++ gdb/symtab.c | @@ -4527,24 +4506,26 @@ global_symbol_searcher::search () const | } | | int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off | ? REG_ICASE : 0); | - preg.emplace (regexp, cflags, _("Invalid regexp")); | - } | - | - if (t_regexp != NULL) | + preg.emplace (symbol_name_regexp, cflags, | + _("Invalid m_symbol_name_regexp")); PS5, Line 4511: These are user visible regexp, I think they should be user friendly messages like "Invalid symbol name regexp" | + } | + | + if (m_symbol_type_regexp != NULL) | { | int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off | ? REG_ICASE : 0); | - treg.emplace (t_regexp, cflags, _("Invalid regexp")); | + treg.emplace (m_symbol_type_regexp, cflags, | + _("Invalid m_symbol_namregexp")); PS5, Line 4519: Same. | } | | /* Search through the partial symtabs *first* for all symbols | - matching the regexp. That way we don't have to reproduce all of | + matching the m_symbol_namregexp. That way we don't have to reproduce all of PS5, Line 4523: m_symbol_name_regexp | the machinery below. */ | expand_symtabs_matching ([&] (const char *filename, bool basenames) | { | - return file_matches (filename, files, nfiles, | + return file_matches (filename, filenames, | basenames); | }, | lookup_name_info::match_any (), | [&] (const char *symname) ... | @@ -4670,16 +4652,16 @@ global_symbol_searcher::search () const | } | } | | if (!result.empty ()) | sort_search_symbols_remove_dups (&result); | | /* If there are no eyes, avoid all contact. I mean, if there are | no debug symbols, then add matching minsyms. But if the user wants | - to see symbols matching a type regexp, then never give a minimal symbol, | + to see symbols matching a type m_symbol_namregexp, then never give a minimal symbol, PS5, Line 4660: That sounds not correct. | as we assume that a minimal symbol does not have a type. */ | | - if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN)) | - && !exclude_minsyms | + if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN)) | + && !m_exclude_minsyms | && !treg.has_value ()) | { | for (objfile *objfile : current_program_space->objfiles ()) -- Gerrit-Project: binutils-gdb Gerrit-Branch: master Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710 Gerrit-Change-Number: 264 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Burgess Gerrit-Reviewer: Andrew Burgess Gerrit-Reviewer: Simon Marchi Gerrit-Reviewer: Tom Tromey Gerrit-CC: Christian Biesinger Gerrit-CC: Joel Brobecker Gerrit-Comment-Date: Sat, 23 Nov 2019 13:17:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Simon Marchi Gerrit-MessageType: comment