public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@imgtec.com>
To: Pedro Alves <palves@redhat.com>
Cc: Trevor Saunders <tbsaunde+binutils@tbsaunde.org>,
	Yao Qi	<qiyaoltc@gmail.com>, <gdb-patches@sourceware.org>
Subject: Re: [PATCH, doc RFA] Remove support for "target m32rsdi" and "target mips/pmon/ddb/rockhopper/lsi"
Date: Tue, 03 May 2016 14:57:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1605031545470.21846@tp.orcam.me.uk> (raw)
In-Reply-To: <a378644c-b0a4-50eb-2fe9-fe046b5af145@redhat.com>

On Mon, 2 May 2016, Pedro Alves wrote:

> >  NB it looks to me `mips_r3041_reg_names' is now dead.  We just *might* 
> > consider rewiring it like `mips_tx39_reg_names', but that would require 
> > defining another BFD machine type and I doubt anybody cares about the 
> > R3041 anymore (cf. the relevant comment you've just removed).  So if you 
> > care to remove it too, then I'll appreciate it and a change to do so is 
> > preapproved.
> > 
> >  Given that the variable is static I wonder why it hasn't triggered a 
> > compilation error in the build actually.
> 
> That's because gdb doesn't use -Wunused presently.

 Hmm, I thought it was implied by -Wall.  Perhaps we should add it then?  
Releases are built without -Werror so the end users will be safe either 
way, and it'll make us easier to avoid code pollution.

> I never managed to come back to this, and looks like we won't need to.
> Trevor sent a patch that removes mips_r3041_reg_names among a ton
> of other unused variables, here:
> 
>  https://sourceware.org/ml/gdb-patches/2016-04/msg00664.html

 Great!  As it happens, I made a patch to remove `mips_r3041_reg_names' on 
Friday, but didn't get to actually pushing it -- and we had a bank holiday 
yesterday.

 I'll give Trevor's change precedence then as a more general clean-up, 
although I'd like to review the MIPS part, as not all variables removed 
from mips-tdep.c are actually "trivially unused" (those would be lone 
declarations, possibly with initialisers).  Especially the heuristic 
unwinder bits look highly suspicious to me, where the variable is updated 
as the analysis proceeds.  It could be that these variables can indeed go, 
but perhaps something is missing that should be there.

 I'll try to get this done by the end of tomorrow.

  Maciej

  reply	other threads:[~2016-05-03 14:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-17 16:00 Pedro Alves
2016-03-22 10:49 ` Yao Qi
2016-03-22 11:54   ` Pedro Alves
2016-03-30 22:19     ` Maciej W. Rozycki
2016-03-31 12:07       ` Pedro Alves
2016-04-02  0:00         ` Maciej W. Rozycki
2016-05-02 11:59           ` Pedro Alves
2016-05-03 14:57             ` Maciej W. Rozycki [this message]
2016-05-03 18:59               ` Pedro Alves
2016-05-04  0:33                 ` Trevor Saunders
2016-05-04  1:03                   ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1605031545470.21846@tp.orcam.me.uk \
    --to=macro@imgtec.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=qiyaoltc@gmail.com \
    --cc=tbsaunde+binutils@tbsaunde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).