Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/4] gdb: make struct output_source_filename_data more C++ like
Date: Mon, 26 Apr 2021 18:07:01 +0100	[thread overview]
Message-ID: <b301ce462403c3cac6d00488fa8eef00192f03b2.1619456691.git.andrew.burgess@embecosm.com> (raw)
In-Reply-To: <cover.1619456691.git.andrew.burgess@embecosm.com>

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 change to make filename_partial_match_opts private within
output_source_filename_data might seem unhelpful (we now have to
create a stack local variable and initialise this field through the
constructor), however, in a later commit I plan to split the function
info_sources_command in two, and having output_source_filename_data
initialised entirely through the constructor will make this process
easier.

There should be no user visible changes after this commit.

gdb/ChangeLog:

	* symtab.c (struct filename_partial_match_opts): Update comment.
	(struct output_source_filename_data)
	<output_source_filename_data>: New constructor.  <regexp>: Rename
	to m_regexp.  <c_regexp>: Rename to m_c_regexp.  <reset_output>:
	New member function.  <filename_seen_cache>: Rename to
	m_filename_seen_cache, change from being a pointer, to being an
	actual object.  <set_regexp>: New member function.  <first>:
	Rename to m_first.  <print_header>: New member
	function. <partial_match>: Renamed to m_partial_match.
	(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 |  24 +++++++
 gdb/symtab.c  | 183 +++++++++++++++++++++++++++++---------------------
 2 files changed, 129 insertions(+), 78 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 061177e1d23..5650d225752 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4200,7 +4200,8 @@ operator_chars (const char *p, const char **end)
 }
 \f
 
-/* What part to match in a file name.  */
+/* 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
 {
@@ -4211,35 +4212,83 @@ struct filename_partial_match_opts
   bool basename = false;
 };
 
-/* Data structure to maintain printing state for output_source_filename.  */
+/* Data structure to maintain the state used for printing the results of
+   the 'info sources' command.  */
 
 struct output_source_filename_data
 {
-  /* 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;
+  output_source_filename_data (const char *regexp,
+			       bool match_on_dirname,
+			       bool match_on_basename)
+  {
+    m_partial_match.dirname = match_on_dirname;
+    m_partial_match.basename = match_on_basename;
 
+    if (regexp != nullptr)
+      set_regexp (regexp);
+  }
 
-  /* Cache of what we've seen so far.  */
-  struct filename_seen_cache *filename_seen_cache;
+  DISABLE_COPY_AND_ASSIGN (output_source_filename_data);
 
-  /* Flag of whether we're printing the first one.  */
-  int first;
+  /* 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.  */
   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:
+
+  /* Set the current regular expression, used for matching against files,
+     to REGEXP.  */
+  void set_regexp (const std::string regexp)
+  {
+    m_regexp = std::move (regexp);
+    if (!m_regexp.empty ())
+      {
+	int cflags = REG_NOSUB;
+#ifdef HAVE_CASE_INSENSITIVE_FILE_SYSTEM
+	cflags |= REG_ICASE;
+#endif
+	m_c_regexp.emplace (m_regexp.c_str (), cflags,
+			  _("Invalid regexp"));
+      }
+  }
+
+  /* Output only filenames matching REGEXP.  */
+  std::string m_regexp;
+  gdb::optional<compiled_regex> m_c_regexp;
+
+  /* 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;
+
+  /* Possibly only match a part of the filename.  */
+  filename_partial_match_opts m_partial_match;
 };
 
+/* See comment is class declaration above.  */
+
 void
 output_source_filename_data::output (const char *name)
 {
@@ -4253,41 +4302,62 @@ output_source_filename_data::output (const char *name)
      symtabs; it doesn't hurt to check.  */
 
   /* Was NAME already seen?  */
-  if (filename_seen_cache->seen (name))
+  if (m_filename_seen_cache.seen (name))
     {
       /* Yes; don't print it again.  */
       return;
     }
 
   /* Does it match regexp?  */
-  if (c_regexp.has_value ())
+  if (m_c_regexp.has_value ())
     {
       const char *to_match;
       std::string dirname;
 
-      if (partial_match.dirname)
+      if (m_partial_match.dirname)
 	{
 	  dirname = ldirname (name);
 	  to_match = dirname.c_str ();
 	}
-      else if (partial_match.basename)
+      else if (m_partial_match.basename)
 	to_match = lbasename (name);
       else
 	to_match = name;
 
-      if (c_regexp->exec (to_match, 0, NULL, 0) != 0)
+      if (m_c_regexp->exec (to_match, 0, NULL, 0) != 0)
 	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);
+  if (!m_regexp.empty ())
+    {
+      if (m_partial_match.dirname)
+	printf_filtered (_("(dirname matching regular expression \"%s\")"),
+			 m_regexp.c_str ());
+      else if (m_partial_match.basename)
+	printf_filtered (_("(basename matching regular expression \"%s\")"),
+			 m_regexp.c_str ());
+      else
+	printf_filtered (_("(filename matching regular expression \"%s\")"),
+			 m_regexp.c_str ());
+    }
+  puts_filtered ("\n");
+}
+
 using isrc_flag_option_def
   = gdb::option::flag_option_def<filename_partial_match_opts>;
 
@@ -4316,31 +4386,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 +4399,33 @@ 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."));
 
+  struct 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 ();
-  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"));
-    }
+  struct output_source_filename_data data (regex, match_opts.dirname,
+					   match_opts.basename);
 
-  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 +4441,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


  parent reply	other threads:[~2021-04-26 17:07 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 17:06 [PATCH 0/4] New option for 'info sources', also better MI support Andrew Burgess
2021-04-26 17:07 ` [PATCH 1/4] gdb: add new function quick_symbol_functions::has_unexpanded_symbols Andrew Burgess
2021-05-13 14:38   ` Simon Marchi via Gdb-patches
2021-05-13 17:29     ` Tom Tromey
2021-05-13 14:46   ` Simon Marchi via Gdb-patches
2021-04-26 17:07 ` Andrew Burgess [this message]
2021-05-13 14:58   ` [PATCH 2/4] gdb: make struct output_source_filename_data more C++ like Simon Marchi via Gdb-patches
2021-04-26 17:07 ` [PATCH 3/4] gdb: add new -group-by-binary flag to info sources command Andrew Burgess
2021-04-26 17:34   ` Eli Zaretskii via Gdb-patches
2021-05-13 15:05   ` Simon Marchi via Gdb-patches
2021-05-15  8:45     ` Andrew Burgess
2021-05-15 13:19       ` Simon Marchi via Gdb-patches
2021-04-26 17:07 ` [PATCH 4/4] gdb/mi: extend -file-list-exec-source-files command Andrew Burgess
2021-04-26 17:39   ` Eli Zaretskii via Gdb-patches
2021-05-13 15:47   ` Simon Marchi via Gdb-patches
2021-05-13 10:34 ` [PATCH 0/4] New option for 'info sources', also better MI support Andrew Burgess
2021-05-19 11:12 ` [PATCHv2 0/5] "info sources" - group by objfile Andrew Burgess
2021-05-19 11:12   ` [PATCHv2 1/5] gdb: add new function quick_symbol_functions::has_unexpanded_symbols Andrew Burgess
2021-05-19 11:12   ` [PATCHv2 2/5] gdb: make struct output_source_filename_data more C++ like Andrew Burgess
2021-05-19 11:12   ` [PATCHv2 3/5] gdb/mi: add regexp filtering to -file-list-exec-source-files Andrew Burgess
2021-05-19 11:51     ` Eli Zaretskii via Gdb-patches
2021-05-19 11:12   ` [PATCHv2 4/5] gdb/mi: add new --group-by-objfile flag for -file-list-exec-source-files Andrew Burgess
2021-05-19 11:44     ` Eli Zaretskii via Gdb-patches
2021-05-19 11:12   ` [PATCHv2 5/5] gdb: change info sources to group results by objfile Andrew Burgess
2021-05-19 11:53     ` Eli Zaretskii via Gdb-patches
2021-06-03 13:08     ` Simon Marchi via Gdb-patches
2021-06-03  9:27   ` [PATCHv2 0/5] "info sources" - group " Andrew Burgess
2021-06-03 13:15     ` Simon Marchi via Gdb-patches
2021-06-07 18:32   ` [PATCHv3 " Andrew Burgess
2021-06-07 18:32     ` [PATCHv3 1/5] gdb: add new function quick_symbol_functions::has_unexpanded_symbols Andrew Burgess
2021-06-07 18:32     ` [PATCHv3 2/5] gdb: make struct output_source_filename_data more C++ like Andrew Burgess
2021-07-05 12:31       ` Tom de Vries
2021-07-26 13:21         ` Andrew Burgess
2021-06-07 18:32     ` [PATCHv3 3/5] gdb/mi: add regexp filtering to -file-list-exec-source-files Andrew Burgess
2021-06-07 18:32     ` [PATCHv3 4/5] gdb/mi: add new --group-by-objfile flag for -file-list-exec-source-files Andrew Burgess
2021-06-07 18:32     ` [PATCHv3 5/5] gdb: change info sources to group results by objfile Andrew Burgess
2021-06-21 12:02     ` PING! Re: [PATCHv3 0/5] "info sources" - group " Andrew Burgess
2021-06-25 20:08       ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b301ce462403c3cac6d00488fa8eef00192f03b2.1619456691.git.andrew.burgess@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox