Skip to content

Commit

Permalink
Rename library_exists_in_dir to file_exists_in_dir and remove som…
Browse files Browse the repository at this point in the history
…e unnecessary string allocations (dotnet#95154)
  • Loading branch information
elinor-fung authored Nov 27, 2023
1 parent f38242b commit a1db638
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 21 deletions.
3 changes: 2 additions & 1 deletion src/native/corehost/fxr/framework_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ bool compare_by_name_and_version(const framework_info &a, const framework_info &

trace::verbose(_X("Gathering FX locations in [%s]"), fx_dir.c_str());

const pal::string_t deps_file_name = fx_name_local + _X(".deps.json");
std::vector<pal::string_t> versions;
pal::readdir_onlydirectories(fx_dir, &versions);
for (const pal::string_t& ver : versions)
Expand All @@ -85,7 +86,7 @@ bool compare_by_name_and_version(const framework_info &a, const framework_info &
// Check that the framework's .deps.json exists.
pal::string_t fx_version_dir = fx_dir;
append_path(&fx_version_dir, ver.c_str());
if (!library_exists_in_dir(fx_version_dir, fx_name_local + _X(".deps.json"), nullptr))
if (!file_exists_in_dir(fx_version_dir, deps_file_name.c_str(), nullptr))
{
trace::verbose(_X("Ignoring FX version [%s] without .deps.json"), ver.c_str());
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/native/corehost/fxr/fx_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ namespace
fx_ref.get_fx_version().c_str());

append_path(&fx_dir, fx_ref.get_fx_version().c_str());
if (library_exists_in_dir(fx_dir, deps_file_name, nullptr))
if (file_exists_in_dir(fx_dir, deps_file_name.c_str(), nullptr))
{
selected_fx_dir = fx_dir;
selected_fx_version = fx_ref.get_fx_version();
Expand Down Expand Up @@ -266,7 +266,7 @@ namespace
// Check that the framework's .deps.json exists. To minimize the file checks done in the most common
// scenario (.deps.json exists), only check after resolving the version and if the .deps.json doesn't
// exist, attempt resolving again without that version.
if (!library_exists_in_dir(resolved_fx_dir, deps_file_name, nullptr))
if (!file_exists_in_dir(resolved_fx_dir, deps_file_name.c_str(), nullptr))
{
// Remove the version and try resolving again
trace::verbose(_X("Ignoring FX version [%s] without .deps.json"), resolved_ver_str.c_str());
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/fxr/sdk_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void sdk_info::enumerate_sdk_paths(
// Check for the existence of dotnet.dll
pal::string_t sdk_version_dir = sdk_dir;
append_path(&sdk_version_dir, version_str.c_str());
if (!library_exists_in_dir(sdk_version_dir, SDK_DOTNET_DLL, nullptr))
if (!file_exists_in_dir(sdk_version_dir, SDK_DOTNET_DLL, nullptr))
{
trace::verbose(_X("Ignoring version [%s] without ") SDK_DOTNET_DLL, version_str.c_str());
continue;
Expand Down
6 changes: 3 additions & 3 deletions src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ namespace
append_path(&path, rel_dir.c_str()); // relative dir containing hostpolicy library

// Check if "path" contains the required library.
if (!library_exists_in_dir(path, LIBHOSTPOLICY_NAME, nullptr))
if (!file_exists_in_dir(path, LIBHOSTPOLICY_NAME, nullptr))
{
trace::verbose(_X("Did not find %s in directory %s"), LIBHOSTPOLICY_NAME, path.c_str());
return false;
Expand Down Expand Up @@ -175,7 +175,7 @@ int hostpolicy_resolver::load(
if (g_hostpolicy == nullptr)
{
pal::string_t host_path;
if (!library_exists_in_dir(lib_dir, LIBHOSTPOLICY_NAME, &host_path))
if (!file_exists_in_dir(lib_dir, LIBHOSTPOLICY_NAME, &host_path))
{
return StatusCode::CoreHostLibMissingFailure;
}
Expand Down Expand Up @@ -289,7 +289,7 @@ bool hostpolicy_resolver::try_get_dir(

// Check if hostpolicy exists in "expected" directory.
trace::verbose(_X("The expected %s directory is [%s]"), LIBHOSTPOLICY_NAME, expected.c_str());
if (library_exists_in_dir(expected, LIBHOSTPOLICY_NAME, nullptr))
if (file_exists_in_dir(expected, LIBHOSTPOLICY_NAME, nullptr))
{
impl_dir->assign(expected);
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/native/corehost/fxr_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace
append_path(&fxr_root, max_ver_str.c_str());
trace::info(_X("Detected latest fxr version=[%s]..."), fxr_root.c_str());

if (library_exists_in_dir(fxr_root, LIBFXR_NAME, out_fxr_path))
if (file_exists_in_dir(fxr_root, LIBFXR_NAME, out_fxr_path))
{
trace::info(_X("Resolved fxr [%s]..."), out_fxr_path->c_str());
return true;
Expand All @@ -58,7 +58,7 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o
// For apphost and libhost, root_path is expected to be a directory.
// For libhost, it may be empty if app-local search is not desired (e.g. com/ijw/winrt hosts, nethost when no assembly path is specified)
// If a hostfxr exists in root_path, then assume self-contained.
if (root_path.length() > 0 && library_exists_in_dir(root_path, LIBFXR_NAME, out_fxr_path))
if (root_path.length() > 0 && file_exists_in_dir(root_path, LIBFXR_NAME, out_fxr_path))
{
trace::info(_X("Resolved fxr [%s]..."), out_fxr_path->c_str());
out_dotnet_root->assign(root_path);
Expand Down
18 changes: 8 additions & 10 deletions src/native/corehost/hostmisc/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,17 @@
#include <_version.c>
#endif

bool library_exists_in_dir(const pal::string_t& lib_dir, const pal::string_t& lib_name, pal::string_t* p_lib_path)
bool file_exists_in_dir(const pal::string_t& dir, const pal::char_t* file_name, pal::string_t* out_file_path)
{
pal::string_t lib_path = lib_dir;
append_path(&lib_path, lib_name.c_str());
pal::string_t file_path = dir;
append_path(&file_path, file_name);

if (!pal::file_exists(lib_path))
{
if (!pal::file_exists(file_path))
return false;
}
if (p_lib_path)
{
*p_lib_path = lib_path;
}

if (out_file_path)
*out_file_path = file_path;

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/hostmisc/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pal::string_t strip_file_ext(const pal::string_t& path);
pal::string_t get_filename(const pal::string_t& path);
pal::string_t get_filename_without_ext(const pal::string_t& path);
void append_path(pal::string_t* path1, const pal::char_t* path2);
bool library_exists_in_dir(const pal::string_t& lib_dir, const pal::string_t& lib_name, pal::string_t* p_lib_path);
bool file_exists_in_dir(const pal::string_t& dir, const pal::char_t* file_name, pal::string_t* out_file_path);
bool coreclr_exists_in_dir(const pal::string_t& candidate);
void remove_trailing_dir_separator(pal::string_t* dir);
void replace_char(pal::string_t* path, pal::char_t match, pal::char_t repl);
Expand Down
4 changes: 3 additions & 1 deletion src/native/corehost/hostpolicy/deps_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,9 @@ bool deps_resolver_t::resolve_probe_dirs(
// App local path
add_unique_path(asset_type, m_app_dir, &items, output, &non_serviced, core_servicing);

(void) library_exists_in_dir(m_app_dir, LIBCORECLR_NAME, &m_coreclr_path);
// deps_resolver treats being able to get the coreclr path as optional, so we ignore the return value here.
// The caller is responsible for checking whether coreclr path is set and handling as appropriate.
(void) file_exists_in_dir(m_app_dir, LIBCORECLR_NAME, &m_coreclr_path);
}

// Handle any additional deps.json that were specified.
Expand Down

0 comments on commit a1db638

Please sign in to comment.