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 6/7] [gold] Add initial source code for dwp utility.


On Fri, Oct 19, 2012 at 11:58 AM, Cary Coutant <ccoutant@google.com> wrote:
>>> This patch adds the new source files for the dwp utility itself, and updates
>>> the gold Makefiles to build it.
>>
>>> -noinst_PROGRAMS = ld-new incremental-dump
>>> +noinst_PROGRAMS = ld-new incremental-dump dwp
>>>  noinst_LIBRARIES = libgold.a
>>
>> Are we going to want to install dwp at some point?  Presumably, unlike
>> incremental-dump, we expect other packages to use it.
>
> Yes, I've made that change already. I left dwp in noinst_PROGRAMS, and
> added the commands to the install-exec-local rule -- I guess I could
> just add a "bin_PROGRAMS = dwp" statement instead and let automake
> take care of it?

I think putting it in bin_PROGRAMS would be fine.  The main thing to
check is that in the generated Makefile.in, the program name is passed
through sed '$(transform)' to get the name for the installed program.
If it is, then I think we are good.  I think the only reason the
linker proper is not in bin_PROGRAMS is so that we can call it ld-new
in the build directory and thus avoid various sorts of confusion.

> I also considered abstracting yet another layer between Relobj and
> Sized_relobj, or between Object and Relobj, so that I could have some
> basic functions for accessing a Relobj without all the linker-specific
> stuff. Multiple inheritance would probably have helped here, too --
> for Dwarf_info_reader, all I need is a small subset of the interfaces
> from Relobj. What's your policy on multiple inheritance --
> specifically in the case where a second base class would be pure
> interface? (This would probably be much easier in Go. (Too bad the
> name "gold" is already taken.))

Yes, I think multiple inheritance is fine when all but one of the
parent classes are pure abstract classes.  That is probably the way to
go here.

>>> +// Return the location of the contents of a section.
>>> +
>>> +template <int size, bool big_endian>
>>> +const unsigned char*
>>> +Sized_relobj_dwo<size, big_endian>::do_section_contents(
>>> +    unsigned int shndx,
>>> +    section_size_type* plen,
>>> +    bool cache)
>>
>> The comment seems to be wrong: this returns the section contents, not
>> the location.
>
> I think "location" means "pointer to" here. In object.h, we seem to
> alternate between "Return a view of the contents of a section" and
> "Return the location of the contents of a section". This was just
> copy/pasted from object.h.

OK.

Ian


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