[01/13] gdbsupport: remove uses of vsprintf
Checks
Commit Message
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.
Change-Id: I589d9a170fdd15cc31b44b76689c6d8c324e340a
---
gdbsupport/common-utils.cc | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
Comments
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
@@ -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;
}