[PATCH] gdb: remove casts to long in dwarf2/loc.c

[PATCH] gdb: remove casts to long in dwarf2/loc.c

[Simon Marchi]

I spotted this while reviewing another patch, these casts to long are
unnecessary.  The result of subtracting two pointers is ptrdiff_t, which
can be printed with the `t` length specifier.

Change-Id: Idf8052b110a61007261be19137e4864ae6b37190
---
 gdb/dwarf2/loc.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 5b2d58ab44e4..606c97c745de 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -3329,10 +3329,9 @@ disassemble_dwarf_expression (struct ui_file *stream,
       name = get_DW_OP_name (op);
 
       if (!name)
-	error (_("Unrecognized DWARF opcode 0x%02x at %ld"),
-	       op, (long) (data - 1 - start));
-      gdb_printf (stream, "  %*ld: %s", indent + 4,
-		  (long) (data - 1 - start), name);
+	error (_("Unrecognized DWARF opcode 0x%02x at %td"), op,
+	       data - 1 - start);
+      gdb_printf (stream, "  %*td: %s", indent + 4, data - 1 - start, name);
 
       switch (op)
 	{
@@ -3515,15 +3514,13 @@ disassemble_dwarf_expression (struct ui_file *stream,
 	case DW_OP_skip:
 	  l = extract_signed_integer (data, 2, gdbarch_byte_order (arch));
 	  data += 2;
-	  gdb_printf (stream, " to %ld",
-		      (long) (data + l - start));
+	  gdb_printf (stream, " to %td", data + l - start);
 	  break;
 
 	case DW_OP_bra:
 	  l = extract_signed_integer (data, 2, gdbarch_byte_order (arch));
 	  data += 2;
-	  gdb_printf (stream, " %ld",
-		      (long) (data + l - start));
+	  gdb_printf (stream, " %td", data + l - start);
 	  break;
 
 	case DW_OP_call2:

base-commit: 9d8fc40eb0e611162844eb7b89f1c76875153fbe
-- 
2.43.0

Re: [PATCH] gdb: remove casts to long in dwarf2/loc.c

[Tom de Vries]

On 12/4/23 21:40, Simon Marchi wrote:
> I spotted this while reviewing another patch, these casts to long are
> unnecessary.  The result of subtracting two pointers is ptrdiff_t, which
> can be printed with the `t` length specifier.
> 

I'm not sure if it's relevant, but gdb_printf uses 
format_pieces::format_pieces, which supports a number of length 
specififers, but not 't'.

Thanks,
- Tom

> Change-Id: Idf8052b110a61007261be19137e4864ae6b37190
> ---
>   gdb/dwarf2/loc.c | 13 +++++--------
>   1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index 5b2d58ab44e4..606c97c745de 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -3329,10 +3329,9 @@ disassemble_dwarf_expression (struct ui_file *stream,
>         name = get_DW_OP_name (op);
>   
>         if (!name)
> -	error (_("Unrecognized DWARF opcode 0x%02x at %ld"),
> -	       op, (long) (data - 1 - start));
> -      gdb_printf (stream, "  %*ld: %s", indent + 4,
> -		  (long) (data - 1 - start), name);
> +	error (_("Unrecognized DWARF opcode 0x%02x at %td"), op,
> +	       data - 1 - start);
> +      gdb_printf (stream, "  %*td: %s", indent + 4, data - 1 - start, name);
>   
>         switch (op)
>   	{
> @@ -3515,15 +3514,13 @@ disassemble_dwarf_expression (struct ui_file *stream,
>   	case DW_OP_skip:
>   	  l = extract_signed_integer (data, 2, gdbarch_byte_order (arch));
>   	  data += 2;
> -	  gdb_printf (stream, " to %ld",
> -		      (long) (data + l - start));
> +	  gdb_printf (stream, " to %td", data + l - start);
>   	  break;
>   
>   	case DW_OP_bra:
>   	  l = extract_signed_integer (data, 2, gdbarch_byte_order (arch));
>   	  data += 2;
> -	  gdb_printf (stream, " %ld",
> -		      (long) (data + l - start));
> +	  gdb_printf (stream, " %td", data + l - start);
>   	  break;
>   
>   	case DW_OP_call2:
> 
> base-commit: 9d8fc40eb0e611162844eb7b89f1c76875153fbe

Re: [PATCH] gdb: remove casts to long in dwarf2/loc.c

[Simon Marchi]

On 12/5/23 03:06, Tom de Vries wrote:
> On 12/4/23 21:40, Simon Marchi wrote:
>> I spotted this while reviewing another patch, these casts to long are
>> unnecessary.  The result of subtracting two pointers is ptrdiff_t, which
>> can be printed with the `t` length specifier.
>>
> 
> I'm not sure if it's relevant, but gdb_printf uses format_pieces::format_pieces, which supports a number of length specififers, but not 't'.

Shortly after sending the patch, I realized that (I only manually tested
a code path that uses the libc's printf, which worked).  I tried to send
this message, but it looks like I failed to reply all:

    Well, I didn't test this enough.  I tried %td with printf, but not with
    gdb_printf.  gdb_printf uses our home-grown implementation of printf,
    and it doesn't support fancy length specifiers.  So, forget about that.
    We could change the error() calls, but it just seems confusing if the
    calls next to it keep the cast.

Simon

Re: [PATCH] gdb: remove casts to long in dwarf2/loc.c

[Tom Tromey]

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

Simon>     Well, I didn't test this enough.  I tried %td with printf, but not with
Simon>     gdb_printf.  gdb_printf uses our home-grown implementation of printf,
Simon>     and it doesn't support fancy length specifiers.

It's fine IMO if you want to add standard length specifiers.  Basically
the idea is to keep it compatible with the standards, so that gcc's
printf checking will still work.

Tom