This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] gold/arm: define $a/$d markers in .plt


On Wed, Apr 18, 2012 at 3:22 PM, Cary Coutant <ccoutant@google.com> wrote:
> We usually break long lines like this after the '=', and indent the
> next line 4 spaces:

That's a marked departure from the usual GNU indentation style, which would
be either what I wrote or breaking the line before the '=' and indenting
the new line two spaces.  But OK.

> Given the change above this one, we will not have added any STB_LOCAL
> symbols to the symbol table, so the force_local call is unnecessary in
> that case. If the symbol is not STB_LOCAL, but
> this->version_script_.symbol_is_local(name) is true, then we do still
> need to call force_local. So perhaps this would work:

That makes sense.

> For completeness, we should apply the same treatment to
> do_define_in_output_segment and do_define_as_constant. It'd be OK with
> me to defer that to a follow-up patch.

Ok, I'll leave it to you then.  IMHO it would be worthwhile to break out
the boilerplate of calling the appropriate template flavor of
define_special_symbol (and now sometimes not) into a common subroutine.

Hmm.  I notice that my addition doesn't #ifdef the use, so it's calling
both endian flavors even if a configuration doesn't have both.  I'll fix
that.

> It looks good to me with those changes, but you still need Ian's
> approval to commit.

I've made the two changes above in my local copy, and won't bother
reposting since you know exactly what it looks like.


Thanks,
Roland


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]