Re: Plugin interfaces to do Pettis Hansen style code layout in the gold linker.

[Sriraman Tallam <tmsriram at google dot com>]

Hi,

   I made all the changes. It is true that I always pass non-negative
values as handles. The comment was a left-over from a previous
implementation that I forgot to cleanup, sorry. I have attached the
new patch with all the changes made.

Thanks,
-Sri.

On Mon, Mar 14, 2011 at 11:22 AM, Cary Coutant <ccoutant@google.com> wrote:
>> ?Like we discussed, I made all the changes. I now have a separate
>> list of unclaimed objects in the Plugin manager and the handle will be
>> an index into it. I have also added a new hook called
>> "inspect_unclaimed_object". This handler allows the plugin to examine
>> the contents of an unclaimed object. Now, I have disallowed examining
>> claimed objects here as the methods to get their contents etc. are not
>> defined in Pluginobj. Also, I have kept the original interfaces to get
>> section count or type or contents. I think I would need this interface
>> for my work rather than only get contents based on filters. I guess
>> more interfaces can be added easily.
>
> Index: main.cc
> ===================================================================
> + ?const char* section_ordering_file
> + ? ?= parameters->options().section_ordering_file();
>
> We usually indent continuations like this by 4 spaces.
>
>
> Index: plugin.cc
> ===================================================================
> +register_inspect_unclaimed_object(ld_plugin_inspect_unclaimed_object_handler
> handler);
>
> Line too long.
>
> +Plugin_manager::get_unclaimed_object(const void* handle)
> +{
> + ?unsigned int index = ?static_cast<unsigned
> int>(reinterpret_cast<intptr_t>(handle));
>
> Line too long.
>
>
> +static enum ld_plugin_status
> +register_inspect_unclaimed_object
> + ?(ld_plugin_inspect_unclaimed_object_handler handler)
> +{
> + ?gold_assert(parameters->options().has_plugins());
> + ?parameters->options().plugins()->set_inspect_unclaimed_object_handler(
> + ? ?handler);
>
> Also indent here by 4 spaces.
>
> + ?const unsigned char* section_contents = obj->section_contents(shndx, &plen,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?false);
>
> I think the indentation here is not quite correct.
>
>
> Index: plugin.h
> ===================================================================
> + ?// Register a inspect_unclaimed_object handler.
> + ?void
> + ?set_inspect_unclaimed_object_handler
> + ? ?(ld_plugin_inspect_unclaimed_object_handler handler)
>
> The left paren should be at the end of the line above, and the
> continuation line indented 4 spaces.
>
> + ?// Register a inspect_unclaimed_object handler.
> + ?void
> + ?set_inspect_unclaimed_object_handler
> + ? ?(ld_plugin_inspect_unclaimed_object_handler handler)
>
> Same here.
>
> + ?// The list of unclaimed objects. ?The negative of the index of an
> + ?// in this list serves as the "handle" that we pass to the plugins.
> + ?// Postive integer values for "handle" are for the claimed objects
> + ?// and negative values are for the unclaimed objects.
>
> It doesn't look like this is how you implemented it -- I think you're
> returning straight non-negative indices in both cases. Also, remember
> that 0 is used as a handle for the first claimed object, so it should
> really be negative vs. "non-negative".
>
> This looks good to me with these changes. (You need Ian's approval to
> commit, though.)
>
> Thanks!
>
> -cary
>

Attachment: gold_patch_new.txt
Description: Text document