[PATCH 1/2] Add styling to macro commands

[PATCH 1/2] Add styling to macro commands

[Tom Tromey]

This adds filename styling to "info macro".

gdb/ChangeLog
2019-02-13  Tom Tromey  <tom@tromey.com>

	* macrocmd.c (show_pp_source_pos): Style the file names.

gdb/testsuite/ChangeLog
2019-02-13  Tom Tromey  <tom@tromey.com>

	* gdb.base/style.exp: Use -g3 to compile when possible.  Add test
	for macro styling.
	* gdb.base/style.c (SOME_MACRO): New macro.
---
 gdb/ChangeLog                    |  4 ++++
 gdb/macrocmd.c                   |  9 ++++++---
 gdb/testsuite/ChangeLog          |  6 ++++++
 gdb/testsuite/gdb.base/style.c   |  2 ++
 gdb/testsuite/gdb.base/style.exp | 16 +++++++++++++++-
 5 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index beb9b78b344..3313e65f929 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -22,6 +22,7 @@
 #include "macrotab.h"
 #include "macroexp.h"
 #include "macroscope.h"
+#include "cli/cli-style.h"
 #include "cli/cli-utils.h"
 #include "command.h"
 #include "gdbcmd.h"
@@ -121,14 +122,16 @@ show_pp_source_pos (struct ui_file *stream,
   char *fullname;
 
   fullname = macro_source_fullname (file);
-  fprintf_filtered (stream, "%s:%d\n", fullname, line);
+  fputs_styled (fullname, file_name_style.style (), stream);
+  fprintf_filtered (stream, ":%d\n", line);
   xfree (fullname);
 
   while (file->included_by)
     {
       fullname = macro_source_fullname (file->included_by);
-      fprintf_filtered (gdb_stdout, "  included at %s:%d\n", fullname,
-                        file->included_at_line);
+      fputs_filtered (_("  included at "), stream);
+      fputs_styled (fullname, file_name_style.style (), stream);
+      fprintf_filtered (stream, ":%d\n", file->included_at_line);
       xfree (fullname);
       file = file->included_by;
     }
diff --git a/gdb/testsuite/gdb.base/style.c b/gdb/testsuite/gdb.base/style.c
index 53f65960374..a44936ed731 100644
--- a/gdb/testsuite/gdb.base/style.c
+++ b/gdb/testsuite/gdb.base/style.c
@@ -13,6 +13,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#define SOME_MACRO 23
+
 int
 main (int argc, char **argv)
 {
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 78d04b02903..cbc5924aca4 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -21,7 +21,16 @@ save_vars { env(TERM) } {
     # We need an ANSI-capable terminal to get the output.
     setenv TERM ansi
 
-    if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    set test_macros 0
+    set options debug
+    get_compiler_info
+    if { [test_compiler_info "gcc-*"] || [test_compiler_info "clang-*"] } {
+	lappend options additional_flags=-g3
+	set test_macros 1
+    }
+
+    if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	     $options]} {
 	return -1
     }
 
@@ -45,6 +54,11 @@ save_vars { env(TERM) } {
 
     gdb_test "print &main" " = .* \033\\\[34m$hex\033\\\[m <$main_expr>"
 
+    if {$test_macros} {
+	gdb_test "info macro SOME_MACRO" \
+	    "Defined at $base_file_expr:16\r\n#define SOME_MACRO 23"
+    }
+
     gdb_exit
     gdb_spawn
 
-- 
2.17.2

[PATCH 0/2] Styling + memory management in macro commands

[Tom Tromey]

This series adds a little styling to the macro commands, like "info
macro".  While there i noticed that some manual memory management
could be removed, so the second patch implements this.

Tested on x86-64 Fedora 29.

Tom

[PATCH 2/2] Change macro_source_fullname to return a std::string

[Tom Tromey]

While working on the previous patch, I noticed that if
macro_source_fullname returned a std::string, then the callers would
be simplified.  This patch implements this idea.

gdb/ChangeLog
2019-02-13  Tom Tromey  <tom@tromey.com>

	* macrotab.h (macro_source_fullname): Return a std::string.
	* macrotab.c (macro_include, check_for_redefinition)
	(macro_undef, macro_lookup_definition, foreach_macro)
	(foreach_macro_in_scope): Update.
	(macro_source_fullname): Return a std::string.
	* macrocmd.c (show_pp_source_pos): Update.
---
 gdb/ChangeLog  |  9 ++++++++
 gdb/macrocmd.c | 10 +++-----
 gdb/macrotab.c | 63 ++++++++++++++++++--------------------------------
 gdb/macrotab.h |  5 ++--
 4 files changed, 37 insertions(+), 50 deletions(-)

diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index 3313e65f929..0f81c3d5da4 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -119,20 +119,16 @@ show_pp_source_pos (struct ui_file *stream,
                     struct macro_source_file *file,
                     int line)
 {
-  char *fullname;
-
-  fullname = macro_source_fullname (file);
-  fputs_styled (fullname, file_name_style.style (), stream);
+  std::string fullname = macro_source_fullname (file);
+  fputs_styled (fullname.c_str (), file_name_style.style (), stream);
   fprintf_filtered (stream, ":%d\n", line);
-  xfree (fullname);
 
   while (file->included_by)
     {
       fullname = macro_source_fullname (file->included_by);
       fputs_filtered (_("  included at "), stream);
-      fputs_styled (fullname, file_name_style.style (), stream);
+      fputs_styled (fullname.c_str (), file_name_style.style (), stream);
       fprintf_filtered (stream, ":%d\n", file->included_at_line);
-      xfree (fullname);
       file = file->included_by;
     }
 }
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index f7ce26f850d..fa3061616bd 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -466,8 +466,6 @@ macro_include (struct macro_source_file *source,
      the new one?  */
   if (*link && line == (*link)->included_at_line)
     {
-      char *link_fullname, *source_fullname;
-
       /* This means the compiler is emitting bogus debug info.  (GCC
          circa March 2002 did this.)  It also means that the splay
          tree ordering function, macro_tree_compare, will abort,
@@ -476,12 +474,11 @@ macro_include (struct macro_source_file *source,
 
          First, squawk.  */
 
-      link_fullname = macro_source_fullname (*link);
-      source_fullname = macro_source_fullname (source);
+      std::string link_fullname = macro_source_fullname (*link);
+      std::string source_fullname = macro_source_fullname (source);
       complaint (_("both `%s' and `%s' allegedly #included at %s:%d"),
-		 included, link_fullname, source_fullname, line);
-      xfree (source_fullname);
-      xfree (link_fullname);
+		 included, link_fullname.c_str (), source_fullname.c_str (),
+		 line);
 
       /* Now, choose a new, unoccupied line number for this
          #inclusion, after the alleged #inclusion line.  */
@@ -725,16 +722,14 @@ check_for_redefinition (struct macro_source_file *source, int line,
 
       if (! same)
         {
-	  char *source_fullname, *found_key_fullname;
-	  
-	  source_fullname = macro_source_fullname (source);
-	  found_key_fullname = macro_source_fullname (found_key->start_file);
+	  std::string source_fullname = macro_source_fullname (source);
+	  std::string found_key_fullname
+	    = macro_source_fullname (found_key->start_file);
 	  complaint (_("macro `%s' redefined at %s:%d; "
 		       "original definition at %s:%d"),
-		     name, source_fullname, line, found_key_fullname,
+		     name, source_fullname.c_str (), line,
+		     found_key_fullname.c_str (),
 		     found_key->start_line);
-	  xfree (found_key_fullname);
-	  xfree (source_fullname);
         }
 
       return found_key;
@@ -854,16 +849,13 @@ macro_undef (struct macro_source_file *source, int line,
              #definition.  */
           if (key->end_file)
             {
-	      char *source_fullname, *key_fullname;
-
-	      source_fullname = macro_source_fullname (source);
-	      key_fullname = macro_source_fullname (key->end_file);
+	      std::string source_fullname = macro_source_fullname (source);
+	      std::string key_fullname = macro_source_fullname (key->end_file);
               complaint (_("macro '%s' is #undefined twice,"
                            " at %s:%d and %s:%d"),
-			 name, source_fullname, line, key_fullname,
+			 name, source_fullname.c_str (), line,
+			 key_fullname.c_str (),
 			 key->end_line);
-	      xfree (key_fullname);
-	      xfree (source_fullname);
             }
 
           /* Whether or not we've seen a prior #undefinition, wipe out
@@ -923,14 +915,9 @@ macro_lookup_definition (struct macro_source_file *source,
 
   if (n)
     {
-      struct macro_definition *retval;
-      char *source_fullname;
-
-      source_fullname = macro_source_fullname (source);
-      retval = fixup_definition (source_fullname, line,
-				 (struct macro_definition *) n->value);
-      xfree (source_fullname);
-      return retval;
+      std::string source_fullname = macro_source_fullname (source);
+      return fixup_definition (source_fullname.c_str (), line,
+			       (struct macro_definition *) n->value);
     }
   else
     return 0;
@@ -974,12 +961,10 @@ foreach_macro (splay_tree_node node, void *arg)
   struct macro_for_each_data *datum = (struct macro_for_each_data *) arg;
   struct macro_key *key = (struct macro_key *) node->key;
   struct macro_definition *def;
-  char *key_fullname;
 
-  key_fullname = macro_source_fullname (key->start_file);
-  def = fixup_definition (key_fullname, key->start_line,
+  std::string key_fullname = macro_source_fullname (key->start_file);
+  def = fixup_definition (key_fullname.c_str (), key->start_line,
 			  (struct macro_definition *) node->value);
-  xfree (key_fullname);
 
   datum->fn (key->name, def, key->start_file, key->start_line);
   return 0;
@@ -1004,12 +989,10 @@ foreach_macro_in_scope (splay_tree_node node, void *info)
   struct macro_for_each_data *datum = (struct macro_for_each_data *) info;
   struct macro_key *key = (struct macro_key *) node->key;
   struct macro_definition *def;
-  char *datum_fullname;
 
-  datum_fullname = macro_source_fullname (datum->file);
-  def = fixup_definition (datum_fullname, datum->line,
+  std::string datum_fullname = macro_source_fullname (datum->file);
+  def = fixup_definition (datum_fullname.c_str (), datum->line,
 			  (struct macro_definition *) node->value);
-  xfree (datum_fullname);
 
   /* See if this macro is defined before the passed-in line, and
      extends past that line.  */
@@ -1083,7 +1066,7 @@ free_macro_table (struct macro_table *table)
 
 /* See macrotab.h for the comment.  */
 
-char *
+std::string
 macro_source_fullname (struct macro_source_file *file)
 {
   const char *comp_dir = NULL;
@@ -1092,7 +1075,7 @@ macro_source_fullname (struct macro_source_file *file)
     comp_dir = COMPUNIT_DIRNAME (file->table->compunit_symtab);
 
   if (comp_dir == NULL || IS_ABSOLUTE_PATH (file->filename))
-    return xstrdup (file->filename);
+    return file->filename;
 
-  return concat (comp_dir, SLASH_STRING, file->filename, (char *) NULL);
+  return std::string (comp_dir) + SLASH_STRING + file->filename;
 }
diff --git a/gdb/macrotab.h b/gdb/macrotab.h
index d14449e2f2e..d1e14537bd3 100644
--- a/gdb/macrotab.h
+++ b/gdb/macrotab.h
@@ -351,12 +351,11 @@ void macro_for_each_in_scope (struct macro_source_file *file, int line,
 
 /* Return FILE->filename with possibly prepended compilation directory name.
    This is raw concatenation without the "set substitute-path" and gdb_realpath
-   applications done by symtab_to_fullname.  Returned string must be freed by
-   xfree.
+   applications done by symtab_to_fullname.
 
    THis function ignores the "set filename-display" setting.  Its default
    setting is "relative" which is backward compatible but the former behavior
    of macro filenames printing was "absolute".  */
-extern char *macro_source_fullname (struct macro_source_file *file);
+extern std::string macro_source_fullname (struct macro_source_file *file);
 
 #endif /* MACROTAB_H */
-- 
2.17.2

Re: [PATCH 2/2] Change macro_source_fullname to return a std::string

[Joel Brobecker]

> While working on the previous patch, I noticed that if
> macro_source_fullname returned a std::string, then the callers would
> be simplified.  This patch implements this idea.

Indeed!

> gdb/ChangeLog
> 2019-02-13  Tom Tromey  <tom@tromey.com>
> 
> 	* macrotab.h (macro_source_fullname): Return a std::string.
> 	* macrotab.c (macro_include, check_for_redefinition)
> 	(macro_undef, macro_lookup_definition, foreach_macro)
> 	(foreach_macro_in_scope): Update.
> 	(macro_source_fullname): Return a std::string.
> 	* macrocmd.c (show_pp_source_pos): Update.

The patch looks good also.

-- 
Joel

Re: [PATCH 1/2] Add styling to macro commands

[Joel Brobecker]

> This adds filename styling to "info macro".
> 
> gdb/ChangeLog
> 2019-02-13  Tom Tromey  <tom@tromey.com>
> 
> 	* macrocmd.c (show_pp_source_pos): Style the file names.
> 
> gdb/testsuite/ChangeLog
> 2019-02-13  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.base/style.exp: Use -g3 to compile when possible.  Add test
> 	for macro styling.
> 	* gdb.base/style.c (SOME_MACRO): New macro.

Hi Tom. Looks good to me.

-- 
Joel