diff mbox

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

Message ID ef92b0b9-11cc-57ce-9253-157161d58860@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves July 1, 2019, 12:23 p.m. UTC 
  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(-)

Comments

Pedro Alves July 1, 2019, 12:55 p.m. UTC | #1 
On 7/1/19 1:23 PM, Pedro Alves wrote:
>> 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.  :-)

There are two other things that I wanted to experiment/try first.

- See if we could get rid of the need for the ".ptr ()" calls.

- See about implementing support for some ui field type other than
  integers, to make sure that we're not missing something.

I'll send some patches for both of these things in reply to this
email.

Thanks,
Pedro Alves
Tom Tromey July 1, 2019, 5:42 p.m. UTC | #2 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

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

Oops.  I meant not to do that, but of course made a mistake.

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

We should probably do this, just to avoid future mistakes along these
same lines.

Tom
Pedro Alves July 1, 2019, 7:29 p.m. UTC | #3 
On 7/1/19 6:42 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Well, you used printf_filtered the new patches, but that doesn't work
> Pedro> with the new format specifiers, yet.  :-)
> 
> Oops.  I meant not to do that, but of course made a mistake.
> 
> Pedro> If we want the new new formatters to work with printf_(un)filtered,
> Pedro> the patch below fixes it by making printf_(un)filtered defer to
> Pedro> cli_ui_out::message for format processing, instead of to
> Pedro> string_printf (and thus to vsnprintf).  It's maybe a bit roundabout, but
> Pedro> I couldn't think of a better/easier way.
> 
> We should probably do this, just to avoid future mistakes along these
> same lines.
> 

I merged this one to the branch as well.

Thanks,
Pedro Alves
diff mbox

Patch

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.  */