This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][patch] Small code refactoring
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