[PATCHv2 2/5] gdb: make struct output_source_filename_data more C++ like
Andrew Burgess
andrew.burgess@embecosm.com
Wed May 19 11:12:39 GMT 2021
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)
<output_source_filename_data>: New constructor. <regexp>: Delete,
this is now in info_sources_filter. <c_regexp>: Delete, this is
now in info_sources_filter. <reset_output>: New member function.
<filename_seen_cache>: Rename to m_filename_seen_cache, change
from being a pointer, to being an actual object. <first>: Rename
to m_first. <print_header>: New member function. <partial_match>:
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<compiled_regex> 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<compiled_regex> 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<filename_partial_match_opts>;
@@ -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
More information about the Gdb-patches
mailing list