diff mbox

[RFA,01/13] Simple unused variable removals

Message ID 87b519fc-318a-84aa-7ced-aea354f059fb@simark.ca
State New
Headers show

Commit Message

Simon Marchi July 14, 2018, 1:15 a.m. UTC
On 2018-07-12 04:51 PM, Tom Tromey wrote:
> This patch holds all the straightforward unused variable deletions.

I've looked at this patch and I didn't find anything that looked wrong
to my eyes.  In some cases, such as:



I wondered this was a bug (should that variable really be used), but I
wouldn't know without some quite extensive research...  so all-in-all, LGTM.

Simon

Comments

Tom Tromey July 14, 2018, 12:40 p.m. UTC | #1
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
Simon> -						 4, byte_order);

Simon> I wondered this was a bug (should that variable really be used), but I
Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.

Yes, this one was on the bubble and I almost put it in its own patch...
I can't recall offhand if there were others like this.

Tom
Pedro Alves July 16, 2018, 1:33 p.m. UTC | #2
On 07/14/2018 01:40 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
> Simon> -						 4, byte_order);
> 
> Simon> I wondered this was a bug (should that variable really be used), but I
> Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.
> 
> Yes, this one was on the bubble and I almost put it in its own patch...
> I can't recall offhand if there were others like this.

I skimmed the patch and it looks good to me to, and, that bit
also gave me pause.  I was wondering whether we should replace
it with a comment, saying that we're skipping 4 bytes which hold
the entry's size.  That might save someone some time in case the size
turns out to be needed.  (I have no idea where the structure being
extracted is documented, for example.)

Thanks,
Pedro Alves
Tom Tromey July 16, 2018, 3:35 p.m. UTC | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
Simon> -						 4, byte_order);
>> 
Simon> I wondered this was a bug (should that variable really be used), but I
Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.
>> 
>> Yes, this one was on the bubble and I almost put it in its own patch...
>> I can't recall offhand if there were others like this.

Pedro> I skimmed the patch and it looks good to me to, and, that bit
Pedro> also gave me pause.  I was wondering whether we should replace
Pedro> it with a comment, saying that we're skipping 4 bytes which hold
Pedro> the entry's size.  That might save someone some time in case the size
Pedro> turns out to be needed.  (I have no idea where the structure being
Pedro> extracted is documented, for example.)

I've split this into a separate patch and added a comment.

Tom
diff mbox

Patch

--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1820,8 +1820,6 @@  spu_get_overlay_table (struct objfile *objfile)
     {
       CORE_ADDR vma  = extract_unsigned_integer (ovly_table + 16*i + 0,
 						 4, byte_order);
-      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
-						 4, byte_order);
       CORE_ADDR pos  = extract_unsigned_integer (ovly_table + 16*i + 8,
 						 4, byte_order);
       CORE_ADDR buf  = extract_unsigned_integer (ovly_table + 16*i + 12,