From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by sourceware.org (Postfix) with ESMTPS id 404CC3939C1E for ; Wed, 19 May 2021 11:12:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 404CC3939C1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x42a.google.com with SMTP id p7so9846623wru.10 for ; Wed, 19 May 2021 04:12:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=F4E/s2dvGcu4IjKiECMxEZZSWFgjeo6LxO+B8kzOz4A=; b=Ykik4qJ8iofav1e1l4lCVI2kI9nrvFIMQQm+LEhx8qRP6voY1Eix6TwlvdS2VNdq5q 9FGdQYOT6ogpTyvGzvQkTTQI7kkvIMXdchr96OHf4KRysQRDG5lp+rNhPzEK6ckJrmKP KKtAlQV9FBryCw+MnW0rrttQHWSgAHZwbUpK9etsz2nxwKT2wJzBKtpQx2pRulzm0o7c ljCyDqi3NgkL71LzjGYhaRQZAxaE/Op/pjiIXb0oMVqPxj9Z+NcEPA0ivOZLqVJ0LNRo pWdd7bFsV9aXGqZR49fCu36jnUA5kXERK4eSQ/pZ3tstN4Cij5k8N9UI5LMiTX82giaa /9LQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=F4E/s2dvGcu4IjKiECMxEZZSWFgjeo6LxO+B8kzOz4A=; b=q1vUHC9gEQEvCRDc/DiMju0x9vB3qE3sy9jvcIrX92tGQrMAe5PKXOVwP0nXps2l5i 6NXVwH1zcOWHyJuQmdw0CxPCxGzKDZDTEmQjfuulJw65sWYvYr7uuLCITiKsIKkp0QWm oDc1hHvTvi5qftXGfOaCbWlskmmOoIZ8oKF4hxq7ApHQul9OAIkA1hnIkj+p2Y+hoKD+ /I4oESLDghhjXgLRDvZl1QpDCkvHS2YEAGpsDmS6NvcY89bpy8WYmYczRwk0IoE66+dT X/PzkRXhyByfeIk08KdASlgnCx6cj8oDfHcyzkDAI1WBdVpPyYgZEgoV5DyDQOtuH8it klHQ== X-Gm-Message-State: AOAM531AxgVQiR5CqOVsTvje6XW0LdC5ZU5oFxiQMbSw1a+G9FkmTXKE zqzjnyMFgOwx1T70IfbZ77z8rEIic6NcMg== X-Google-Smtp-Source: ABdhPJxAbHxOuIcF6NWUHv0rf8rWpka/pjD+rYbjBdnwssq1hz8Tt4hhpaellh/mrKzhoiQpAkMfrw== X-Received: by 2002:adf:8b09:: with SMTP id n9mr14094317wra.148.1621422768898; Wed, 19 May 2021 04:12:48 -0700 (PDT) Received: from localhost (host109-151-46-70.range109-151.btcentralplus.com. [109.151.46.70]) by smtp.gmail.com with ESMTPSA id a11sm7311264wrr.48.2021.05.19.04.12.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 May 2021 04:12:48 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCHv2 2/5] gdb: make struct output_source_filename_data more C++ like Date: Wed, 19 May 2021 12:12:39 +0100 Message-Id: <6fa87cbb106b261c42693e13a9caecf58a1a872e.1621421987.git.andrew.burgess@embecosm.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 May 2021 11:12:52 -0000 In a future commit I'm going to be making some changes to the 'info sources' command. While looking at the code I noticed that things could be improved by making struct output_source_filename_data more C++ like (private member variables, and more member functions). That's what this commit does. The 'info sources' filename filtering is split out into a separate class in this commit. In a future commit this new filter class (info_sources_filter) will move into the header file and be used from the MI code. There should be no user visible changes after this commit. gdb/ChangeLog: * symtab.c (struct info_sources_filter): New. (info_sources_filter::info_sources_filter): New function. (info_sources_filter::matches): New function. (info_sources_filter::print): New function. (struct filename_partial_match_opts): Moved to later in the file and update the comment. (struct output_source_filename_data) : New constructor. : Delete, this is now in info_sources_filter. : Delete, this is now in info_sources_filter. : New member function. : Rename to m_filename_seen_cache, change from being a pointer, to being an actual object. : Rename to m_first. : New member function. : Delete. (output_source_filename_data::output): Update now m_filename_seen_cache is no longer a pointer, and for other member variable name changes. Add a header comment. (print_info_sources_header): Renamed to... (output_source_filename_data::print_header): ...this. Update now it's a member function and to take account of member variable renaming. (info_sources_command): Add a header comment, delete stack local filename_seen_cache, initialization of output_source_filename_data is now done by the constructor. Call print_header member function instead of print_info_sources_header, call reset_output member function instead of manually performing the reset. --- gdb/ChangeLog | 29 +++++ gdb/symtab.c | 324 +++++++++++++++++++++++++++++++++----------------- 2 files changed, 247 insertions(+), 106 deletions(-) diff --git a/gdb/symtab.c b/gdb/symtab.c index 9555f94707d..c14ab91996e 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -4200,46 +4200,190 @@ operator_chars (const char *p, const char **end) } -/* What part to match in a file name. */ - -struct filename_partial_match_opts +/* Class used to encapsulate the filename filtering for the "info sources" + command. */ +struct info_sources_filter { - /* Only match the directory name part. */ - bool dirname = false; + /* If filename filtering is being used (see M_C_REGEXP) then which part + of the filename is being filtered against? */ + enum class match_on + { + /* Match against the full filename. */ + FULLNAME, - /* Only match the basename part. */ - bool basename = false; + /* Match only against the directory part of the full filename. */ + DIRNAME, + + /* Match only against the basename part of the full filename. */ + BASENAME + }; + + /* Create a filter of MATCH_TYPE using regular expression REGEXP. If + REGEXP is nullptr then all files will match the filter and MATCH_TYPE + is ignored. + + The string pointed too by REGEXP must remain live and unchanged for + this lifetime of this object as the object only retains a copy of the + pointer. */ + info_sources_filter (match_on match_type, const char *regexp); + + DISABLE_COPY_AND_ASSIGN (info_sources_filter); + + /* Does FULLNAME match the filter defined by this object, return true if + it does, otherwise, return false. If there is no filtering defined + then this function will always return true. */ + bool matches (const char *fullname) const; + + /* Print a single line describing this filter, used as part of the "info + sources" command output. If there is no filter in place then nothing + is printed. */ + void print () const; + +private: + + /* The type of filtering in place. */ + match_on m_match_type; + + /* Points to the original regexp used to create this filter. */ + const char *m_regexp; + + /* A compiled version of M_REGEXP. This object is only given a value if + M_REGEXP is not nullptr and is not the empty string. */ + gdb::optional m_c_regexp; }; -/* Data structure to maintain printing state for output_source_filename. */ +/* See class declaration. */ -struct output_source_filename_data +info_sources_filter::info_sources_filter (match_on match_type, + const char *regexp) + : m_match_type (match_type), + m_regexp (regexp) { - /* Output only filenames matching REGEXP. */ - std::string regexp; - gdb::optional c_regexp; - /* Possibly only match a part of the filename. */ - filename_partial_match_opts partial_match; + /* Setup the compiled regular expression M_C_REGEXP based on M_REGEXP. */ + if (m_regexp != nullptr && *m_regexp != '\0') + { + gdb_assert (m_regexp != nullptr); + int cflags = REG_NOSUB; +#ifdef HAVE_CASE_INSENSITIVE_FILE_SYSTEM + cflags |= REG_ICASE; +#endif + m_c_regexp.emplace (m_regexp, cflags, _("Invalid regexp")); + } +} - /* Cache of what we've seen so far. */ - struct filename_seen_cache *filename_seen_cache; +/* See class declaration. */ - /* Flag of whether we're printing the first one. */ - int first; +bool +info_sources_filter::matches (const char *fullname) const +{ + /* Does it match regexp? */ + if (m_c_regexp.has_value ()) + { + const char *to_match; + std::string dirname; + + switch (m_match_type) + { + case match_on::DIRNAME: + dirname = ldirname (fullname); + to_match = dirname.c_str (); + break; + case match_on::BASENAME: + to_match = lbasename (fullname); + break; + case match_on::FULLNAME: + to_match = fullname; + break; + } + + if (m_c_regexp->exec (to_match, 0, NULL, 0) != 0) + return false; + } + + return true; +} + +/* See class declaration. */ + +void +info_sources_filter::print () const +{ + if (m_c_regexp.has_value ()) + { + gdb_assert (m_regexp != nullptr); + + switch (m_match_type) + { + case match_on::DIRNAME: + printf_filtered (_("(dirname matching regular expression \"%s\")"), + m_regexp); + break; + case match_on::BASENAME: + printf_filtered (_("(basename matching regular expression \"%s\")"), + m_regexp); + break; + case match_on::FULLNAME: + printf_filtered (_("(filename matching regular expression \"%s\")"), + m_regexp); + break; + } + } +} + +/* Data structure to maintain the state used for printing the results of + the 'info sources' command. */ + +struct output_source_filename_data +{ + /* Create an object for displaying the results of the 'info sources' + command. FILTER must remain valid and unchanged for the lifetime of + this object as this object retains a reference to FILTER. */ + output_source_filename_data (const info_sources_filter &filter) + : m_filter (filter) + { /* Nothing. */ } + + DISABLE_COPY_AND_ASSIGN (output_source_filename_data); + + /* Reset enough state of this object so we can match against a new set of + files. The existing regular expression is retained though. */ + void reset_output () + { + m_first = true; + m_filename_seen_cache.clear (); + } - /* Worker for sources_info. Force line breaks at ,'s. - NAME is the name to print. */ + /* Worker for sources_info. Force line breaks at ,'s. NAME is the name + to print. */ void output (const char *name); + /* Prints the header messages for the source files that will be printed + with the matching info present in the current object state. + SYMBOL_MSG is a message that describes what will or has been done with + the symbols of the matching source files. */ + void print_header (const char *symbol_msg); + /* An overload suitable for use as a callback to quick_symbol_functions::map_symbol_filenames. */ void operator() (const char *filename, const char *fullname) { output (fullname != nullptr ? fullname : filename); } + +private: + + /* Flag of whether we're printing the first one. */ + bool m_first = true; + + /* Cache of what we've seen so far. */ + filename_seen_cache m_filename_seen_cache; + + /* How source filename should be filtered. */ + const info_sources_filter &m_filter; }; +/* See comment in class declaration above. */ + void output_source_filename_data::output (const char *name) { @@ -4252,42 +4396,45 @@ output_source_filename_data::output (const char *name) situation. I'm not sure whether this can also happen for symtabs; it doesn't hurt to check. */ - /* Was NAME already seen? */ - if (filename_seen_cache->seen (name)) - { - /* Yes; don't print it again. */ - return; - } - - /* Does it match regexp? */ - if (c_regexp.has_value ()) - { - const char *to_match; - std::string dirname; - - if (partial_match.dirname) - { - dirname = ldirname (name); - to_match = dirname.c_str (); - } - else if (partial_match.basename) - to_match = lbasename (name); - else - to_match = name; + /* Was NAME already seen? If so, then don't print it again. */ + if (m_filename_seen_cache.seen (name)) + return; - if (c_regexp->exec (to_match, 0, NULL, 0) != 0) - return; - } + /* If the filter rejects this file then don't print it. */ + if (!m_filter.matches (name)) + return; /* Print it and reset *FIRST. */ - if (! first) + if (!m_first) printf_filtered (", "); - first = 0; + m_first = false; wrap_here (""); fputs_styled (name, file_name_style.style (), gdb_stdout); } +/* See comment is class declaration above. */ + +void +output_source_filename_data::print_header (const char *symbol_msg) +{ + puts_filtered (symbol_msg); + m_filter.print (); + puts_filtered ("\n"); +} + +/* For the 'info sources' command, what part of the file names should we be + matching the user supplied regular expression against? */ + +struct filename_partial_match_opts +{ + /* Only match the directory name part. */ + bool dirname = false; + + /* Only match the basename part. */ + bool basename = false; +}; + using isrc_flag_option_def = gdb::option::flag_option_def; @@ -4316,31 +4463,6 @@ make_info_sources_options_def_group (filename_partial_match_opts *isrc_opts) return {{info_sources_option_defs}, isrc_opts}; } -/* Prints the header message for the source files that will be printed - with the matching info present in DATA. SYMBOL_MSG is a message - that tells what will or has been done with the symbols of the - matching source files. */ - -static void -print_info_sources_header (const char *symbol_msg, - const struct output_source_filename_data *data) -{ - puts_filtered (symbol_msg); - if (!data->regexp.empty ()) - { - if (data->partial_match.dirname) - printf_filtered (_("(dirname matching regular expression \"%s\")"), - data->regexp.c_str ()); - else if (data->partial_match.basename) - printf_filtered (_("(basename matching regular expression \"%s\")"), - data->regexp.c_str ()); - else - printf_filtered (_("(filename matching regular expression \"%s\")"), - data->regexp.c_str ()); - } - puts_filtered ("\n"); -} - /* Completer for "info sources". */ static void @@ -4354,49 +4476,41 @@ info_sources_command_completer (cmd_list_element *ignore, return; } +/* Implement the 'info sources' command. */ + static void info_sources_command (const char *args, int from_tty) { - struct output_source_filename_data data; - if (!have_full_symbols () && !have_partial_symbols ()) - { - error (_("No symbol table is loaded. Use the \"file\" command.")); - } - - filename_seen_cache filenames_seen; - - auto group = make_info_sources_options_def_group (&data.partial_match); + error (_("No symbol table is loaded. Use the \"file\" command.")); + filename_partial_match_opts match_opts; + auto group = make_info_sources_options_def_group (&match_opts); gdb::option::process_options (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group); - if (args != NULL && *args != '\000') - data.regexp = args; + if (match_opts.dirname && match_opts.basename) + error (_("You cannot give both -basename and -dirname to 'info sources'.")); - data.filename_seen_cache = &filenames_seen; - data.first = 1; + const char *regex = nullptr; + if (args != nullptr && *args != '\000') + regex = args; - if (data.partial_match.dirname && data.partial_match.basename) - error (_("You cannot give both -basename and -dirname to 'info sources'.")); - if ((data.partial_match.dirname || data.partial_match.basename) - && data.regexp.empty ()) - error (_("Missing REGEXP for 'info sources'.")); + if ((match_opts.dirname || match_opts.basename) && regex == nullptr) + error (_("Missing REGEXP for 'info sources'.")); - if (data.regexp.empty ()) - data.c_regexp.reset (); + info_sources_filter::match_on match_type; + if (match_opts.dirname) + match_type = info_sources_filter::match_on::DIRNAME; + else if (match_opts.basename) + match_type = info_sources_filter::match_on::BASENAME; else - { - int cflags = REG_NOSUB; -#ifdef HAVE_CASE_INSENSITIVE_FILE_SYSTEM - cflags |= REG_ICASE; -#endif - data.c_regexp.emplace (data.regexp.c_str (), cflags, - _("Invalid regexp")); - } + match_type = info_sources_filter::match_on::FULLNAME; + + info_sources_filter filter (match_type, regex); + output_source_filename_data data (filter); - print_info_sources_header - (_("Source files for which symbols have been read in:\n"), &data); + data.print_header (_("Source files for which symbols have been read in:\n")); for (objfile *objfile : current_program_space->objfiles ()) { @@ -4412,11 +4526,9 @@ info_sources_command (const char *args, int from_tty) } printf_filtered ("\n\n"); - print_info_sources_header - (_("Source files for which symbols will be read in on demand:\n"), &data); + data.print_header (_("Source files for which symbols will be read in on demand:\n")); - filenames_seen.clear (); - data.first = 1; + data.reset_output (); map_symbol_filenames (data, true /*need_fullname*/); printf_filtered ("\n"); } -- 2.25.4