[PATCH] Remove ui_out::do_field_fmt

[PATCH] Remove ui_out::do_field_fmt

[Tom Tromey]

A user pointed out that certain MI output was not correctly quoted.
He tracked this down to mi_ui_out::do_field_fmt.

Looking into this, it seems to me that do_field_fmt is not needed at
all.  Instead, ui_out can handle the formatting, and then delegate to
do_field_string.

This approach fixes the original bug, because the MI implementation of
do_field_string does perform the quoting.

This also slips in a small change to use checked_static_cast in
mi_ui_out::main_stream.

Regression tested on x86-64 Fedora 43.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34599
---
 gdb/cli-out.c         | 15 ------------
 gdb/cli-out.h         |  4 ----
 gdb/mi/mi-out.c       | 54 +++++++++++++++++++++++++++++--------------
 gdb/mi/mi-out.h       |  4 ----
 gdb/python/py-mi.c    | 12 ----------
 gdb/python/py-uiout.h |  4 ----
 gdb/ui-out.c          | 11 ++++-----
 gdb/ui-out.h          |  4 ----
 8 files changed, 42 insertions(+), 66 deletions(-)

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index d8ac13ab827..0c339d0e819 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -187,21 +187,6 @@ cli_ui_out::do_field_string (int fldno, int width, ui_align align,
     field_separator ();
 }
 
-/* Output field containing ARGS using printf formatting in FORMAT.  */
-
-void
-cli_ui_out::do_field_fmt (int fldno, int width, ui_align align,
-			  const char *fldname, const ui_file_style &style,
-			  const char *format, va_list args)
-{
-  if (m_suppress_output)
-    return;
-
-  std::string str = string_vprintf (format, args);
-
-  do_field_string (fldno, width, align, fldname, str.c_str (), style);
-}
-
 void
 cli_ui_out::do_spaces (int numspaces)
 {
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index 74307074a7c..4c7ae6e656e 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -65,10 +65,6 @@ class cli_ui_out : public ui_out
 				const char *fldname,
 				const char *string,
 				const ui_file_style &style) override;
-  virtual void do_field_fmt (int fldno, int width, ui_align align,
-			     const char *fldname, const ui_file_style &style,
-			     const char *format, va_list args)
-    override ATTRIBUTE_PRINTF (7, 0);
   virtual void do_spaces (int numspaces) override;
   virtual void do_text (const char *string) override;
   virtual void do_message (ui_file_style &current_style,
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 41b0ee8cdbd..cba6acbbab5 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -27,6 +27,7 @@
 #include "ui-out.h"
 #include "utils.h"
 #include "gdbsupport/gdb-checked-static-cast.h"
+#include "gdbsupport/selftest.h"
 
 /* Mark beginning of a table.  */
 
@@ -140,22 +141,6 @@ mi_ui_out::do_field_string (int fldno, int width, ui_align align,
   gdb_printf (stream, "\"");
 }
 
-void
-mi_ui_out::do_field_fmt (int fldno, int width, ui_align align,
-			 const char *fldname, const ui_file_style &style,
-			 const char *format, va_list args)
-{
-  ui_file *stream = m_streams.back ();
-  field_separator ();
-
-  if (fldname)
-    gdb_printf (stream, "%s=\"", fldname);
-  else
-    gdb_puts ("\"", stream);
-  gdb_vprintf (stream, format, args);
-  gdb_puts ("\"", stream);
-}
-
 void
 mi_ui_out::do_spaces (int numspaces)
 {
@@ -257,7 +242,7 @@ mi_ui_out::main_stream ()
 {
   gdb_assert (m_streams.size () == 1);
 
-  return (string_file *) m_streams.back ();
+  return gdb::checked_static_cast<string_file *> (m_streams.back ());
 }
 
 /* Initialize a progress update to be displayed with
@@ -371,3 +356,38 @@ mi_out_rewind (ui_out *uiout)
 {
   return as_mi_ui_out (uiout)->rewind ();
 }
+
+#if GDB_SELF_TEST
+
+namespace selftests
+{
+
+static void
+mi_check (mi_ui_out &uiout, const char *expected)
+{
+  string_file *stream
+    = gdb::checked_static_cast<string_file *> (uiout.current_stream ());
+  SELF_CHECK (stream->string () == expected);
+  uiout.rewind ();
+}
+
+static void
+test_mi_out ()
+{
+  /* The version doesn't matter for these tests.  */
+  mi_ui_out uiout (2);
+
+  uiout.field_fmt ("field", "%s", "test \"quoted\"");
+  mi_check (uiout, ",field=\"test \\\"quoted\\\"\"");
+}
+
+}
+
+#endif
+
+INIT_GDB_FILE (mi_out)
+{
+#if GDB_SELF_TEST
+  selftests::register_test ("mi-out", selftests::test_mi_out);
+#endif /* GDB_SELF_TEST */
+}
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index d1d26718ac4..ce1e5dadc4c 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -72,10 +72,6 @@ class mi_ui_out : public ui_out
   virtual void do_field_string (int fldno, int width, ui_align align,
 				const char *fldname, const char *string,
 				const ui_file_style &style) override;
-  virtual void do_field_fmt (int fldno, int width, ui_align align,
-			     const char *fldname, const ui_file_style &style,
-			     const char *format, va_list args)
-    override ATTRIBUTE_PRINTF (7,0);
   virtual void do_spaces (int numspaces) override;
   virtual void do_text (const char *string) override;
   virtual void do_message (ui_file_style &current_style,
diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 73daefecb20..7e8105fdade 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -119,18 +119,6 @@ py_ui_out::do_field_string (int fldno, int width, ui_align align,
   add_field (fldname, val);
 }
 
-void
-py_ui_out::do_field_fmt (int fldno, int width, ui_align align,
-			 const char *fldname, const ui_file_style &style,
-			 const char *format, va_list args)
-{
-  if (m_error.has_value ())
-    return;
-
-  std::string str = string_vprintf (format, args);
-  do_field_string (fldno, width, align, fldname, str.c_str (), style);
-}
-
 /* Implementation of the gdb.execute_mi command.  */
 
 PyObject *
diff --git a/gdb/python/py-uiout.h b/gdb/python/py-uiout.h
index 21c068e9077..5a1ee9c09b3 100644
--- a/gdb/python/py-uiout.h
+++ b/gdb/python/py-uiout.h
@@ -98,10 +98,6 @@ class py_ui_out : public ui_out
   void do_field_string (int fldno, int width, ui_align align,
 			const char *fldname, const char *string,
 			const ui_file_style &style) override;
-  void do_field_fmt (int fldno, int width, ui_align align,
-		     const char *fldname, const ui_file_style &style,
-		     const char *format, va_list args) override
-    ATTRIBUTE_PRINTF (7, 0);
 
   void do_spaces (int numspaces) override
   { }
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index ac792b43905..45bb1d821a1 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -534,9 +534,9 @@ ui_out::field_fmt (const char *fldname, const char *format, ...)
   verify_field (&fldno, &width, &align);
 
   va_start (args, format);
-
-  do_field_fmt (fldno, width, align, fldname, ui_file_style (), format, args);
-
+  std::string str = string_vprintf (format, args);
+  do_field_string (fldno, width, align, fldname, str.c_str (),
+		   ui_file_style ());
   va_end (args);
 }
 
@@ -552,9 +552,8 @@ ui_out::field_fmt (const char *fldname, const ui_file_style &style,
   verify_field (&fldno, &width, &align);
 
   va_start (args, format);
-
-  do_field_fmt (fldno, width, align, fldname, style, format, args);
-
+  std::string str = string_vprintf (format, args);
+  do_field_string (fldno, width, align, fldname, str.c_str (), style);
   va_end (args);
 }
 
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 0c82e82bf6e..26d4a6b7d0e 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -360,10 +360,6 @@ class ui_out
   virtual void do_field_string (int fldno, int width, ui_align align,
 				const char *fldname, const char *string,
 				const ui_file_style &style) = 0;
-  virtual void do_field_fmt (int fldno, int width, ui_align align,
-			     const char *fldname, const ui_file_style &style,
-			     const char *format, va_list args)
-    ATTRIBUTE_PRINTF (7, 0) = 0;
   virtual void do_spaces (int numspaces) = 0;
   virtual void do_text (const char *string) = 0;
 

base-commit: 0595410d007e5f628e14717cd268e557e166675f
-- 
2.55.0

Re: [PATCH] Remove ui_out::do_field_fmt

[Christian Walther]

On 17 Sep 2026, at 18:16, Tom Tromey <tromey@adacore.com> wrote:
> A user pointed out that certain MI output was not correctly quoted.
> He tracked this down to mi_ui_out::do_field_fmt.
> 
> Looking into this, it seems to me that do_field_fmt is not needed at
> all.  Instead, ui_out can handle the formatting, and then delegate to
> do_field_string.
> 
> This approach fixes the original bug, because the MI implementation of
> do_field_string does perform the quoting.
> 
> This also slips in a small change to use checked_static_cast in
> mi_ui_out::main_stream.
> 
> Regression tested on x86-64 Fedora 43.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34599

Looks good to me, as far as I can tell as an outsider, and does fix my issue. Thanks!

  -Christian