Re: [PATCH 02/13] gdbsupport: remove uses of sprintf

Simon Marchi <[email protected]> writes:

> When building on macOS, I get some errors about the uses of sprintf:
>
>       CXX      xml-utils.o
>     /Users/smarchi/src/binutils-gdb/gdbsupport/xml-utils.cc:91:8: error: 'sprintf' 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 snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
>        91 |               sprintf (str, "%d", va_arg (ap, int));
>           |               ^
>
> We know they are safe, because the 32 byte destination buffer is large
> enough for all conversions.  But I also don't think it's a big deal to
> switch to xsnprintf to avoid these errors, and to catch any future
> error.

Approved-By: Andrew Burgess <[email protected]>

Thanks,
Andrew

>
> Change-Id: If3531e1916e103dfccd0ec033639b14a2b6df3cf
> ---
>  gdbsupport/xml-utils.cc | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/gdbsupport/xml-utils.cc b/gdbsupport/xml-utils.cc
> index 13dc27499120..cec7281c9297 100644
> --- a/gdbsupport/xml-utils.cc
> +++ b/gdbsupport/xml-utils.cc
> @@ -88,52 +88,55 @@ string_xml_appendf (std::string &buffer, const char *format, ...)
>  	      str = va_arg (ap, char *);
>  	      break;
>  	    case 'd':
> -	      sprintf (str, "%d", va_arg (ap, int));
> +	      xsnprintf (buf, sizeof (buf), "%d", va_arg (ap, int));
>  	      break;
>  	    case 'u':
> -	      sprintf (str, "%u", va_arg (ap, unsigned int));
> +	      xsnprintf (buf, sizeof (buf), "%u", va_arg (ap, unsigned int));
>  	      break;
>  	    case 'x':
> -	      sprintf (str, "%x", va_arg (ap, unsigned int));
> +	      xsnprintf (buf, sizeof (buf), "%x", va_arg (ap, unsigned int));
>  	      break;
>  	    case 'o':
> -	      sprintf (str, "%o", va_arg (ap, unsigned int));
> +	      xsnprintf (buf, sizeof (buf), "%o", va_arg (ap, unsigned int));
>  	      break;
>  	    case 'l':
>  	      f++;
>  	      switch (*f)
>  		{
>  		case 'd':
> -		  sprintf (str, "%ld", va_arg (ap, long));
> +		  xsnprintf (buf, sizeof (buf), "%ld", va_arg (ap, long));
>  		  break;
>  		case 'u':
> -		  sprintf (str, "%lu", va_arg (ap, unsigned long));
> +		  xsnprintf (buf, sizeof (buf), "%lu",
> +			     va_arg (ap, unsigned long));
>  		  break;
>  		case 'x':
> -		  sprintf (str, "%lx", va_arg (ap, unsigned long));
> +		  xsnprintf (buf, sizeof (buf), "%lx",
> +			     va_arg (ap, unsigned long));
>  		  break;
>  		case 'o':
> -		  sprintf (str, "%lo", va_arg (ap, unsigned long));
> +		  xsnprintf (buf, sizeof (buf), "%lo",
> +			     va_arg (ap, unsigned long));
>  		  break;
>  		case 'l':
>  		  f++;
>  		  switch (*f)
>  		    {
>  		    case 'd':
> -		      sprintf (str, "%" PRId64,
> -			       (int64_t) va_arg (ap, long long));
> +		      xsnprintf (buf, sizeof (buf), "%" PRId64,
> +				 (int64_t) va_arg (ap, long long));
>  		      break;
>  		    case 'u':
> -		      sprintf (str, "%" PRIu64,
> -			       (uint64_t) va_arg (ap, unsigned long long));
> +		      xsnprintf (buf, sizeof (buf), "%" PRIu64,
> +				 (uint64_t) va_arg (ap, unsigned long long));
>  		      break;
>  		    case 'x':
> -		      sprintf (str, "%" PRIx64,
> -			       (uint64_t) va_arg (ap, unsigned long long));
> +		      xsnprintf (buf, sizeof (buf), "%" PRIx64,
> +				 (uint64_t) va_arg (ap, unsigned long long));
>  		      break;
>  		    case 'o':
> -		      sprintf (str, "%" PRIo64,
> -			       (uint64_t) va_arg (ap, unsigned long long));
> +		      xsnprintf (buf, sizeof (buf), "%" PRIo64,
> +				 (uint64_t) va_arg (ap, unsigned long long));
>  		      break;
>  		    default:
>  		      str = 0;
> -- 
> 2.55.0