From: Joel Brobecker <brobecker@adacore.com>
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: sanimir.agovic@intel.com, gdb-patches@sourceware.org
Subject: Re: [PATCH] cleanup: remove declared variables without any references
Date: Wed, 12 Feb 2014 03:32:00 -0000 [thread overview]
Message-ID: <20140212033234.GQ5485@adacore.com> (raw)
In-Reply-To: <201402111412.s1BECWEX000161@glazunov.sibelius.xs4all.nl>
Hello Sanimir,
Thanks for sending the patches!
As you can see, bunching the changes together is causing a little
extra work, now, because multiple reviews end up being involved,
and we end up having to keep track of what was already reviewed,
and what has not.
I know it's a lot of files, but all these changes should be mostly
independent, so we could go one file at a time, a few files each day.
When we've finished reviewing the a batch, just send another. WDYT?
>
> The mipsnbsd-tdep.c one is not ok. For one thing, you're leaving
> behind the comment describing the code. But I think this is better
> left alone, as somebody might want to add (or put back) the missing
> glue that uses those bits.
Hey Mark! The problem with leaving them behind is that they cause
a build failure when building the HEAD, I think, because we build
with -Werror. Can we find a way to avoid that? For instance, could
we #if zero the code?
> > 2014-02-11 Sanimir Agovic <sanimir.agovic@intel.com>
> >
> > * cris-tdep.c (cris_supply_gregset): Remove zerobuf.
> > (_initialize_cris_tdep): Remove cris_set_cmdlist, cris_show_cmdlist.
> > * h8300-tdep.c (setmachinelist): Remove global.
> > * hppa-tdep.c (hppa_sigtramp): Remove global.
> > * ia64-tdep.c (fp_regnum, lr_regnum): Remove globals.
> > * mipsnbsd-tdep.c (sigtramp_retcode_mipsel, sigtramp_retcode_mipseb):
> > Remove globals.
> > * ravenscar-thread.c (update_target_observer): Remove global.
> > * rs6000-tdep.c (rs6000_gdbarch_init): Remove segment_regs.
> > * symfile-debug.c (debug_sym_fns): Remove global.
The h8300-tdep.c, ravenscar-thread.c changes are OK.
the ia64-tdep.c is officially Jan's jurisdiction, but the change looks
OK to me. If he doesn't object by, say, end of this week, please apply
the change (Jan: I'll revert immediately if you object).
The rs6000-tdep.c change is OK. It looks like it was already
unused when it was introduced, and I think it was an unintentional
left over from the development phase where the table was first
defined at one location, and then later moved to a different location
(init_sim_regno_table). See commit:
commit 7cc46491b1832408d1d5472e3f0c17c8a1d78ccb
Date: Mon Oct 15 19:45:31 2007 +0000
This leaves symfile-debug.c, for which I don't know.
--
Joel
next prev parent reply other threads:[~2014-02-12 3:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-11 13:55 Sanimir Agovic
2014-02-11 14:12 ` Mark Kettenis
2014-02-11 15:17 ` Agovic, Sanimir
2014-02-12 3:32 ` Joel Brobecker [this message]
2014-02-12 10:58 ` Mark Kettenis
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=20140212033234.GQ5485@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=mark.kettenis@xs4all.nl \
--cc=sanimir.agovic@intel.com \
/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