Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: Doug Evans <dje@google.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA] Remove target_section.bfd
Date: Tue, 16 Jul 2013 18:13:00 -0000	[thread overview]
Message-ID: <87oba2jruh.fsf@fleche.redhat.com> (raw)
In-Reply-To: <yjt2ip0bbg2u.fsf@ruffy.mtv.corp.google.com> (Doug Evans's	message of "Mon, 15 Jul 2013 15:42:33 -0700")

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> While reviewing the remove-symbol-file patchset I came across
Doug> target_section.bfd.
Doug> Unnecessarily storing copies of things can lead to confusion and bugs.
Doug> This patch isn't intended to be a space savings, simply a cleanup.

I think it is a nice cleanup.  Thanks.

Doug> We already liberally reference bfd_section.owner, so I have no problem
Doug> with adding more references here.

The only danger is that some BFD sections do not have an owner.  For
example, this is true of the absolute section, which is shared by all
BFDs, something I found out the hard way.  Now, I think this is wrong of
BFD to do, but fixing it seemed hard.

Sometimes it is safe to use the section owner nevertheless; for instance
if you're sure that an ownerless section will never be used in this
context.

I don't know whether this applies in this case.  Perhaps not because
build_section_table uses bfd_map_over_sections, and I think that doesn't
include the ownerless sections.

Doug> [Ideally bfd would provide accessor functions/macros for struct
Doug> bfd_section, but none exist at all, and I'm not inclined to add all of
Doug> them just for this patch.]

I went through that same thought process.

Doug> Ok to check in?

I think it is fine; but if you could verify about bfd_map_over_sections,
that would be good.

Tom


  reply	other threads:[~2013-07-16 18:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-15 22:42 Doug Evans
2013-07-16 18:13 ` Tom Tromey [this message]
2013-07-16 18:27   ` Doug Evans
2013-07-16 18:39     ` Tom Tromey
2013-07-17  4:33 ` Build regression with --enable-targets=all [Re: [RFA] Remove target_section.bfd] Jan Kratochvil
2013-07-17  5:31   ` Doug Evans
2013-07-18 10:54 ` Runtime regression for gdb.base/reread.exp & co. " Jan Kratochvil
2013-07-18 16:24   ` Doug Evans
2013-07-18 16:33     ` Jan Kratochvil
2013-07-19  0:20       ` Doug Evans
2013-07-22 16:25         ` Tom Tromey
2013-07-22 20:53           ` [commit] " Jan Kratochvil
2013-07-19  0:27       ` Doug Evans

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=87oba2jruh.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=dje@google.com \
    --cc=gdb-patches@sourceware.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