[03/13] opcodes/z80: remove use of sprintf

Message ID 20260817151646.152571-4-simon.marchi@efficios.com
State New
Headers
Series Fix various warnings when building on macOS |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply

Commit Message

Simon Marchi Aug. 17, 2026, 3:16 p.m. UTC
  When building on macOS, I get:

      CC       z80-dis.lo
    /Users/smarchi/src/binutils-gdb/opcodes/z80-dis.c:804:41: 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]
      804 |   info->fprintf_func = (fprintf_ftype) &sprintf;
          |                                         ^

Replace this use of sprintf with the safer snprintf.  Add a small
structure and wrappers around snprintf in order to glue everything
together.

When asked to review my patch, Claude Code mentioned that the existing
code had a latent bug: while info->fprintf_func and info->stream get set
temporarily, info->fprintf_styled_func doesn't.  If fprintf_styled_func
happened to be called, it would receive a `stream` it doesn't expect.
It's probably not a problem today, if the disassembler doesn't emit
styling, but it seems like a good moment to fix it.  Use
the disassemble_set_printf function to set both fprintf functions and
the stream argument at the same time.

Change-Id: I85dee82f3a0c53f38e52ca1158bc605854ab4896
---
 opcodes/z80-dis.c | 52 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 4 deletions(-)
  

Comments

Jan Beulich Aug. 18, 2026, 6:40 a.m. UTC | #1
On 17.08.2026 17:16, Simon Marchi wrote:
> --- a/opcodes/z80-dis.c
> +++ b/opcodes/z80-dis.c
> @@ -768,11 +768,55 @@ pref_ind (struct buffer *buf, disassemble_info *info, const char *txt)
>  static int
>  print_insn_z80_buf (struct buffer *buf, disassemble_info *info);
>  
> +struct sized_buf
> +{
> +  char *buf;
> +  size_t size;
> +};
> +
> +/* An fprintf_ftype implementation writing to STREAM, which must point to a
> +   struct sized_buf.  */
> +
> +static int ATTRIBUTE_PRINTF_2
> +sized_buf_printf (void *stream, const char *format, ...)
> +{
> +  va_list ap;
> +  int ret;
> +  struct sized_buf *sbuf = (struct sized_buf *) stream;

We're in the (slow going) process of removing such unnecessary casts, in the
interest of getting the amount of casts down in general. Please drop this one
as well as ...

> +  va_start (ap, format);
> +  ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
> +  va_end (ap);
> +
> +  return ret;
> +}
> +
> +/* Same as sized_buf_printf, but as an fprintf_styled_ftype implementation.
> +   The style is ignored for now.  */
> +
> +static int ATTRIBUTE_PRINTF_3
> +sized_buf_styled_printf (void *stream,
> +			 enum disassembler_style style ATTRIBUTE_UNUSED,
> +			 const char *format, ...)
> +{
> +  va_list ap;
> +  int ret;
> +  struct sized_buf *sbuf = (struct sized_buf *) stream;

... the one here.

> +  va_start (ap, format);
> +  ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
> +  va_end (ap);
> +
> +  return ret;
> +}
> +
>  static int
>  suffix (struct buffer *buf, disassemble_info *info, const char *txt)
>  {
>    char mybuf[TXTSIZ*4];
> +  struct sized_buf sbuf = { mybuf, sizeof (mybuf) };

Please use ARRAY_SIZE() here. sizeof() happens to be correct for char[], but
wouldn't be correct for e.g. wchar_t[].

Okay with these adjustments.

Jan
  
Simon Marchi Aug. 18, 2026, 4:48 p.m. UTC | #2
On 8/18/26 2:40 AM, Jan Beulich wrote:
> On 17.08.2026 17:16, Simon Marchi wrote:
>> --- a/opcodes/z80-dis.c
>> +++ b/opcodes/z80-dis.c
>> @@ -768,11 +768,55 @@ pref_ind (struct buffer *buf, disassemble_info *info, const char *txt)
>>  static int
>>  print_insn_z80_buf (struct buffer *buf, disassemble_info *info);
>>  
>> +struct sized_buf
>> +{
>> +  char *buf;
>> +  size_t size;
>> +};
>> +
>> +/* An fprintf_ftype implementation writing to STREAM, which must point to a
>> +   struct sized_buf.  */
>> +
>> +static int ATTRIBUTE_PRINTF_2
>> +sized_buf_printf (void *stream, const char *format, ...)
>> +{
>> +  va_list ap;
>> +  int ret;
>> +  struct sized_buf *sbuf = (struct sized_buf *) stream;
> 
> We're in the (slow going) process of removing such unnecessary casts, in the
> interest of getting the amount of casts down in general. Please drop this one
> as well as ...

Ah sorry, C++ habit.

> 
>> +  va_start (ap, format);
>> +  ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
>> +  va_end (ap);
>> +
>> +  return ret;
>> +}
>> +
>> +/* Same as sized_buf_printf, but as an fprintf_styled_ftype implementation.
>> +   The style is ignored for now.  */
>> +
>> +static int ATTRIBUTE_PRINTF_3
>> +sized_buf_styled_printf (void *stream,
>> +			 enum disassembler_style style ATTRIBUTE_UNUSED,
>> +			 const char *format, ...)
>> +{
>> +  va_list ap;
>> +  int ret;
>> +  struct sized_buf *sbuf = (struct sized_buf *) stream;
> 
> ... the one here.

Fixed.

>> +  va_start (ap, format);
>> +  ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
>> +  va_end (ap);
>> +
>> +  return ret;
>> +}
>> +
>>  static int
>>  suffix (struct buffer *buf, disassemble_info *info, const char *txt)
>>  {
>>    char mybuf[TXTSIZ*4];
>> +  struct sized_buf sbuf = { mybuf, sizeof (mybuf) };
> 
> Please use ARRAY_SIZE() here. sizeof() happens to be correct for char[], but
> wouldn't be correct for e.g. wchar_t[].

Fixed.

> Okay with these adjustments.

Thanks, pushed with those changes.

Simon
  

Patch

diff --git a/opcodes/z80-dis.c b/opcodes/z80-dis.c
index d5b4c4210d0c..fd446d3f128d 100644
--- a/opcodes/z80-dis.c
+++ b/opcodes/z80-dis.c
@@ -768,11 +768,55 @@  pref_ind (struct buffer *buf, disassemble_info *info, const char *txt)
 static int
 print_insn_z80_buf (struct buffer *buf, disassemble_info *info);
 
+struct sized_buf
+{
+  char *buf;
+  size_t size;
+};
+
+/* An fprintf_ftype implementation writing to STREAM, which must point to a
+   struct sized_buf.  */
+
+static int ATTRIBUTE_PRINTF_2
+sized_buf_printf (void *stream, const char *format, ...)
+{
+  va_list ap;
+  int ret;
+  struct sized_buf *sbuf = (struct sized_buf *) stream;
+
+  va_start (ap, format);
+  ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
+  va_end (ap);
+
+  return ret;
+}
+
+/* Same as sized_buf_printf, but as an fprintf_styled_ftype implementation.
+   The style is ignored for now.  */
+
+static int ATTRIBUTE_PRINTF_3
+sized_buf_styled_printf (void *stream,
+			 enum disassembler_style style ATTRIBUTE_UNUSED,
+			 const char *format, ...)
+{
+  va_list ap;
+  int ret;
+  struct sized_buf *sbuf = (struct sized_buf *) stream;
+
+  va_start (ap, format);
+  ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
+  va_end (ap);
+
+  return ret;
+}
+
 static int
 suffix (struct buffer *buf, disassemble_info *info, const char *txt)
 {
   char mybuf[TXTSIZ*4];
+  struct sized_buf sbuf = { mybuf, sizeof (mybuf) };
   fprintf_ftype old_fprintf;
+  fprintf_styled_ftype old_fprintf_styled;
   void *old_stream;
   char *p;
 
@@ -800,15 +844,15 @@  suffix (struct buffer *buf, disassemble_info *info, const char *txt)
     }
 
   old_fprintf = info->fprintf_func;
+  old_fprintf_styled = info->fprintf_styled_func;
   old_stream = info->stream;
-  info->fprintf_func = (fprintf_ftype) &sprintf;
-  info->stream = mybuf;
+  disassemble_set_printf (info, &sbuf, sized_buf_printf,
+			  sized_buf_styled_printf);
   mybuf[0] = 0;
   buf->base++;
   if (print_insn_z80_buf (buf, info) >= 0)
     buf->n_used++;
-  info->fprintf_func = old_fprintf;
-  info->stream = old_stream;
+  disassemble_set_printf (info, old_stream, old_fprintf, old_fprintf_styled);
 
   for (p = mybuf; *p; ++p)
     if (*p == ' ')