Re: [PATCH] Implement putstr and putstrn in ui_file

[Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>]

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.