Re: [PATCH v2 01/31] Introduce string_printf

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2016-10-18 21:11, Pedro Alves wrote:
> This introduces the string_printf function.  Like asprintf, but
> returns a std::string.
> 
> gdb/ChangeLog:
> yyyy-mm-yy  Pedro Alves  <palves@redhat.com>
> 
> 	* common/common-utils.c (string_printf): New function.
> 	* common/common-utils.h: Include <string>.
> 	(string_printf): Declare.
> ---
>  gdb/common/common-utils.c | 30 ++++++++++++++++++++++++++++++
>  gdb/common/common-utils.h |  6 ++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
> index 5a346ec..05ba3aa 100644
> --- a/gdb/common/common-utils.c
> +++ b/gdb/common/common-utils.c
> @@ -150,6 +150,36 @@ xsnprintf (char *str, size_t size, const char 
> *format, ...)
>    return ret;
>  }
> 
> +/* See documentation in common-utils.h.  */
> +
> +std::string
> +string_printf (const char* fmt, ...)
> +{
> +  std::string str;
> +  va_list vp;
> +
> +  /* Start by assuming some reasonable size will be sufficient.  */
> +  str.resize (1024);
> +
> +  while (1)
> +    {
> +      size_t size;
> +      int result;
> +
> +      va_start (vp, fmt);
> +      size = str.size ();
> +      result = vsnprintf (&str[0], size, fmt, vp);
> +      va_end (vp);
> +
> +      str.resize (result);

I think you have an off-by-one here, which causes an infinite loop if 
the 1024 bytes buffer is not large enough.  vsnprintf returns the size 
needed without the terminating \0, so the resize here should be of 
"result + 1".  To reproduce/test easily, just change your str.resize 
(1024) to something small.

I thought the use of a while loop for this a bit strange, but I 
understand it's to avoid duplicating the code.

I think you should try writing a unit test for this :).

Simon