Re: [patch 6/7] [gold] Add initial source code for dwp utility.

[Ian Lance Taylor <iant at google dot com>]

On Thu, Oct 18, 2012 at 11:16 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.

> +dwp_SOURCES = dwp.cc
> +dwp_DEPENDENCIES = libgold.a $(LIBIBERTY) $(LIBINTL_DEP)
> +dwp_LDADD = libgold.a $(LIBIBERTY) $(LIBINTL) $(THREADSLIB) $(LIBDL)

In practice the same arguments that caused us to add $(GOLD_LDADD) and
$(GOLD_LDFLAGS) are going to apply here.  $(GOLD_LDADD) should be
added to dwp_LDADD and $(GOLD_LDFLAGS should be in a new dwp_LDFLAGS
variable.


> +// An ELF input file.
> +// We derive from Sized_relobj so that we can use interfaces
> +// in libgold to access the file.

Did you consider deriving from Sized_relobj_file?  That would let you
skip defining some of the unreachable virtual funtions.  It would also
give you an Elf_file accessible via elf_file().



> + private:
> +  // General access to the ELF file.
> +  elfcpp::Elf_file<size, big_endian, Object> elf_file_;
> +
> +};

Extra blank line before }.



> +// 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.


> +  // Collect file names and options.
> +  typedef std::vector<char*> File_list;
> +  File_list files;
> +  const char* output_filename = NULL;
> +  bool no_more_options = false;
> +  bool verbose = false;
> +  for (int i = 1; i < argc; ++i)
> +    {
> +      if (no_more_options || argv[i][0] != '-')
> +       files.push_back(argv[i]);

Use getopt here.  gold can't easily use getopt because it cares about
the ordering of option and non-option arguments.  But this program can
use getopt.  There are examples over in the binutils directory.


> +#ifdef ENABLE_NLS
> +# include <libintl.h>
> +# define _(String) gettext (String)
> +# ifdef gettext_noop
> +#  define N_(String) gettext_noop (String)
> +# else
> +#  define N_(String) (String)
> +# endif
> +#else
> +# define gettext(Msgid) (Msgid)
> +# define dgettext(Domainname, Msgid) (Msgid)
> +# define dcgettext(Domainname, Msgid, Category) (Msgid)
> +# define textdomain(Domainname) do {} while (0) /* nothing */
> +# define bindtextdomain(Domainname, Dirname) do {} while (0) /* nothing */
> +# define _(String) (String)
> +# define N_(String) (String)
> +#endif
> +
> +// Figure out how to get a hash set and a hash map.

Let's pull this stuff copied from gold.h into a shared header file,
perhaps system.h.

Ian