[PATCH] Implement putstr and putstrn in ui_file

[PATCH] Implement putstr and putstrn in ui_file

[Tom Tromey]

In my tour of the ui_file subsystem, I found that fputstr and fputstrn
can be simplified.  The _filtered forms are never used (and IMO
unlikely to ever be used) and so can be removed.  And, the interface
can be simplified by removing a callback function and moving the
implementation directly to ui_file.

Regression tested on x86-64 Fedora 34.
---
 gdb/guile/scm-ports.c |  2 +-
 gdb/mi/mi-console.c   | 20 ++-------
 gdb/mi/mi-main.c      |  2 +-
 gdb/mi/mi-out.c       |  2 +-
 gdb/ui-file.c         | 69 ++++++++++++++++++++++++++++--
 gdb/ui-file.h         | 21 +++++++++-
 gdb/utils.c           | 97 -------------------------------------------
 gdb/utils.h           | 15 -------
 8 files changed, 93 insertions(+), 135 deletions(-)

diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 41ccf30202d..2d3f36bee36 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -191,7 +191,7 @@ ioscm_open_port (scm_t_port_type *port_type, long mode_bits, scm_t_bits stream)
 \f
 /* Support for connecting Guile's stdio ports to GDB's stdio ports.  */
 
-/* Like fputstrn_filtered, but don't escape characters, except nul.
+/* Print a string, but don't escape characters, except nul.
    Also like fputs_filtered, but a length is specified.  */
 
 static void
diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c
index be0129d3a88..10e496e1e14 100644
--- a/gdb/mi/mi-console.c
+++ b/gdb/mi/mi-console.c
@@ -48,16 +48,6 @@ mi_console_file::write (const char *buf, long length_buf)
     this->flush ();
 }
 
-/* Write C to STREAM's in an async-safe way.  */
-
-static int
-do_fputc_async_safe (int c, ui_file *stream)
-{
-  char ch = c;
-  stream->write_async_safe (&ch, 1);
-  return c;
-}
-
 void
 mi_console_file::write_async_safe (const char *buf, long length_buf)
 {
@@ -65,12 +55,11 @@ mi_console_file::write_async_safe (const char *buf, long length_buf)
   if (m_quote)
     {
       m_raw->write_async_safe (&m_quote, 1);
-      fputstrn_unfiltered (buf, length_buf, m_quote, do_fputc_async_safe,
-			   m_raw);
+      m_raw->putstrn (buf, length_buf, m_quote, true);
       m_raw->write_async_safe (&m_quote, 1);
     }
   else
-    fputstrn_unfiltered (buf, length_buf, 0, do_fputc_async_safe, m_raw);
+    m_raw->putstrn (buf, length_buf, 0, true);
 
   char nl = '\n';
   m_raw->write_async_safe (&nl, 1);
@@ -91,14 +80,13 @@ mi_console_file::flush ()
       if (m_quote)
 	{
 	  fputc_unfiltered (m_quote, m_raw);
-	  fputstrn_unfiltered (buf, length_buf, m_quote, fputc_unfiltered,
-			       m_raw);
+	  m_raw->putstrn (buf, length_buf, m_quote);
 	  fputc_unfiltered (m_quote, m_raw);
 	  fputc_unfiltered ('\n', m_raw);
 	}
       else
 	{
-	  fputstrn_unfiltered (buf, length_buf, 0, fputc_unfiltered, m_raw);
+	  m_raw->putstrn (buf, length_buf, 0);
 	  fputc_unfiltered ('\n', m_raw);
 	}
       gdb_flush (m_raw);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 1f210c161e6..9f1a0c3ddee 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1866,7 +1866,7 @@ mi_print_exception (const char *token, const struct gdb_exception &exception)
   if (exception.message == NULL)
     fputs_unfiltered ("unknown error", mi->raw_stdout);
   else
-    fputstr_unfiltered (exception.what (), '"', mi->raw_stdout);
+    mi->raw_stdout->putstr (exception.what (), '"');
   fputs_unfiltered ("\"", mi->raw_stdout);
 
   switch (exception.error)
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 0b223476167..af5f4ee007c 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -135,7 +135,7 @@ mi_ui_out::do_field_string (int fldno, int width, ui_align align,
     fprintf_unfiltered (stream, "%s=", fldname);
   fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', stream);
+    stream->putstr (string, '"');
   fprintf_unfiltered (stream, "\"");
 }
 
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index eb1d72bf8b3..f76f71aea03 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -47,13 +47,15 @@ ui_file::printf (const char *format, ...)
 void
 ui_file::putstr (const char *str, int quoter)
 {
-  fputstr_unfiltered (str, quoter, this);
+  while (*str)
+    printchar (*str++, quoter, false);
 }
 
 void
-ui_file::putstrn (const char *str, int n, int quoter)
+ui_file::putstrn (const char *str, int n, int quoter, bool async_safe)
 {
-  fputstrn_unfiltered (str, n, quoter, fputc_unfiltered, this);
+  for (int i = 0; i < n; i++)
+    printchar (str[i], quoter, async_safe);
 }
 
 int
@@ -68,6 +70,67 @@ ui_file::vprintf (const char *format, va_list args)
   vfprintf_unfiltered (this, format, args);
 }
 
+/* See ui-file.h.  */
+
+void
+ui_file::printchar (int c, int quoter, bool async_safe)
+{
+  char buf[4];
+  int out = 0;
+
+  c &= 0xFF;			/* Avoid sign bit follies */
+
+  if (c < 0x20 ||		/* Low control chars */
+      (c >= 0x7F && c < 0xA0) ||	/* DEL, High controls */
+      (sevenbit_strings && c >= 0x80))
+    {				/* high order bit set */
+      buf[out++] = '\\';
+
+      switch (c)
+	{
+	case '\n':
+	  buf[out++] = 'n';
+	  break;
+	case '\b':
+	  buf[out++] = 'b';
+	  break;
+	case '\t':
+	  buf[out++] = 't';
+	  break;
+	case '\f':
+	  buf[out++] = 'f';
+	  break;
+	case '\r':
+	  buf[out++] = 'r';
+	  break;
+	case '\033':
+	  buf[out++] = 'e';
+	  break;
+	case '\007':
+	  buf[out++] = 'a';
+	  break;
+	default:
+	  {
+	    buf[out++] = '0';
+	    buf[out++] = '0';
+	    buf[out++] = '0';
+	    break;
+	  }
+	}
+    }
+  else
+    {
+      if (quoter != 0 && (c == '\\' || c == quoter))
+	buf[out++] = '\\';
+      buf[out++] = c;
+    }
+
+  if (async_safe)
+    this->write_async_safe (buf, out);
+  else
+    this->write (buf, out);
+}
+
 \f
 
 void
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 0faf84996aa..a4448d8ea9f 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -39,7 +39,10 @@ class ui_file
      independent of the language of the program being debugged.  */
   void putstr (const char *str, int quoter);
 
-  void putstrn (const char *str, int n, int quoter);
+  /* Like putstr, but only print the first N characters.  If
+     ASYNC_SAFE is true, then the output is done via the
+     write_async_safe method.  */
+  void putstrn (const char *str, int n, int quoter, bool async_safe = false);
 
   int putc (int c);
 
@@ -96,6 +99,22 @@ class ui_file
        default.  */
     return false;
   }
+
+private:
+
+  /* Helper function for putstr and putstrn.
+
+     Print the character C on this stream as part of the contents of a
+     literal string whose delimiter is QUOTER.  Note that this routine
+     should only be called for printing things which are independent
+     of the language of the program being debugged.
+
+     printchar will normally escape backslashes and instances of
+     QUOTER. If QUOTER is 0, printchar won't escape backslashes or any
+     quoting character.  As a side effect, if you pass the backslash
+     character as the QUOTER, printchar will escape backslashes as
+     usual, but not any other quoting character. */
+  void printchar (int c, int quoter, bool async_safe);
 };
 
 typedef std::unique_ptr<ui_file> ui_file_up;
diff --git a/gdb/utils.c b/gdb/utils.c
index f8c898dd502..a28503e4853 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1129,103 +1129,6 @@ parse_escape (struct gdbarch *gdbarch, const char **string_ptr)
   return target_char;
 }
 \f
-/* Print the character C on STREAM as part of the contents of a literal
-   string whose delimiter is QUOTER.  Note that this routine should only
-   be called for printing things which are independent of the language
-   of the program being debugged.
-
-   printchar will normally escape backslashes and instances of QUOTER. If
-   QUOTER is 0, printchar won't escape backslashes or any quoting character.
-   As a side effect, if you pass the backslash character as the QUOTER,
-   printchar will escape backslashes as usual, but not any other quoting
-   character. */
-
-static void
-printchar (int c, do_fputc_ftype do_fputc, ui_file *stream, int quoter)
-{
-  c &= 0xFF;			/* Avoid sign bit follies */
-
-  if (c < 0x20 ||		/* Low control chars */
-      (c >= 0x7F && c < 0xA0) ||	/* DEL, High controls */
-      (sevenbit_strings && c >= 0x80))
-    {				/* high order bit set */
-      do_fputc ('\\', stream);
-
-      switch (c)
-	{
-	case '\n':
-	  do_fputc ('n', stream);
-	  break;
-	case '\b':
-	  do_fputc ('b', stream);
-	  break;
-	case '\t':
-	  do_fputc ('t', stream);
-	  break;
-	case '\f':
-	  do_fputc ('f', stream);
-	  break;
-	case '\r':
-	  do_fputc ('r', stream);
-	  break;
-	case '\033':
-	  do_fputc ('e', stream);
-	  break;
-	case '\007':
-	  do_fputc ('a', stream);
-	  break;
-	default:
-	  {
-	    do_fputc ('0' + ((c >> 6) & 0x7), stream);
-	    do_fputc ('0' + ((c >> 3) & 0x7), stream);
-	    do_fputc ('0' + ((c >> 0) & 0x7), stream);
-	    break;
-	  }
-	}
-    }
-  else
-    {
-      if (quoter != 0 && (c == '\\' || c == quoter))
-	do_fputc ('\\', stream);
-      do_fputc (c, stream);
-    }
-}
-
-/* Print the character C on STREAM as part of the contents of a
-   literal string whose delimiter is QUOTER.  Note that these routines
-   should only be call for printing things which are independent of
-   the language of the program being debugged.  */
-
-void
-fputstr_filtered (const char *str, int quoter, struct ui_file *stream)
-{
-  while (*str)
-    printchar (*str++, fputc_filtered, stream, quoter);
-}
-
-void
-fputstr_unfiltered (const char *str, int quoter, struct ui_file *stream)
-{
-  while (*str)
-    printchar (*str++, fputc_unfiltered, stream, quoter);
-}
-
-void
-fputstrn_filtered (const char *str, int n, int quoter,
-		   struct ui_file *stream)
-{
-  for (int i = 0; i < n; i++)
-    printchar (str[i], fputc_filtered, stream, quoter);
-}
-
-void
-fputstrn_unfiltered (const char *str, int n, int quoter,
-		     do_fputc_ftype do_fputc, struct ui_file *stream)
-{
-  for (int i = 0; i < n; i++)
-    printchar (str[i], do_fputc, stream, quoter);
-}
-\f
 
 /* Number of lines per page or UINT_MAX if paging is disabled.  */
 static unsigned int lines_per_page;
diff --git a/gdb/utils.h b/gdb/utils.h
index 54cf090974a..d199e1f0d15 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -464,21 +464,6 @@ extern void print_spaces_filtered (int, struct ui_file *);
 
 extern const char *n_spaces (int);
 
-extern void fputstr_filtered (const char *str, int quotr,
-			      struct ui_file * stream);
-
-extern void fputstr_unfiltered (const char *str, int quotr,
-				struct ui_file * stream);
-
-extern void fputstrn_filtered (const char *str, int n, int quotr,
-			       struct ui_file * stream);
-
-typedef int (*do_fputc_ftype) (int c, ui_file *stream);
-
-extern void fputstrn_unfiltered (const char *str, int n, int quotr,
-				 do_fputc_ftype do_fputc,
-				 struct ui_file * stream);
-
 /* Return nonzero if filtered printing is initialized.  */
 extern int filtered_printing_initialized (void);
 
-- 
2.31.1

Re: [PATCH] Implement putstr and putstrn in ui_file

[Lancelot SIX]

Hi,

I have included comments and questions below.

> +/* See ui-file.h.  */
> +
> +void
> +ui_file::printchar (int c, int quoter, bool async_safe)
> +{
> +  char buf[4];
> +  int out = 0;
> +
> +  c &= 0xFF;			/* Avoid sign bit follies */
> +
> +  if (c < 0x20 ||		/* Low control chars */
> +      (c >= 0x7F && c < 0xA0) ||	/* DEL, High controls */
> +      (sevenbit_strings && c >= 0x80))
> +    {				/* high order bit set */
> +      buf[out++] = '\\';
> +
> +      switch (c)
> +	{
> +	case '\n':
> +	  buf[out++] = 'n';
> +	  break;
> +	case '\b':
> +	  buf[out++] = 'b';
> +	  break;
> +	case '\t':
> +	  buf[out++] = 't';
> +	  break;
> +	case '\f':
> +	  buf[out++] = 'f';
> +	  break;
> +	case '\r':
> +	  buf[out++] = 'r';
> +	  break;
> +	case '\033':
> +	  buf[out++] = 'e';
> +	  break;
> +	case '\007':
> +	  buf[out++] = 'a';
> +	  break;
> +	default:
> +	  {
> +	    buf[out++] = '0';
> +	    buf[out++] = '0';
> +	    buf[out++] = '0';

This is different from the printchar implementation you moved (and it
seems that always printing '\000' is a loss of information).  Maybe you
forgot to finalize this part?


> +	    break;
> +	  }
> +	}
> +    }
> +  else
> +    {
> +      if (quoter != 0 && (c == '\\' || c == quoter))
> +	buf[out++] = '\\';
> +      buf[out++] = c;
> +    }
> +
> +  if (async_safe)
> +    this->write_async_safe (buf, out);
> +  else
> +    this->write (buf, out);
> +}
> +
>  \f
>  
>  void
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 0faf84996aa..a4448d8ea9f 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -39,7 +39,10 @@ class ui_file
>       independent of the language of the program being debugged.  */
>    void putstr (const char *str, int quoter);
>  
> -  void putstrn (const char *str, int n, int quoter);
> +  /* Like putstr, but only print the first N characters.  If

I find the comment slightly misleading (or incomplete).  The function
prints exactly N chars.  So if 'strlen (str) < n', putstrn prints
more stuff than putstrn.

Maybe “Like putstr, but print exactly N characters, starting at the
position pointed by STR”, or something similar?

> +     ASYNC_SAFE is true, then the output is done via the
> +     write_async_safe method.  */
> +  void putstrn (const char *str, int n, int quoter, bool async_safe = false);
>  
>    int putc (int c);
>  
> @@ -96,6 +99,22 @@ class ui_file
>         default.  */
>      return false;
>    }
> +
> +private:
> +
> +  /* Helper function for putstr and putstrn.
> +
> +     Print the character C on this stream as part of the contents of a
> +     literal string whose delimiter is QUOTER.  Note that this routine
> +     should only be called for printing things which are independent
> +     of the language of the program being debugged.
> +
> +     printchar will normally escape backslashes and instances of
> +     QUOTER. If QUOTER is 0, printchar won't escape backslashes or any
> +     quoting character.  As a side effect, if you pass the backslash
> +     character as the QUOTER, printchar will escape backslashes as
> +     usual, but not any other quoting character. */
                                                   ^
You are missing one space after the '.'.

Best,
Lancelot.

Re: [PATCH] Implement putstr and putstrn in ui_file

[Tom Tromey]

Lancelot> I have included comments and questions below.

Thanks for the review.

>> +	    buf[out++] = '0';
>> +	    buf[out++] = '0';
>> +	    buf[out++] = '0';

Lancelot> This is different from the printchar implementation you moved (and it
Lancelot> seems that always printing '\000' is a loss of information).  Maybe you
Lancelot> forgot to finalize this part?

Yikes, I certainly did.  This means that there aren't any tests for
this, so in v2 of the patch (appended) I added a self-test.

Lancelot> I find the comment slightly misleading (or incomplete).  The function
Lancelot> prints exactly N chars.  So if 'strlen (str) < n', putstrn prints
Lancelot> more stuff than putstrn.

Lancelot> Maybe “Like putstr, but print exactly N characters, starting at the
Lancelot> position pointed by STR”, or something similar?

I rewrote this to try to clarify it, and also rearranged so that the
more detailed comment from printchar is now by putstr.

>> +     usual, but not any other quoting character. */

Lancelot> You are missing one space after the '.'.

Fixed (well, avoided) by the comment rearrangement.

Tom

commit a87c1495874162fa41171ff8a43dcfa1ce2e48e2
Author: Tom Tromey <tom@tromey.com>
Date:   Fri Dec 31 10:40:02 2021 -0700

    Implement putstr and putstrn in ui_file
    
    In my tour of the ui_file subsystem, I found that fputstr and fputstrn
    can be simplified.  The _filtered forms are never used (and IMO
    unlikely to ever be used) and so can be removed.  And, the interface
    can be simplified by removing a callback function and moving the
    implementation directly to ui_file.
    
    A new self-test is included.  Previously, I think nothing was testing
    this code.
    
    Regression tested on x86-64 Fedora 34.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8cd97475e86..d0db5fbdee1 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -477,6 +477,7 @@ SELFTESTS_SRCS = \
 	unittests/style-selftests.c \
 	unittests/tracepoint-selftests.c \
 	unittests/tui-selftests.c \
+	unittests/ui-file-selftests.c \
 	unittests/unpack-selftests.c \
 	unittests/utils-selftests.c \
 	unittests/vec-utils-selftests.c \
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index bc6266ae055..9ce9cb4f678 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -191,7 +191,7 @@ ioscm_open_port (scm_t_port_type *port_type, long mode_bits, scm_t_bits stream)
 \f
 /* Support for connecting Guile's stdio ports to GDB's stdio ports.  */
 
-/* Like fputstrn_filtered, but don't escape characters, except nul.
+/* Print a string, but don't escape characters, except nul.
    Also like fputs_filtered, but a length is specified.  */
 
 static void
diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c
index 157154b361c..9db87fe180c 100644
--- a/gdb/mi/mi-console.c
+++ b/gdb/mi/mi-console.c
@@ -48,16 +48,6 @@ mi_console_file::write (const char *buf, long length_buf)
     this->flush ();
 }
 
-/* Write C to STREAM's in an async-safe way.  */
-
-static int
-do_fputc_async_safe (int c, ui_file *stream)
-{
-  char ch = c;
-  stream->write_async_safe (&ch, 1);
-  return c;
-}
-
 void
 mi_console_file::write_async_safe (const char *buf, long length_buf)
 {
@@ -65,12 +55,11 @@ mi_console_file::write_async_safe (const char *buf, long length_buf)
   if (m_quote)
     {
       m_raw->write_async_safe (&m_quote, 1);
-      fputstrn_unfiltered (buf, length_buf, m_quote, do_fputc_async_safe,
-			   m_raw);
+      m_raw->putstrn (buf, length_buf, m_quote, true);
       m_raw->write_async_safe (&m_quote, 1);
     }
   else
-    fputstrn_unfiltered (buf, length_buf, 0, do_fputc_async_safe, m_raw);
+    m_raw->putstrn (buf, length_buf, 0, true);
 
   char nl = '\n';
   m_raw->write_async_safe (&nl, 1);
@@ -91,14 +80,13 @@ mi_console_file::flush ()
       if (m_quote)
 	{
 	  fputc_unfiltered (m_quote, m_raw);
-	  fputstrn_unfiltered (buf, length_buf, m_quote, fputc_unfiltered,
-			       m_raw);
+	  m_raw->putstrn (buf, length_buf, m_quote);
 	  fputc_unfiltered (m_quote, m_raw);
 	  fputc_unfiltered ('\n', m_raw);
 	}
       else
 	{
-	  fputstrn_unfiltered (buf, length_buf, 0, fputc_unfiltered, m_raw);
+	  m_raw->putstrn (buf, length_buf, 0);
 	  fputc_unfiltered ('\n', m_raw);
 	}
       gdb_flush (m_raw);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e729a861c61..4860da7536a 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1866,7 +1866,7 @@ mi_print_exception (const char *token, const struct gdb_exception &exception)
   if (exception.message == NULL)
     fputs_unfiltered ("unknown error", mi->raw_stdout);
   else
-    fputstr_unfiltered (exception.what (), '"', mi->raw_stdout);
+    mi->raw_stdout->putstr (exception.what (), '"');
   fputs_unfiltered ("\"", mi->raw_stdout);
 
   switch (exception.error)
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index e9a44437a90..20c6f0f9194 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -135,7 +135,7 @@ mi_ui_out::do_field_string (int fldno, int width, ui_align align,
     fprintf_unfiltered (stream, "%s=", fldname);
   fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', stream);
+    stream->putstr (string, '"');
   fprintf_unfiltered (stream, "\"");
 }
 
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 57352e44a34..c6a4888ed48 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -47,13 +47,15 @@ ui_file::printf (const char *format, ...)
 void
 ui_file::putstr (const char *str, int quoter)
 {
-  fputstr_unfiltered (str, quoter, this);
+  while (*str)
+    printchar (*str++, quoter, false);
 }
 
 void
-ui_file::putstrn (const char *str, int n, int quoter)
+ui_file::putstrn (const char *str, int n, int quoter, bool async_safe)
 {
-  fputstrn_unfiltered (str, n, quoter, fputc_unfiltered, this);
+  for (int i = 0; i < n; i++)
+    printchar (str[i], quoter, async_safe);
 }
 
 int
@@ -68,6 +70,67 @@ ui_file::vprintf (const char *format, va_list args)
   vfprintf_unfiltered (this, format, args);
 }
 
+/* See ui-file.h.  */
+
+void
+ui_file::printchar (int c, int quoter, bool async_safe)
+{
+  char buf[4];
+  int out = 0;
+
+  c &= 0xFF;			/* Avoid sign bit follies */
+
+  if (c < 0x20			 /* Low control chars */
+      || (c >= 0x7F && c < 0xA0) /* DEL, High controls */
+      || (sevenbit_strings && c >= 0x80))
+    {				/* high order bit set */
+      buf[out++] = '\\';
+
+      switch (c)
+	{
+	case '\n':
+	  buf[out++] = 'n';
+	  break;
+	case '\b':
+	  buf[out++] = 'b';
+	  break;
+	case '\t':
+	  buf[out++] = 't';
+	  break;
+	case '\f':
+	  buf[out++] = 'f';
+	  break;
+	case '\r':
+	  buf[out++] = 'r';
+	  break;
+	case '\033':
+	  buf[out++] = 'e';
+	  break;
+	case '\007':
+	  buf[out++] = 'a';
+	  break;
+	default:
+	  {
+	    buf[out++] = '0' + ((c >> 6) & 0x7);
+	    buf[out++] = '0' + ((c >> 3) & 0x7);
+	    buf[out++] = '0' + ((c >> 0) & 0x7);
+	    break;
+	  }
+	}
+    }
+  else
+    {
+      if (quoter != 0 && (c == '\\' || c == quoter))
+	buf[out++] = '\\';
+      buf[out++] = c;
+    }
+
+  if (async_safe)
+    this->write_async_safe (buf, out);
+  else
+    this->write (buf, out);
+}
+
 \f
 
 void
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 89f249ead5e..c097abf0c29 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -34,12 +34,22 @@ class ui_file
 
   void printf (const char *, ...) ATTRIBUTE_PRINTF (2, 3);
 
-  /* Print a string whose delimiter is QUOTER.  Note that these
-     routines should only be called for printing things which are
-     independent of the language of the program being debugged.  */
+  /* Print a NUL-terminated string whose delimiter is QUOTER.  Note
+     that these routines should only be called for printing things
+     which are independent of the language of the program being
+     debugged.
+
+     This will normally escape backslashes and instances of QUOTER.
+     If QUOTER is 0, it won't escape backslashes or any quoting
+     character.  As a side effect, if you pass the backslash character
+     as the QUOTER, this will escape backslashes as usual, but not any
+     other quoting character.  */
   void putstr (const char *str, int quoter);
 
-  void putstrn (const char *str, int n, int quoter);
+  /* Like putstr, but only print the first N characters of STR.  If
+     ASYNC_SAFE is true, then the output is done via the
+     write_async_safe method.  */
+  void putstrn (const char *str, int n, int quoter, bool async_safe = false);
 
   int putc (int c);
 
@@ -96,6 +106,13 @@ class ui_file
        default.  */
     return false;
   }
+
+private:
+
+  /* Helper function for putstr and putstrn.  Print the character C on
+     this stream as part of the contents of a literal string whose
+     delimiter is QUOTER.  */
+  void printchar (int c, int quoter, bool async_safe);
 };
 
 typedef std::unique_ptr<ui_file> ui_file_up;
diff --git a/gdb/unittests/ui-file-selftests.c b/gdb/unittests/ui-file-selftests.c
new file mode 100644
index 00000000000..55854386169
--- /dev/null
+++ b/gdb/unittests/ui-file-selftests.c
@@ -0,0 +1,62 @@
+/* Self tests for ui_file
+
+   Copyright (C) 2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "gdbsupport/selftest.h"
+#include "ui-file.h"
+
+namespace selftests {
+namespace file {
+
+static void
+check_one (const char *str, int quoter, const char *result)
+{
+  string_file out;
+  out.putstr (str, quoter);
+  SELF_CHECK (out.string () == result);
+}
+
+static void
+run_tests ()
+{
+  check_one ("basic stuff: \\", '\\',
+	     "basic stuff: \\\\");
+  check_one ("more basic stuff: \\Q", 'Q',
+	     "more basic stuff: \\\\\\Q");
+  check_one ("more basic stuff: \\Q", '\0',
+	     "more basic stuff: \\Q");
+
+  check_one ("weird stuff: \x1f\x90\n\b\t\f\r\033\007", '\\',
+	     "weird stuff: \\037\\220\\n\\b\\t\\f\\r\\e\\a");
+
+  scoped_restore save_7 = make_scoped_restore (&sevenbit_strings, true);
+  check_one ("more weird stuff: \xa5", '\\',
+	     "more weird stuff: \\245");
+}
+
+} /* namespace file*/
+} /* namespace selftests */
+
+void _initialize_ui_file_selftest ();
+void
+_initialize_ui_file_selftest ()
+{
+  selftests::register_test ("ui-file",
+			    selftests::file::run_tests);
+}
diff --git a/gdb/utils.c b/gdb/utils.c
index 11f88d6f991..91662dc5fd8 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1129,103 +1129,6 @@ parse_escape (struct gdbarch *gdbarch, const char **string_ptr)
   return target_char;
 }
 \f
-/* Print the character C on STREAM as part of the contents of a literal
-   string whose delimiter is QUOTER.  Note that this routine should only
-   be called for printing things which are independent of the language
-   of the program being debugged.
-
-   printchar will normally escape backslashes and instances of QUOTER. If
-   QUOTER is 0, printchar won't escape backslashes or any quoting character.
-   As a side effect, if you pass the backslash character as the QUOTER,
-   printchar will escape backslashes as usual, but not any other quoting
-   character. */
-
-static void
-printchar (int c, do_fputc_ftype do_fputc, ui_file *stream, int quoter)
-{
-  c &= 0xFF;			/* Avoid sign bit follies */
-
-  if (c < 0x20 ||		/* Low control chars */
-      (c >= 0x7F && c < 0xA0) ||	/* DEL, High controls */
-      (sevenbit_strings && c >= 0x80))
-    {				/* high order bit set */
-      do_fputc ('\\', stream);
-
-      switch (c)
-	{
-	case '\n':
-	  do_fputc ('n', stream);
-	  break;
-	case '\b':
-	  do_fputc ('b', stream);
-	  break;
-	case '\t':
-	  do_fputc ('t', stream);
-	  break;
-	case '\f':
-	  do_fputc ('f', stream);
-	  break;
-	case '\r':
-	  do_fputc ('r', stream);
-	  break;
-	case '\033':
-	  do_fputc ('e', stream);
-	  break;
-	case '\007':
-	  do_fputc ('a', stream);
-	  break;
-	default:
-	  {
-	    do_fputc ('0' + ((c >> 6) & 0x7), stream);
-	    do_fputc ('0' + ((c >> 3) & 0x7), stream);
-	    do_fputc ('0' + ((c >> 0) & 0x7), stream);
-	    break;
-	  }
-	}
-    }
-  else
-    {
-      if (quoter != 0 && (c == '\\' || c == quoter))
-	do_fputc ('\\', stream);
-      do_fputc (c, stream);
-    }
-}
-
-/* Print the character C on STREAM as part of the contents of a
-   literal string whose delimiter is QUOTER.  Note that these routines
-   should only be call for printing things which are independent of
-   the language of the program being debugged.  */
-
-void
-fputstr_filtered (const char *str, int quoter, struct ui_file *stream)
-{
-  while (*str)
-    printchar (*str++, fputc_filtered, stream, quoter);
-}
-
-void
-fputstr_unfiltered (const char *str, int quoter, struct ui_file *stream)
-{
-  while (*str)
-    printchar (*str++, fputc_unfiltered, stream, quoter);
-}
-
-void
-fputstrn_filtered (const char *str, int n, int quoter,
-		   struct ui_file *stream)
-{
-  for (int i = 0; i < n; i++)
-    printchar (str[i], fputc_filtered, stream, quoter);
-}
-
-void
-fputstrn_unfiltered (const char *str, int n, int quoter,
-		     do_fputc_ftype do_fputc, struct ui_file *stream)
-{
-  for (int i = 0; i < n; i++)
-    printchar (str[i], do_fputc, stream, quoter);
-}
-\f
 
 /* Number of lines per page or UINT_MAX if paging is disabled.  */
 static unsigned int lines_per_page;
diff --git a/gdb/utils.h b/gdb/utils.h
index 69f84e0489c..ac30fd5f114 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -464,21 +464,6 @@ extern void print_spaces_filtered (int, struct ui_file *);
 
 extern const char *n_spaces (int);
 
-extern void fputstr_filtered (const char *str, int quotr,
-			      struct ui_file * stream);
-
-extern void fputstr_unfiltered (const char *str, int quotr,
-				struct ui_file * stream);
-
-extern void fputstrn_filtered (const char *str, int n, int quotr,
-			       struct ui_file * stream);
-
-typedef int (*do_fputc_ftype) (int c, ui_file *stream);
-
-extern void fputstrn_unfiltered (const char *str, int n, int quotr,
-				 do_fputc_ftype do_fputc,
-				 struct ui_file * stream);
-
 /* Return nonzero if filtered printing is initialized.  */
 extern int filtered_printing_initialized (void);
 

Re: [PATCH] Implement putstr and putstrn in ui_file

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2022-01-02 16:46:17 -0700]:

> Lancelot> I have included comments and questions below.
> 
> Thanks for the review.
> 
> >> +	    buf[out++] = '0';
> >> +	    buf[out++] = '0';
> >> +	    buf[out++] = '0';
> 
> Lancelot> This is different from the printchar implementation you moved (and it
> Lancelot> seems that always printing '\000' is a loss of information).  Maybe you
> Lancelot> forgot to finalize this part?
> 
> Yikes, I certainly did.  This means that there aren't any tests for
> this, so in v2 of the patch (appended) I added a self-test.
> 
> Lancelot> I?find the comment slightly misleading (or incomplete).  The function
> Lancelot> prints exactly N?chars.  So if 'strlen (str) < n', putstrn prints
> Lancelot> more stuff than putstrn.
> 
> Lancelot> Maybe ?Like putstr, but print exactly N characters, starting at the
> Lancelot> position pointed by STR?, or something similar?
> 
> I rewrote this to try to clarify it, and also rearranged so that the
> more detailed comment from printchar is now by putstr.
> 
> >> +     usual, but not any other quoting character. */
> 
> Lancelot> You are missing one space after the '.'.
> 
> Fixed (well, avoided) by the comment rearrangement.
> 
> Tom
> 
> commit a87c1495874162fa41171ff8a43dcfa1ce2e48e2
> Author: Tom Tromey <tom@tromey.com>
> Date:   Fri Dec 31 10:40:02 2021 -0700
> 
>     Implement putstr and putstrn in ui_file
>     
>     In my tour of the ui_file subsystem, I found that fputstr and fputstrn
>     can be simplified.  The _filtered forms are never used (and IMO
>     unlikely to ever be used) and so can be removed.  And, the interface
>     can be simplified by removing a callback function and moving the
>     implementation directly to ui_file.
>     
>     A new self-test is included.  Previously, I think nothing was testing
>     this code.
>     
>     Regression tested on x86-64 Fedora 34.
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 8cd97475e86..d0db5fbdee1 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -477,6 +477,7 @@ SELFTESTS_SRCS = \
>  	unittests/style-selftests.c \
>  	unittests/tracepoint-selftests.c \
>  	unittests/tui-selftests.c \
> +	unittests/ui-file-selftests.c \
>  	unittests/unpack-selftests.c \
>  	unittests/utils-selftests.c \
>  	unittests/vec-utils-selftests.c \
> diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
> index bc6266ae055..9ce9cb4f678 100644
> --- a/gdb/guile/scm-ports.c
> +++ b/gdb/guile/scm-ports.c
> @@ -191,7 +191,7 @@ ioscm_open_port (scm_t_port_type *port_type, long mode_bits, scm_t_bits stream)
>  
> 
>  /* Support for connecting Guile's stdio ports to GDB's stdio ports.  */
>  
> -/* Like fputstrn_filtered, but don't escape characters, except nul.
> +/* Print a string, but don't escape characters, except nul.
>     Also like fputs_filtered, but a length is specified.  */

Given you're changing this comment anyway, please could you get rid of
the "Also like fputs_filtered...", we can just say something like:

  /* Print a string S, length SIZE, but don't escape characters,
     except nul.  */

Then, everything is self contained.

Otherwise, this version looks great.

Thanks,
Andrew

>  
>  static void
> diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c
> index 157154b361c..9db87fe180c 100644
> --- a/gdb/mi/mi-console.c
> +++ b/gdb/mi/mi-console.c
> @@ -48,16 +48,6 @@ mi_console_file::write (const char *buf, long length_buf)
>      this->flush ();
>  }
>  
> -/* Write C to STREAM's in an async-safe way.  */
> -
> -static int
> -do_fputc_async_safe (int c, ui_file *stream)
> -{
> -  char ch = c;
> -  stream->write_async_safe (&ch, 1);
> -  return c;
> -}
> -
>  void
>  mi_console_file::write_async_safe (const char *buf, long length_buf)
>  {
> @@ -65,12 +55,11 @@ mi_console_file::write_async_safe (const char *buf, long length_buf)
>    if (m_quote)
>      {
>        m_raw->write_async_safe (&m_quote, 1);
> -      fputstrn_unfiltered (buf, length_buf, m_quote, do_fputc_async_safe,
> -			   m_raw);
> +      m_raw->putstrn (buf, length_buf, m_quote, true);
>        m_raw->write_async_safe (&m_quote, 1);
>      }
>    else
> -    fputstrn_unfiltered (buf, length_buf, 0, do_fputc_async_safe, m_raw);
> +    m_raw->putstrn (buf, length_buf, 0, true);
>  
>    char nl = '\n';
>    m_raw->write_async_safe (&nl, 1);
> @@ -91,14 +80,13 @@ mi_console_file::flush ()
>        if (m_quote)
>  	{
>  	  fputc_unfiltered (m_quote, m_raw);
> -	  fputstrn_unfiltered (buf, length_buf, m_quote, fputc_unfiltered,
> -			       m_raw);
> +	  m_raw->putstrn (buf, length_buf, m_quote);
>  	  fputc_unfiltered (m_quote, m_raw);
>  	  fputc_unfiltered ('\n', m_raw);
>  	}
>        else
>  	{
> -	  fputstrn_unfiltered (buf, length_buf, 0, fputc_unfiltered, m_raw);
> +	  m_raw->putstrn (buf, length_buf, 0);
>  	  fputc_unfiltered ('\n', m_raw);
>  	}
>        gdb_flush (m_raw);
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index e729a861c61..4860da7536a 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1866,7 +1866,7 @@ mi_print_exception (const char *token, const struct gdb_exception &exception)
>    if (exception.message == NULL)
>      fputs_unfiltered ("unknown error", mi->raw_stdout);
>    else
> -    fputstr_unfiltered (exception.what (), '"', mi->raw_stdout);
> +    mi->raw_stdout->putstr (exception.what (), '"');
>    fputs_unfiltered ("\"", mi->raw_stdout);
>  
>    switch (exception.error)
> diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
> index e9a44437a90..20c6f0f9194 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -135,7 +135,7 @@ mi_ui_out::do_field_string (int fldno, int width, ui_align align,
>      fprintf_unfiltered (stream, "%s=", fldname);
>    fprintf_unfiltered (stream, "\"");
>    if (string)
> -    fputstr_unfiltered (string, '"', stream);
> +    stream->putstr (string, '"');
>    fprintf_unfiltered (stream, "\"");
>  }
>  
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 57352e44a34..c6a4888ed48 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -47,13 +47,15 @@ ui_file::printf (const char *format, ...)
>  void
>  ui_file::putstr (const char *str, int quoter)
>  {
> -  fputstr_unfiltered (str, quoter, this);
> +  while (*str)
> +    printchar (*str++, quoter, false);
>  }
>  
>  void
> -ui_file::putstrn (const char *str, int n, int quoter)
> +ui_file::putstrn (const char *str, int n, int quoter, bool async_safe)
>  {
> -  fputstrn_unfiltered (str, n, quoter, fputc_unfiltered, this);
> +  for (int i = 0; i < n; i++)
> +    printchar (str[i], quoter, async_safe);
>  }
>  
>  int
> @@ -68,6 +70,67 @@ ui_file::vprintf (const char *format, va_list args)
>    vfprintf_unfiltered (this, format, args);
>  }
>  
> +/* See ui-file.h.  */
> +
> +void
> +ui_file::printchar (int c, int quoter, bool async_safe)
> +{
> +  char buf[4];
> +  int out = 0;
> +
> +  c &= 0xFF;			/* Avoid sign bit follies */
> +
> +  if (c < 0x20			 /* Low control chars */
> +      || (c >= 0x7F && c < 0xA0) /* DEL, High controls */
> +      || (sevenbit_strings && c >= 0x80))
> +    {				/* high order bit set */
> +      buf[out++] = '\\';
> +
> +      switch (c)
> +	{
> +	case '\n':
> +	  buf[out++] = 'n';
> +	  break;
> +	case '\b':
> +	  buf[out++] = 'b';
> +	  break;
> +	case '\t':
> +	  buf[out++] = 't';
> +	  break;
> +	case '\f':
> +	  buf[out++] = 'f';
> +	  break;
> +	case '\r':
> +	  buf[out++] = 'r';
> +	  break;
> +	case '\033':
> +	  buf[out++] = 'e';
> +	  break;
> +	case '\007':
> +	  buf[out++] = 'a';
> +	  break;
> +	default:
> +	  {
> +	    buf[out++] = '0' + ((c >> 6) & 0x7);
> +	    buf[out++] = '0' + ((c >> 3) & 0x7);
> +	    buf[out++] = '0' + ((c >> 0) & 0x7);
> +	    break;
> +	  }
> +	}
> +    }
> +  else
> +    {
> +      if (quoter != 0 && (c == '\\' || c == quoter))
> +	buf[out++] = '\\';
> +      buf[out++] = c;
> +    }
> +
> +  if (async_safe)
> +    this->write_async_safe (buf, out);
> +  else
> +    this->write (buf, out);
> +}
> +
>  
> 
>  
>  void
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 89f249ead5e..c097abf0c29 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -34,12 +34,22 @@ class ui_file
>  
>    void printf (const char *, ...) ATTRIBUTE_PRINTF (2, 3);
>  
> -  /* Print a string whose delimiter is QUOTER.  Note that these
> -     routines should only be called for printing things which are
> -     independent of the language of the program being debugged.  */
> +  /* Print a NUL-terminated string whose delimiter is QUOTER.  Note
> +     that these routines should only be called for printing things
> +     which are independent of the language of the program being
> +     debugged.
> +
> +     This will normally escape backslashes and instances of QUOTER.
> +     If QUOTER is 0, it won't escape backslashes or any quoting
> +     character.  As a side effect, if you pass the backslash character
> +     as the QUOTER, this will escape backslashes as usual, but not any
> +     other quoting character.  */
>    void putstr (const char *str, int quoter);
>  
> -  void putstrn (const char *str, int n, int quoter);
> +  /* Like putstr, but only print the first N characters of STR.  If
> +     ASYNC_SAFE is true, then the output is done via the
> +     write_async_safe method.  */
> +  void putstrn (const char *str, int n, int quoter, bool async_safe = false);
>  
>    int putc (int c);
>  
> @@ -96,6 +106,13 @@ class ui_file
>         default.  */
>      return false;
>    }
> +
> +private:
> +
> +  /* Helper function for putstr and putstrn.  Print the character C on
> +     this stream as part of the contents of a literal string whose
> +     delimiter is QUOTER.  */
> +  void printchar (int c, int quoter, bool async_safe);
>  };
>  
>  typedef std::unique_ptr<ui_file> ui_file_up;
> diff --git a/gdb/unittests/ui-file-selftests.c b/gdb/unittests/ui-file-selftests.c
> new file mode 100644
> index 00000000000..55854386169
> --- /dev/null
> +++ b/gdb/unittests/ui-file-selftests.c
> @@ -0,0 +1,62 @@
> +/* Self tests for ui_file
> +
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "gdbsupport/selftest.h"
> +#include "ui-file.h"
> +
> +namespace selftests {
> +namespace file {
> +
> +static void
> +check_one (const char *str, int quoter, const char *result)
> +{
> +  string_file out;
> +  out.putstr (str, quoter);
> +  SELF_CHECK (out.string () == result);
> +}
> +
> +static void
> +run_tests ()
> +{
> +  check_one ("basic stuff: \\", '\\',
> +	     "basic stuff: \\\\");
> +  check_one ("more basic stuff: \\Q", 'Q',
> +	     "more basic stuff: \\\\\\Q");
> +  check_one ("more basic stuff: \\Q", '\0',
> +	     "more basic stuff: \\Q");
> +
> +  check_one ("weird stuff: \x1f\x90\n\b\t\f\r\033\007", '\\',
> +	     "weird stuff: \\037\\220\\n\\b\\t\\f\\r\\e\\a");
> +
> +  scoped_restore save_7 = make_scoped_restore (&sevenbit_strings, true);
> +  check_one ("more weird stuff: \xa5", '\\',
> +	     "more weird stuff: \\245");
> +}
> +
> +} /* namespace file*/
> +} /* namespace selftests */
> +
> +void _initialize_ui_file_selftest ();
> +void
> +_initialize_ui_file_selftest ()
> +{
> +  selftests::register_test ("ui-file",
> +			    selftests::file::run_tests);
> +}
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 11f88d6f991..91662dc5fd8 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1129,103 +1129,6 @@ parse_escape (struct gdbarch *gdbarch, const char **string_ptr)
>    return target_char;
>  }
>  
> 
> -/* Print the character C on STREAM as part of the contents of a literal
> -   string whose delimiter is QUOTER.  Note that this routine should only
> -   be called for printing things which are independent of the language
> -   of the program being debugged.
> -
> -   printchar will normally escape backslashes and instances of QUOTER. If
> -   QUOTER is 0, printchar won't escape backslashes or any quoting character.
> -   As a side effect, if you pass the backslash character as the QUOTER,
> -   printchar will escape backslashes as usual, but not any other quoting
> -   character. */
> -
> -static void
> -printchar (int c, do_fputc_ftype do_fputc, ui_file *stream, int quoter)
> -{
> -  c &= 0xFF;			/* Avoid sign bit follies */
> -
> -  if (c < 0x20 ||		/* Low control chars */
> -      (c >= 0x7F && c < 0xA0) ||	/* DEL, High controls */
> -      (sevenbit_strings && c >= 0x80))
> -    {				/* high order bit set */
> -      do_fputc ('\\', stream);
> -
> -      switch (c)
> -	{
> -	case '\n':
> -	  do_fputc ('n', stream);
> -	  break;
> -	case '\b':
> -	  do_fputc ('b', stream);
> -	  break;
> -	case '\t':
> -	  do_fputc ('t', stream);
> -	  break;
> -	case '\f':
> -	  do_fputc ('f', stream);
> -	  break;
> -	case '\r':
> -	  do_fputc ('r', stream);
> -	  break;
> -	case '\033':
> -	  do_fputc ('e', stream);
> -	  break;
> -	case '\007':
> -	  do_fputc ('a', stream);
> -	  break;
> -	default:
> -	  {
> -	    do_fputc ('0' + ((c >> 6) & 0x7), stream);
> -	    do_fputc ('0' + ((c >> 3) & 0x7), stream);
> -	    do_fputc ('0' + ((c >> 0) & 0x7), stream);
> -	    break;
> -	  }
> -	}
> -    }
> -  else
> -    {
> -      if (quoter != 0 && (c == '\\' || c == quoter))
> -	do_fputc ('\\', stream);
> -      do_fputc (c, stream);
> -    }
> -}
> -
> -/* Print the character C on STREAM as part of the contents of a
> -   literal string whose delimiter is QUOTER.  Note that these routines
> -   should only be call for printing things which are independent of
> -   the language of the program being debugged.  */
> -
> -void
> -fputstr_filtered (const char *str, int quoter, struct ui_file *stream)
> -{
> -  while (*str)
> -    printchar (*str++, fputc_filtered, stream, quoter);
> -}
> -
> -void
> -fputstr_unfiltered (const char *str, int quoter, struct ui_file *stream)
> -{
> -  while (*str)
> -    printchar (*str++, fputc_unfiltered, stream, quoter);
> -}
> -
> -void
> -fputstrn_filtered (const char *str, int n, int quoter,
> -		   struct ui_file *stream)
> -{
> -  for (int i = 0; i < n; i++)
> -    printchar (str[i], fputc_filtered, stream, quoter);
> -}
> -
> -void
> -fputstrn_unfiltered (const char *str, int n, int quoter,
> -		     do_fputc_ftype do_fputc, struct ui_file *stream)
> -{
> -  for (int i = 0; i < n; i++)
> -    printchar (str[i], do_fputc, stream, quoter);
> -}
> -
> 
>  
>  /* Number of lines per page or UINT_MAX if paging is disabled.  */
>  static unsigned int lines_per_page;
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 69f84e0489c..ac30fd5f114 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -464,21 +464,6 @@ extern void print_spaces_filtered (int, struct ui_file *);
>  
>  extern const char *n_spaces (int);
>  
> -extern void fputstr_filtered (const char *str, int quotr,
> -			      struct ui_file * stream);
> -
> -extern void fputstr_unfiltered (const char *str, int quotr,
> -				struct ui_file * stream);
> -
> -extern void fputstrn_filtered (const char *str, int n, int quotr,
> -			       struct ui_file * stream);
> -
> -typedef int (*do_fputc_ftype) (int c, ui_file *stream);
> -
> -extern void fputstrn_unfiltered (const char *str, int n, int quotr,
> -				 do_fputc_ftype do_fputc,
> -				 struct ui_file * stream);
> -
>  /* Return nonzero if filtered printing is initialized.  */
>  extern int filtered_printing_initialized (void);
>  
> 

Re: [PATCH] Implement putstr and putstrn in ui_file

[Tom Tromey]

>> -/* Like fputstrn_filtered, but don't escape characters, except nul.
>> +/* Print a string, but don't escape characters, except nul.
>> Also like fputs_filtered, but a length is specified.  */

Andrew> Given you're changing this comment anyway, please could you get rid of
Andrew> the "Also like fputs_filtered...", we can just say something like:

Andrew>   /* Print a string S, length SIZE, but don't escape characters,
Andrew>      except nul.  */

Andrew> Then, everything is self contained.

Andrew> Otherwise, this version looks great.

Thanks.  I made that change and I'm checking it in.

Tom