This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][patch] Add the --start-lib end --end-lib options
- From: Ian Lance Taylor <iant at google dot com>
- To: Rafael Espindola <espindola at google dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Fri, 19 Mar 2010 08:14:32 -0700
- Subject: Re: [gold][patch] Add the --start-lib end --end-lib options
- References: <38a0d8451003171324h5705010ex5e7fb8113e3a74ba@mail.gmail.com>
Rafael Espindola <espindola@google.com> writes:
> 2010-03-17 Rafael Espindola <espindola@google.com>
>
> * archive.cc (Should_include): Move to archive.h.
> (should_include_member): Make it a member of Archive.
> (Lib_group): New.
> (Add_lib_group_symbols): New.
> * archive.h: Include options.h.
> (Archive_member): Moved from Archive.
> (Should_include): Moved from archive.cc.
> (Lib_group): New.
> (Add_lib_group_symbols): New.
> * dynobj.cc (do_should_include_member): New.
> * dynobj.h (do_should_include_member): New.
> * gold.cc (queue_initial_tasks): Update call to queue.
> * main.cc (main): Print lib group stats.
> * object.cc (do_should_include_member): New.
> * object.h: Include archive.h.
> (Object::should_include_member): New.
> (Object::do_should_include_member): New.
> (Sized_relobj::do_should_include_member): New.
> * options.cc (General_options::parse_start_lib): New.
> (General_options::parse_end_lib): New.
> (Input_arguments::add_file): Handle lib groups.
> (Input_arguments::start_group): Check we are not in a lib.
> (Input_arguments::start_lib): New.
> (Input_arguments::end_lib): New.
> * options.h (General_options): Add start_lib and end_lib.
> (Input_argument::lib_): New.
> (Input_argument::lib): New.
> (Input_argument::is_lib): New.
> (Input_file_lib): New.
> (Input_arguments::in_lib_): New.
> (Input_arguments::in_lib): New.
> (Input_arguments::start_lib): New.
> (Input_arguments::end_lib_): New.
> * plugin.cc (Pluginobj::get_symbol_resolution_info): Mark symbols
> in unused members as preempted.
> (Sized_pluginobj::do_should_include_member): New.
> * plugin.h (Sized_pluginobj::do_should_include_member): New.
> * readsyms.cc (Read_symbols::locks): If we are just reading a member,
> return the blocker.
> (Read_symbols::do_whole_lib_group): New.
> (Read_symbols::do_lib_group): New.
> (Read_symbols::do_read_symbols): Handle lib groups.
> (Read_symbols::get_name): Handle lib groups.
> * readsyms.h (Read_symbols): Add an archive member pointer.
> (Read_symbols::do_whole_lib_group): New.
> (Read_symbols::do_lib_group): New.
> (Read_symbols::member_): New.
> * script.cc (read_input_script): Update call to queue_soon.
> - if (t == SHOULD_INCLUDE_NO || t == SHOULD_INCLUDE_YES)
> + if (t == Archive::SHOULD_INCLUDE_NO ||
> + t == Archive::SHOULD_INCLUDE_YES)
> this->armap_checked_[i] = true;
Move the "||" to the start of the second line of the conditional.
> + // Skip files with no symbols. Plugin objects have
> + // member.sd_ == NULL
Period at end of sentence.
> + if (obj != NULL &&
> + (member.sd_ == NULL || member.sd_->symbol_names != NULL))
Move "&&" to start of second line of conditional.
> + if (member.sd_)
> + delete member.sd_;
Use an explicit member.sd_ != NULL.
> --- a/gold/archive.h
> +++ b/gold/archive.h
> @@ -28,6 +28,7 @@
>
> #include "fileread.h"
> #include "workqueue.h"
> +#include "options.h"
I don't see why this #include is necessary. Please don't add it if it
is not needed.
> +// Include a lib group member in the link.
> +void
> +Lib_group::include_member(Symbol_table* symtab, Layout* layout,
> + Input_objects* input_objects,
> + const Archive_member& member)
> +{
Add a blank line after the comment.
> +// This class represents the files surrunded by a --start-lib ... --end-lib.
> +
> +class Lib_group
> +{
> + public:
> + Lib_group(const Input_file_lib* lib, Task* task)
> + : lib_(lib), task_(task), members_()
> + {
> + members_.resize(lib->size());
> + }
Write this->members_.
> +class Add_lib_group_symbols : public Task
> +{
This class needs a comment.
> +template<int size, bool big_endian>
> +Archive::Should_include
> +Sized_relobj<size, big_endian>::do_should_include_member(
> + Symbol_table* symtab, Read_symbols_data* sd,
> + std::string* why)
> +{
When the first parameter is on a new line, it should be indented only
four spaces, and each parameter should be on a separate line.
> +void
> +General_options::parse_start_lib(const char*, const char*,
> + Command_line* cmdline)
> +{
> + cmdline->inputs().start_lib(cmdline->position_dependent_options());
> +}
> +
> +void
> +General_options::parse_end_lib(const char*, const char*,
> + Command_line* cmdline)
> +{
> + cmdline->inputs().end_lib();
> +}
> +
> +
Only add one blank line here.
> @@ -1193,6 +1216,31 @@ Input_arguments::end_group()
> this->in_group_ = false;
> }
>
> +
> +// Start a lib.
> +
> +void
> +Input_arguments::start_lib(const Position_dependent_options& options)
> +{
Only one blank line before "Start a lib" comment.
> +
> + if (size_t(nsyms) > this->symbols_.size())
> + {
Write static_cast<size_t>(nsyms).
> void
> -Read_symbols::locks(Task_locker*)
> +Read_symbols::locks(Task_locker* tl)
> {
> + if(this->member_)
> + tl->add(this, this->next_blocker_);
Add space after "if". Write an explicit != NULL.
> @@ -246,6 +338,13 @@ Read_symbols::do_read_symbols(Workqueue* workqueue)
> // We are done with the file at this point, so unlock it.
> obj->unlock(this);
>
> + if (this->member_)
> + {
Write an explicit this->member_ != NULL.
> + if (this->member_)
> + {
Likewise.
> - Input_group* input_group, Task_token* this_blocker,
> - Task_token* next_blocker)
> + Input_group* input_group, Archive_member *member,
> + Task_token* this_blocker, Task_token* next_blocker)
s/Archive_member */Archive_member* /
This is OK with those changes.
Thanks.
Ian