Re: [PATCH 01/13] gdbsupport: remove uses of vsprintf

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simon.marchi@efficios.com> writes:

> When building on macOS, I get:
>
>       CXX      common-utils.o
>     /Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:106:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
>       106 |   vsprintf (&str[0], fmt, vp);
>           |   ^
>     /Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:128:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
>       128 |   vsprintf (&str[0], fmt, args);
>           |   ^
>     /Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:166:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
>       166 |   vsprintf (&str[curr_size], fmt, args);
>           |   ^
>
> We know that those calls should be safe because we computed the size that
> fmt+args take just before, and allocated that many bytes.  But I also
> don't see a real downside in switching those calls to use vsnprintf and
> double check that everything went right.
>
> Change the type of the existing "size" variable in "string_vprintf" to
> "int", since that's what vsnprintf returns.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> Change-Id: I589d9a170fdd15cc31b44b76689c6d8c324e340a
> ---
>  gdbsupport/common-utils.cc | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc
> index 3ae3afcc380b..f31699be13a1 100644
> --- a/gdbsupport/common-utils.cc
> +++ b/gdbsupport/common-utils.cc
> @@ -92,10 +92,9 @@ std::string
>  string_printf (const char* fmt, ...)
>  {
>    va_list vp;
> -  int size;
>  
>    va_start (vp, fmt);
> -  size = vsnprintf (NULL, 0, fmt, vp);
> +  int size = vsnprintf (NULL, 0, fmt, vp);
>    va_end (vp);
>  
>    std::string str (size, '\0');
> @@ -103,7 +102,8 @@ string_printf (const char* fmt, ...)
>    /* C++11 and later guarantee std::string uses contiguous memory and
>       always includes the terminating '\0'.  */
>    va_start (vp, fmt);
> -  vsprintf (&str[0], fmt, vp);
> +  int ret = vsnprintf (&str[0], size + 1, fmt, vp);
> +  gdb_assert (ret == size);
>    va_end (vp);
>  
>    return str;
> @@ -115,17 +115,17 @@ std::string
>  string_vprintf (const char* fmt, va_list args)
>  {
>    va_list vp;
> -  size_t size;
>  
>    va_copy (vp, args);
> -  size = vsnprintf (NULL, 0, fmt, vp);
> +  int size = vsnprintf (NULL, 0, fmt, vp);
>    va_end (vp);
>  
>    std::string str (size, '\0');
>  
>    /* C++11 and later guarantee std::string uses contiguous memory and
>       always includes the terminating '\0'.  */
> -  vsprintf (&str[0], fmt, args);
> +  int ret = vsnprintf (&str[0], size + 1, fmt, args);
> +  gdb_assert (ret == size);
>  
>    return str;
>  }
> @@ -152,10 +152,9 @@ std::string &
>  string_vappendf (std::string &str, const char *fmt, va_list args)
>  {
>    va_list vp;
> -  int grow_size;
>  
>    va_copy (vp, args);
> -  grow_size = vsnprintf (NULL, 0, fmt, vp);
> +  int grow_size = vsnprintf (NULL, 0, fmt, vp);
>    va_end (vp);
>  
>    size_t curr_size = str.size ();
> @@ -163,7 +162,8 @@ string_vappendf (std::string &str, const char *fmt, va_list args)
>  
>    /* C++11 and later guarantee std::string uses contiguous memory and
>       always includes the terminating '\0'.  */
> -  vsprintf (&str[curr_size], fmt, args);
> +  int ret = vsnprintf (&str[curr_size], grow_size + 1, fmt, args);
> +  gdb_assert (ret == grow_size);
>  
>    return str;
>  }
> -- 
> 2.55.0