Re: ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output)

[Pedro Alves <palves@redhat.com>]

On 6/7/19 8:20 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> My patch also removes (really just comments out) %pS and %pN in favor of
> Tom> requiring %ps.  Those could be restored with a bit of effort I guess.
> 
> Tom> I think we do need these.
> 
> I pushed a couple more patches to the branch, including one intended to
> fix this.
> 
> What's left to do on that branch in order to merge it?

Well, you used printf_filtered the new patches, but that doesn't work
with the new format specifiers, yet.  :-)

 @@ -12112,23 +12112,21 @@ say_where (struct breakpoint *b)
    else
      {
        if (opts.addressprint || b->loc->symtab == NULL)
 -       {
 -         printf_filtered (" at ");
 -         fputs_styled (paddress (b->loc->gdbarch, b->loc->address),
 -                       address_style.style (),
 -                       gdb_stdout);
 -       }
 +       printf_filtered (" at %ps",
 +                        styled_string (address_style.style (),
 +                                       paddress (b->loc->gdbarch,
 +                                                 b->loc->address)).ptr ());

branch, buggy:

 (top-gdb) start
 Temporary breakpoint 3 at 0x7fffc8e67590s: file 0x7fffc8e675e0s, line 28.

fixed:

 (top-gdb) start
 Temporary breakpoint 3 at 0x469a06: file /home/pedro/gdb/mygit/src/gdb/gdb.c, line 28.


If we want the new new formatters to work with printf_(un)filtered,
the patch below fixes it by making printf_(un)filtered defer to
cli_ui_out::message for format processing, instead of to
string_printf (and thus to vsnprintf).  It's maybe a bit roundabout, but
I couldn't think of a better/easier way.

From de45ea7e7544f1cb3b7facc2fa47609006d1aeb0 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 28 Jun 2019 22:40:50 +0100
Subject: [PATCH] printf_filtered

---
 gdb/cli-out.c |  5 ++++-
 gdb/ui-out.c  | 14 ++++++++++----
 gdb/ui-out.h  |  1 +
 gdb/utils.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++---------
 gdb/utils.h   |  7 +++++++
 5 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index aaea20e1512..1f95e4b8ddd 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -209,7 +209,10 @@ cli_ui_out::do_message (const ui_file_style &style,
   if (m_suppress_output)
     return;
 
-  vfprintf_styled (m_streams.back (), style, format, args);
+  /* Use the "no_gdbfmt" variant here to avoid recursion.
+     vfprintf_styled calls into cli_ui_out::message to handle the
+     gdb-specific printf formats.  */
+  vfprintf_styled_no_gdbfmt (m_streams.back (), style, format, args);
 }
 
 void
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 4116c47ea43..d50fb3a3e3f 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -560,15 +560,12 @@ ui_out::call_do_message (const ui_file_style &style, const char *format,
 }
 
 void
-ui_out::message (const char *format, ...)
+ui_out::vmessage (const char *format, va_list args)
 {
   format_pieces fpieces (&format, true);
 
   ui_file_style style;
 
-  va_list args;
-  va_start (args, format);
-
   for (auto &&piece : fpieces)
     {
       const char *current_substring = piece.string;
@@ -647,6 +644,15 @@ ui_out::message (const char *format, ...)
 			  _("failed internal consistency check"));
 	}
     }
+}
+
+void
+ui_out::message (const char *format, ...)
+{
+  va_list args;
+  va_start (args, format);
+
+  vmessage (format, args);
 
   va_end (args);
 }
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index f4c3857fe2d..35bc8b21824 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -149,6 +149,7 @@ class ui_out
   void spaces (int numspaces);
   void text (const char *string);
   void message (const char *format, ...) ATTRIBUTE_PRINTF (2, 3);
+  void vmessage (const char *format, va_list args) ATTRIBUTE_PRINTF (2, 0);
   void wrap_hint (const char *identstring);
 
   void flush ();
diff --git a/gdb/utils.c b/gdb/utils.c
index 6ba4e41faa2..22badda7713 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -73,13 +73,15 @@
 #include "common/pathstuff.h"
 #include "cli/cli-style.h"
 #include "common/scope-exit.h"
+#include "cli-out.h"
 
 void (*deprecated_error_begin_hook) (void);
 
 /* Prototypes for local functions */
 
 static void vfprintf_maybe_filtered (struct ui_file *, const char *,
-				     va_list, int) ATTRIBUTE_PRINTF (2, 0);
+				     va_list, bool, bool)
+  ATTRIBUTE_PRINTF (2, 0);
 
 static void fputs_maybe_filtered (const char *, struct ui_file *, int);
 
@@ -2027,6 +2029,26 @@ puts_debug (char *prefix, char *string, char *suffix)
     }
 }
 
+/* Like string_vprintf, but if GDBFMT is true, also process the
+   gdb-specific format specifiers.  */
+
+static std::string string_vprintf_maybe_gdbfmt (const char *format,
+						va_list args,
+						bool gdbfmt)
+  ATTRIBUTE_PRINTF (1, 0);
+
+static std::string
+string_vprintf_maybe_gdbfmt (const char *format, va_list args, bool gdbfmt)
+{
+  if (gdbfmt)
+    {
+      string_file sfile;
+      cli_ui_out (&sfile, 0).vmessage (format, args);
+      return std::move (sfile.string ());
+    }
+  else
+    return string_vprintf (format, args);
+}
 
 /* Print a variable number of ARGS using format FORMAT.  If this
    information is going to put the amount written (since the last call
@@ -2038,15 +2060,14 @@ puts_debug (char *prefix, char *string, char *suffix)
    We implement three variants, vfprintf (takes a vararg list and stream),
    fprintf (takes a stream to write on), and printf (the usual).
 
-   Note also that a longjmp to top level may occur in this routine
-   (since prompt_for_continue may do so) so this routine should not be
-   called when cleanups are not in place.  */
+   Note also that this may throw a quit (since prompt_for_continue may
+   do so).  */
 
 static void
 vfprintf_maybe_filtered (struct ui_file *stream, const char *format,
-			 va_list args, int filter)
+			 va_list args, bool filter, bool gdbfmt)
 {
-  std::string linebuffer = string_vprintf (format, args);
+  std::string linebuffer = string_vprintf_maybe_gdbfmt (format, args, gdbfmt);
   fputs_maybe_filtered (linebuffer.c_str (), stream, filter);
 }
 
@@ -2054,13 +2075,19 @@ vfprintf_maybe_filtered (struct ui_file *stream, const char *format,
 void
 vfprintf_filtered (struct ui_file *stream, const char *format, va_list args)
 {
-  vfprintf_maybe_filtered (stream, format, args, 1);
+  vfprintf_maybe_filtered (stream, format, args, true, true);
+}
+
+static void
+vfprintf_filtered_no_gdbfmt (struct ui_file *stream, const char *format, va_list args)
+{
+  vfprintf_maybe_filtered (stream, format, args, true, false);
 }
 
 void
 vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
 {
-  std::string linebuffer = string_vprintf (format, args);
+  std::string linebuffer = string_vprintf_maybe_gdbfmt (format, args, true);
   if (debug_timestamp && stream == gdb_stdlog)
     {
       using namespace std::chrono;
@@ -2087,7 +2114,7 @@ vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
 void
 vprintf_filtered (const char *format, va_list args)
 {
-  vfprintf_maybe_filtered (gdb_stdout, format, args, 1);
+  vfprintf_maybe_filtered (gdb_stdout, format, args, true, false);
 }
 
 void
@@ -2158,6 +2185,17 @@ vfprintf_styled (struct ui_file *stream, const ui_file_style &style,
   set_output_style (stream, ui_file_style ());
 }
 
+/* See utils.h.  */
+
+void
+vfprintf_styled_no_gdbfmt (struct ui_file *stream, const ui_file_style &style,
+			   const char *format, va_list args)
+{
+  set_output_style (stream, style);
+  vfprintf_filtered_no_gdbfmt (stream, format, args);
+  set_output_style (stream, ui_file_style ());
+}
+
 void
 printf_filtered (const char *format, ...)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index bb73d4a8e72..61b7b5e3bb3 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -435,6 +435,13 @@ extern void vfprintf_styled (struct ui_file *stream,
 			     va_list args)
   ATTRIBUTE_PRINTF (3, 0);
 
+/* Like vfprintf_styled, but do not process gdb-specific format
+   specifiers.  */
+extern void vfprintf_styled_no_gdbfmt (struct ui_file *stream,
+				       const ui_file_style &style,
+				       const char *fmt, va_list args)
+  ATTRIBUTE_PRINTF (3, 0);
+
 /* Like fputs_filtered, but styles the output according to STYLE, when
    appropriate.  */
 
-- 
2.14.5