[patch] cleanup: Wunused - amd64

[patch] cleanup: Wunused - amd64

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]

Hello,

This is removing unused variables and allows compiling with -Wunused.

Thanks,

Aleksandar Ristovski
QNX Software Systems


ChangeLog:

   Aleksandar Ristovski  <aristovski@qnx.com>

      * amd64-tdep.c (fixup_riprel): Remove unused BYTE_ORDER, DISP.


[-- Attachment #2: Wunused-amd64-201301302100.patch --]
[-- Type: text/x-patch, Size: 1142 bytes --]

Index: gdb/amd64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
retrieving revision 1.117
diff -u -p -r1.117 amd64-tdep.c
--- gdb/amd64-tdep.c	1 Jan 2013 06:32:37 -0000	1.117
+++ gdb/amd64-tdep.c	30 Jan 2013 22:25:14 -0000
@@ -1225,12 +1225,10 @@ static void
 fixup_riprel (struct gdbarch *gdbarch, struct displaced_step_closure *dsc,
 	      CORE_ADDR from, CORE_ADDR to, struct regcache *regs)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   const struct amd64_insn *insn_details = &dsc->insn_details;
   int modrm_offset = insn_details->modrm_offset;
   gdb_byte *insn = insn_details->raw_insn + modrm_offset;
   CORE_ADDR rip_base;
-  int32_t disp;
   int insn_length;
   int arch_tmp_regno, tmp_regno;
   ULONGEST orig_value;
@@ -1239,7 +1237,6 @@ fixup_riprel (struct gdbarch *gdbarch, s
   ++insn;
 
   /* Compute the rip-relative address.	*/
-  disp = extract_signed_integer (insn, sizeof (int32_t), byte_order);
   insn_length = gdb_buffered_insn_length (gdbarch, dsc->insn_buf,
 					  dsc->max_len, from);
   rip_base = from + insn_length;

Re: [patch] cleanup: Wunused - amd64

[Tom Tromey]

>>>>> "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:

Aleksandar>      * amd64-tdep.c (fixup_riprel): Remove unused
Aleksandar>      BYTE_ORDER, DISP.

This one isn't obvious to me.

Aleksandar>    /* Compute the rip-relative address.	*/
Aleksandar> -  disp = extract_signed_integer (insn, sizeof (int32_t), byte_order);

Is it really the case that this code does not need to use 'disp'?
Or is it a bug that it doesn't use it?

Tom

Re: [patch] cleanup: Wunused - amd64

[Aleksandar Ristovski]

On 13-01-31 11:09 AM, Tom Tromey wrote:
>>>>>> "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:
>
> Aleksandar>      * amd64-tdep.c (fixup_riprel): Remove unused
> Aleksandar>      BYTE_ORDER, DISP.
>
> This one isn't obvious to me.
>
> Aleksandar>    /* Compute the rip-relative address.	*/
> Aleksandar> -  disp = extract_signed_integer (insn, sizeof (int32_t), byte_order);
>
> Is it really the case that this code does not need to use 'disp'?
> Or is it a bug that it doesn't use it?

It is not used. Is it a bug, I don't know, I haven't analyzed any deeper 
(but my patch would not make it any worse than what it is now).

---
Aleksandar

Re: [patch] cleanup: Wunused - amd64

[Tom Tromey]

>>>>> "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:

Aleksandar> It is not used. Is it a bug, I don't know, I haven't
Aleksandar> analyzed any deeper (but my patch would not make it any
Aleksandar> worse than what it is now).

I think in this case the patch needs a fuller investigation.  I agree
your patch doesn't change the meaning of the code, but I think that if
there is in fact a bug here, then the patch would obscure that.

Tom

Re: [patch] cleanup: Wunused - amd64

[Aleksandar Ristovski]

On 13-01-31 01:20 PM, Tom Tromey wrote:
if
> there is in fact a bug here, then the patch would obscure that.
>

That is true.

Ok, I'll investigate, but not before another week or two. (there is no 
rush about any of Wunused patches anyway...)

Thanks,

Aleksandar

Re: [patch] cleanup: Wunused - amd64

[Joel Brobecker]

>   Aleksandar Ristovski  <aristovski@qnx.com>
> 
>      * amd64-tdep.c (fixup_riprel): Remove unused BYTE_ORDER, DISP.

Was this ever reviewed? If not, I apologize. Approved.

-- 
Joel

Re: [patch] cleanup: Wunused - amd64

[Aleksandar Ristovski]

On 13-03-28 08:26 PM, Joel Brobecker wrote:
>>    Aleksandar Ristovski  <aristovski@qnx.com>
>>
>>       * amd64-tdep.c (fixup_riprel): Remove unused BYTE_ORDER, DISP.
>
> Was this ever reviewed? If not, I apologize. Approved.
>

Tom has expressed concerns about removing 'disp', and I haven't had time 
to investigate why is 'disp' there and if it is a bug that it's not used 
(http://sourceware.org/ml/gdb-patches/2013-01/msg00767.html) , so I 
won't apply this patch, but will proceed with the other two.

Thanks for reviewing,

Aleksandar

Re: [patch] cleanup: Wunused - amd64

[Aleksandar Ristovski]

On 13-03-28 08:26 PM, Joel Brobecker wrote:
>>    Aleksandar Ristovski  <aristovski@qnx.com>
>>
>>       * amd64-tdep.c (fixup_riprel): Remove unused BYTE_ORDER, DISP.
>
> Was this ever reviewed? If not, I apologize. Approved.
>

Tom has expressed concerns about removing 'disp', and I haven't had time 
to investigate why is 'disp' there and if it is a bug that it's not used 
(http://sourceware.org/ml/gdb-patches/2013-01/msg00767.html) , so I 
won't apply this patch, but will proceed with the other two.

Thanks for reviewing,

Aleksandar