Re: [PATCH,GOLD] direct bindings support

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

On 3/11/13, Liang, Zhenyu <zhenyu.liang@intel.com> wrote:
> I'm trying to add direct bindings support to GNU binutils. This feature was
> introduced by Solaris linker. See
> http://docs.oracle.com/cd/E19963-01/html/819-0690/gejgf.html for more
> details.
> Here is my first patch for gold linker. Currently, support -Bdirect,
> -Bnodirect and -z [direct|nodirect] options.
> Request to review.

This looks pretty good.  Do you have a copyright assignment with the
FSF?  As far as I know Intel prefers to make copyright assignments on
an individual rather than a company basis.

In general comments should be complete sentences with proper
capitalization and punctuation.  A number of new comments needs to be
fixed.

> +  // Return the index of DT_NEEDED should be in the target shared object.

That comment doesn't quite make sense as written.  Should "of" be
"that"?  Same with the immediately following comment on set_dtndx.

> +  bool direct = false;

This variable should be named is_direct.

> +      Output_section_data* sidata = new Output_data_fixed_space(index * 4, 4,
> +								"** syminfo");

Rather than a hard-coded 4 in "index * 4" I'd prefer to add the struct
to Elf_sizes in elfcpp.h and use that.

> +  const section_size_type syminfo_size = dynamic_count * 4;

Use Elf_sizes here too.

> +	      unsigned char* pi = syminfo_view + (dynsym_index * 4);

Here too.

Thanks.

Ian