Home Original page

[gold][patch] Small code refactoring

Ian Lance Taylor iant@google.com
Fri Feb 13 15:23:00 GMT 2009 
Rafael Espindola <espindola@google.com> writes:

> 2009/2/12 Rafael Espindola <espindola@google.com>:

> 2009-02-12  Rafael Avila de Espindola  <espindola@google.com>
> 
> 	* archive.cc (Archive::get_elf_object_for_member): Update for
> 	new make_elf_object signature.
> 	* object.cc (make_elf_object): Rename to make_elf_object2. Make it
> 	static.
> 	(make_elf_object): New.
> 	* object.h (make_elf_object): Update signature.
> 	* readsyms.cc (Read_symbols::do_read_symbols): Update for
> 	new make_elf_object signature.
> 	* testsuite/binary_unittest.cc (Sized_binary_test): Update for
> 	new make_elf_object signature.
> 	* testsuite/object_unittest.cc (Sized_object_test): Update for
> 	new make_elf_object signature.



> +  if (!o)
> +    gold_error(_("%s: member at %zu is not an ELF object"),
> +               this->name().c_str(), static_cast<size_t>(off));
 
Write "if (o == NULL)" rather than "if (!o)".


Actually, I think you are taking slightly the wrong approach here.  The
two cases have different error handling behaviour--that's why they wound
up being different.  In the archive case, if it's not an ELF file, we
fail immediately.  In the readsyms case, we go on to check for a linker
script.

Also, your version calls get_view twice in the normal case.  gold gets a
surprising amount of its speed by carefully avoiding these sorts of
micro-deoptimizations in the normal case.  I spent a lot of time
profiling to find them.

One approach here would be to write a predicate is_elf_object which
takes read_size and ehdr and returns whether it looks like ELF.  Then if
is_elf_object returns true, call make_elf_object.

Thanks for looking at this.

Ian
More information about the Binutils mailing list