This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][patch] Small code refactoring
> 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.
The attached patch implements a small variation. is_elf_object calls
get_view and writes it back to the caller. With this I can remove the
duplicated call to gold_error in Archive::get_elf_object_for_member.
2009-02-14 Rafael Avila de Espindola <espindola@google.com>
* archive.cc (Archive::get_elf_object_for_member): Use is_elf_object.
* object.cc (is_elf_object): New.
* object.h (is_elf_object): New.
* readsyms.cc (Read_symbols::do_read_symbols): Use is_elf_object.
> Thanks for looking at this.
>
> Ian
>
Cheers,
--
Rafael Avila de Espindola
Google | Gordon House | Barrow Street | Dublin 4 | Ireland
Registered in Dublin, Ireland | Registration Number: 368047
diff --git a/gold/archive.cc b/gold/archive.cc
index b1ba6d9..21bdbe9 100644
--- a/gold/archive.cc
+++ b/gold/archive.cc
@@ -544,22 +544,9 @@ Archive::get_elf_object_for_member(off_t off, Input_objects* input_objects)
if (filesize - memoff < read_size)
read_size = filesize - memoff;
- if (read_size < 4)
- {
- gold_error(_("%s: member at %zu is not an ELF object"),
- this->name().c_str(), static_cast<size_t>(off));
- return NULL;
- }
-
- const unsigned char* ehdr = input_file->file().get_view(memoff, 0, read_size,
- true, false);
-
- static unsigned char elfmagic[4] =
- {
- elfcpp::ELFMAG0, elfcpp::ELFMAG1,
- elfcpp::ELFMAG2, elfcpp::ELFMAG3
- };
- if (memcmp(ehdr, elfmagic, 4) != 0)
+ const unsigned char* ehdr;
+
+ if (!is_elf_object(input_file, memoff, read_size, &ehdr))
{
gold_error(_("%s: member at %zu is not an ELF object"),
this->name().c_str(), static_cast<size_t>(off));
diff --git a/gold/object.cc b/gold/object.cc
index aa45127..76d8b78 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -2112,6 +2112,25 @@ make_elf_sized_object(const std::string& name, Input_file* input_file,
namespace gold
{
+bool
+is_elf_object(Input_file* input_file, off_t offset, int read_size,
+ const unsigned char** ehdr)
+{
+ if (read_size < 4)
+ return false;
+
+ const unsigned char* p = input_file->file().get_view(offset, 0, read_size,
+ true, false);
+ *ehdr = p;
+
+ static unsigned char elfmagic[4] =
+ {
+ elfcpp::ELFMAG0, elfcpp::ELFMAG1,
+ elfcpp::ELFMAG2, elfcpp::ELFMAG3
+ };
+ return memcmp(p, elfmagic, 4) == 0;
+}
+
// Read an ELF file and return the appropriate instance of Object.
Object*
diff --git a/gold/object.h b/gold/object.h
index 614a02e..7447927 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -1950,6 +1950,10 @@ make_elf_object(const std::string& name, Input_file*,
off_t offset, const unsigned char* p,
section_offset_type bytes);
+bool
+is_elf_object(Input_file* input_file, off_t offset, int read_size,
+ const unsigned char** ehdr);
+
} // end namespace gold
#endif // !defined(GOLD_OBJECT_H)
diff --git a/gold/readsyms.cc b/gold/readsyms.cc
index 412ffcd..6e35e03 100644
--- a/gold/readsyms.cc
+++ b/gold/readsyms.cc
@@ -157,8 +157,9 @@ Read_symbols::do_read_symbols(Workqueue* workqueue)
if (filesize < read_size)
read_size = filesize;
- const unsigned char* ehdr = input_file->file().get_view(0, 0, read_size,
- true, false);
+ const unsigned char* ehdr;
+
+ bool is_elf = is_elf_object(input_file, 0, read_size, &ehdr);
if (read_size >= Archive::sarmag)
{
@@ -209,46 +210,38 @@ Read_symbols::do_read_symbols(Workqueue* workqueue)
}
}
- if (read_size >= 4)
+ if (is_elf)
{
- static unsigned char elfmagic[4] =
- {
- elfcpp::ELFMAG0, elfcpp::ELFMAG1,
- elfcpp::ELFMAG2, elfcpp::ELFMAG3
- };
- if (memcmp(ehdr, elfmagic, 4) == 0)
- {
- // This is an ELF object.
+ // This is an ELF object.
- Object* obj = make_elf_object(input_file->filename(),
- input_file, 0, ehdr, read_size);
- if (obj == NULL)
- return false;
+ Object* obj = make_elf_object(input_file->filename(),
+ input_file, 0, ehdr, read_size);
+ if (obj == NULL)
+ return false;
- Read_symbols_data* sd = new Read_symbols_data;
- obj->read_symbols(sd);
+ Read_symbols_data* sd = new Read_symbols_data;
+ obj->read_symbols(sd);
- // Opening the file locked it, so now we need to unlock it.
- // We need to unlock it before queuing the Add_symbols task,
- // because the workqueue doesn't know about our lock on the
- // file. If we queue the Add_symbols task first, it will be
- // stuck on the end of the file lock, but since the
- // workqueue doesn't know about that lock, it will never
- // release the Add_symbols task.
+ // Opening the file locked it, so now we need to unlock it.
+ // We need to unlock it before queuing the Add_symbols task,
+ // because the workqueue doesn't know about our lock on the
+ // file. If we queue the Add_symbols task first, it will be
+ // stuck on the end of the file lock, but since the
+ // workqueue doesn't know about that lock, it will never
+ // release the Add_symbols task.
- input_file->file().unlock(this);
+ input_file->file().unlock(this);
- // We use queue_next because everything is cached for this
- // task to run right away if possible.
+ // We use queue_next because everything is cached for this
+ // task to run right away if possible.
- workqueue->queue_next(new Add_symbols(this->input_objects_,
- this->symtab_, this->layout_,
- obj, sd,
- this->this_blocker_,
- this->next_blocker_));
+ workqueue->queue_next(new Add_symbols(this->input_objects_,
+ this->symtab_, this->layout_,
+ obj, sd,
+ this->this_blocker_,
+ this->next_blocker_));
- return true;
- }
+ return true;
}
// Queue up a task to try to parse this file as a script. We use a