This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][patch] Add a plugin callback for setting the extra search path
- From: Cary Coutant <ccoutant at google dot com>
- To: Rafael Espindola <espindola at google dot com>
- Cc: Binutils <binutils at sourceware dot org>, Ian Lance Taylor <iant at google dot com>
- Date: Fri, 18 Jun 2010 15:02:33 -0700
- Subject: Re: [gold][patch] Add a plugin callback for setting the extra search path
- References: <AANLkTildio1BtNdPhe-MAMox0CZtX8cTuHaIOBMuenDA@mail.gmail.com>
> include/
> 2010-06-18 ?Rafael Espindola ?<espindola@google.com>
>
> ? ? ? ?* plugin-api.h (ld_plugin_set_extra_library_path): New.
> ? ? ? ?(ld_plugin_tag): Add LDPT_SET_EXTRA_LIBRARY_PATH.
> ? ? ? ?(ld_plugin_tv): Add tv_set_extra_library_path.
>
> gold/
> 2010-06-18 ?Rafael Espindola ?<espindola@google.com>
>
> ? ? ? ?* fileread.cc (Input_file::find_fie): New
> ? ? ? ?(Input_file::open): Use Input_file::find_fie.
> ? ? ? ?* fileread.h (Input_file::find_fie): New
> ? ? ? ?* plugin.cc (set_extra_library_path): New.
> ? ? ? ?(Plugin::load): Add set_extra_library_path to the transfer vector.
> ? ? ? ?(Plugin_manager::set_extra_library_path): New.
> ? ? ? ?(Plugin_manager::add_input_file): Use the extra search path if set.
> ? ? ? ?(set_extra_library_path(): New.
> ? ? ? ?* plugin.h (Plugin_manager): Adde set_extra_library_path and
> ? ? ? ?extra_search_path_.
s/Adde/Add/
-// Open the file.
-
-// If the filename is not absolute, we assume it is in the current
-// directory *except* when:
-// A) input_argument_->is_lib() is true;
-// B) input_argument_->is_searched_file() is true; or
-// C) input_argument_->extra_search_path() is not empty.
-// In each, we look in extra_search_path + library_path to find
-// the file location, rather than the current directory.
-
bool
-Input_file::open(const Dirsearch& dirpath, const Task* task, int *pindex)
+Input_file::find_file(const Dirsearch& dirpath, int *pindex,
std::string *namep)
find_file() needs a top comment, but I think the block of comments
under "Open the file" should go with the find_file() rather than
remain with open().
I'd suggest that you pass input_argument, is_in_sysroot, and
&found_name as additional parameters to find_file(), and that should
be sufficient to allow find_file() to live outside the Input_file
class.
While I'm not opposed to factoring find_file() out from open(), it's
not at all clear why you bothered -- there's still only the one call
to it, isn't there? Are you planning to make use of this routine in a
future patch?
+ if (this->input_argument_->extra_search_path())
+ {
+ // First, check extra_search_path.
+ std::string prefix = this->input_argument_->extra_search_path();
+ if (!IS_DIR_SEPARATOR (prefix[prefix.length() - 1]))
+ prefix += '/';
+
+ name = prefix + n1;
+ struct stat dummy_stat;
+ if (*pindex <= 0 && ::stat(name.c_str(), &dummy_stat) >= 0)
+ {
+ this->found_name_ = this->input_argument_->name();
+ *namep = name;
+ return true;
+ }
+
+ name = prefix + n2;
+ if (!n2.empty() && *pindex <= 0
+ && ::stat(name.c_str(), &dummy_stat) >= 0)
+ {
+ this->found_name_ = this->input_argument_->name();
+ *namep = name;
+ return true;
+ }
+ }
I think this code, which is nearly identical with that under "Case 4"
should be refactored into its own function (which does not need to be
in the class, either).
+ // an extra directory to look for the libraries passed by
+ // add_input_library.
Capitalize first word of the sentence. s/look/search/.
-cary