diff --git a/Userland/DevTools/UserspaceEmulator/main.cpp b/Userland/DevTools/UserspaceEmulator/main.cpp index b661b78ef8..f011b5442d 100644 --- a/Userland/DevTools/UserspaceEmulator/main.cpp +++ b/Userland/DevTools/UserspaceEmulator/main.cpp @@ -49,7 +49,7 @@ int main(int argc, char** argv, char** env) if (arguments[0].contains("/"sv)) executable_path = Core::File::real_path_for(arguments[0]); else - executable_path = Core::find_executable_in_path(arguments[0]); + executable_path = Core::File::resolve_executable_from_environment(arguments[0]).value_or({}); if (executable_path.is_empty()) { reportln("Cannot find executable for '{}'."sv, arguments[0]); return 1; diff --git a/Userland/Libraries/LibC/unistd.cpp b/Userland/Libraries/LibC/unistd.cpp index 0e44c2df5a..cd2f91bc89 100644 --- a/Userland/Libraries/LibC/unistd.cpp +++ b/Userland/Libraries/LibC/unistd.cpp @@ -179,12 +179,15 @@ int execve(char const* filename, char* const argv[], char* const envp[]) __RETURN_WITH_ERRNO(rc, rc, -1); } +// https://linux.die.net/man/3/execvpe (GNU extension) int execvpe(char const* filename, char* const argv[], char* const envp[]) { if (strchr(filename, '/')) return execve(filename, argv, envp); ScopedValueRollback errno_rollback(errno); + + // TODO: Make this use the PATH search implementation from Core::File. String path = getenv("PATH"); if (path.is_empty()) path = DEFAULT_PATH; diff --git a/Userland/Libraries/LibCore/DirIterator.cpp b/Userland/Libraries/LibCore/DirIterator.cpp index 527c36b579..36c9c8ea2c 100644 --- a/Userland/Libraries/LibCore/DirIterator.cpp +++ b/Userland/Libraries/LibCore/DirIterator.cpp @@ -95,28 +95,6 @@ String DirIterator::next_full_path() return builder.to_string(); } -String find_executable_in_path(String filename) -{ - if (filename.is_empty()) - return {}; - - if (filename.starts_with('/')) { - if (access(filename.characters(), X_OK) == 0) - return filename; - - return {}; - } - - for (auto directory : String { getenv("PATH") }.split(':')) { - auto fullpath = String::formatted("{}/{}", directory, filename); - - if (access(fullpath.characters(), X_OK) == 0) - return fullpath; - } - - return {}; -} - int DirIterator::fd() const { if (!m_dir) diff --git a/Userland/Libraries/LibCore/DirIterator.h b/Userland/Libraries/LibCore/DirIterator.h index 3fdd3d613c..e3a4784771 100644 --- a/Userland/Libraries/LibCore/DirIterator.h +++ b/Userland/Libraries/LibCore/DirIterator.h @@ -44,6 +44,4 @@ private: bool advance_next(); }; -String find_executable_in_path(String filename); - } diff --git a/Userland/Libraries/LibCore/File.cpp b/Userland/Libraries/LibCore/File.cpp index 436ac612bd..c3a83a5c85 100644 --- a/Userland/Libraries/LibCore/File.cpp +++ b/Userland/Libraries/LibCore/File.cpp @@ -557,4 +557,35 @@ ErrorOr File::remove(String const& path, RecursionMode return {}; } +Optional File::resolve_executable_from_environment(StringView filename) +{ + if (filename.is_empty()) + return {}; + + // Paths that aren't just a file name generally count as already resolved. + if (filename.contains('/')) { + if (access(String { filename }.characters(), X_OK) != 0) + return {}; + + return filename; + } + + auto const* path_str = getenv("PATH"); + StringView path { path_str, strlen(path_str) }; + + if (path.is_empty()) + path = DEFAULT_PATH_SV; + + auto directories = path.split_view(':'); + + for (auto directory : directories) { + auto file = String::formatted("{}/{}", directory, filename); + + if (access(file.characters(), X_OK) == 0) + return file; + } + + return {}; +}; + } diff --git a/Userland/Libraries/LibCore/File.h b/Userland/Libraries/LibCore/File.h index c331deaf66..02eb5ce34f 100644 --- a/Userland/Libraries/LibCore/File.h +++ b/Userland/Libraries/LibCore/File.h @@ -110,6 +110,8 @@ public: static NonnullRefPtr standard_output(); static NonnullRefPtr standard_error(); + static Optional resolve_executable_from_environment(StringView filename); + private: File(Object* parent = nullptr) : IODevice(parent) diff --git a/Userland/Libraries/LibCore/System.cpp b/Userland/Libraries/LibCore/System.cpp index 8ffcd93870..5497a1bff1 100644 --- a/Userland/Libraries/LibCore/System.cpp +++ b/Userland/Libraries/LibCore/System.cpp @@ -953,22 +953,6 @@ ErrorOr adjtime(const struct timeval* delta, struct timeval* old_delta) } #endif -ErrorOr find_file_in_path(StringView filename) -{ - auto const* path_ptr = getenv("PATH"); - StringView path { path_ptr, strlen(path_ptr) }; - if (path.is_empty()) - path = DEFAULT_PATH_SV; - auto parts = path.split_view(':'); - for (auto& part : parts) { - auto candidate = String::formatted("{}/{}", part, filename); - if (Core::File::exists(candidate)) { - return candidate; - } - } - return Error::from_errno(ENOENT); -} - ErrorOr exec(StringView filename, Span arguments, SearchInPath search_in_path, Optional> environment) { #ifdef __serenity__ @@ -1009,8 +993,18 @@ ErrorOr exec(StringView filename, Span arguments, SearchInPath return {}; }; - bool should_search_in_path = search_in_path == SearchInPath::Yes && !filename.contains('/'); - String exec_filename = should_search_in_path ? TRY(find_file_in_path(filename)) : filename.to_string(); + String exec_filename; + + if (search_in_path == SearchInPath::Yes) { + auto maybe_executable = Core::File::resolve_executable_from_environment(filename); + + if (!maybe_executable.has_value()) + return ENOENT; + + exec_filename = maybe_executable.release_value(); + } else { + exec_filename = filename.to_string(); + } params.path = { exec_filename.characters(), exec_filename.length() }; TRY(run_exec(params)); @@ -1039,21 +1033,16 @@ ErrorOr exec(StringView filename, Span arguments, SearchInPath if (search_in_path == SearchInPath::Yes && !filename.contains('/')) { # if defined(__APPLE__) || defined(__FreeBSD__) // These BSDs don't support execvpe(), so we'll have to manually search the PATH. - // This is copy-pasted from LibC's execvpe() with minor changes. ScopedValueRollback errno_rollback(errno); - String path = getenv("PATH"); - if (path.is_empty()) - path = DEFAULT_PATH; - auto parts = path.split(':'); - for (auto& part : parts) { - auto candidate = String::formatted("{}/{}", part, filename); - rc = ::execve(candidate.characters(), argv.data(), envp.data()); - if (rc < 0 && errno != ENOENT) { - errno_rollback.set_override_rollback_value(errno); - return Error::from_syscall("exec"sv, rc); - } + + auto maybe_executable = Core::File::resolve_executable_from_environment(filename_string); + + if (!maybe_executable.has_value()) { + errno_rollback.set_override_rollback_value(ENOENT); + return Error::from_errno(ENOENT); } - errno_rollback.set_override_rollback_value(ENOENT); + + rc = ::execve(maybe_executable.release_value().characters(), argv.data(), envp.data()); # else rc = ::execvpe(filename_string.characters(), argv.data(), envp.data()); # endif diff --git a/Userland/Libraries/LibCore/System.h b/Userland/Libraries/LibCore/System.h index 7875d8737d..56628527c2 100644 --- a/Userland/Libraries/LibCore/System.h +++ b/Userland/Libraries/LibCore/System.h @@ -161,7 +161,6 @@ ErrorOr> pipe2(int flags); #ifndef AK_OS_ANDROID ErrorOr adjtime(const struct timeval* delta, struct timeval* old_delta); #endif -ErrorOr find_file_in_path(StringView filename); enum class SearchInPath { No, Yes, diff --git a/Userland/Shell/Builtin.cpp b/Userland/Shell/Builtin.cpp index a0409ed3a2..caf9c8034b 100644 --- a/Userland/Shell/Builtin.cpp +++ b/Userland/Shell/Builtin.cpp @@ -232,9 +232,9 @@ int Shell::builtin_type(int argc, char const** argv) } // check if its an executable in PATH - auto fullpath = Core::find_executable_in_path(command); - if (!fullpath.is_empty()) { - printf("%s is %s\n", command.characters(), escape_token(fullpath).characters()); + auto fullpath = Core::File::resolve_executable_from_environment(command); + if (fullpath.has_value()) { + printf("%s is %s\n", command.characters(), escape_token(fullpath.release_value()).characters()); continue; } something_not_found = true; @@ -1075,12 +1075,12 @@ int Shell::builtin_kill(int argc, char const** argv) { // Simply translate the arguments and pass them to `kill' Vector replaced_values; - auto kill_path = find_in_path("kill"sv); - if (kill_path.is_empty()) { + auto kill_path = Core::File::resolve_executable_from_environment("kill"sv); + if (!kill_path.has_value()) { warnln("kill: `kill' not found in PATH"); return 126; } - replaced_values.append(kill_path); + replaced_values.append(kill_path.release_value()); for (auto i = 1; i < argc; ++i) { if (auto job_id = resolve_job_spec({ argv[i], strlen(argv[1]) }); job_id.has_value()) { auto job = find_job(job_id.value()); diff --git a/Userland/Shell/Shell.cpp b/Userland/Shell/Shell.cpp index 6a8fdc3d62..add8fb4977 100644 --- a/Userland/Shell/Shell.cpp +++ b/Userland/Shell/Shell.cpp @@ -1338,27 +1338,6 @@ String Shell::unescape_token(StringView token) return builder.build(); } -String Shell::find_in_path(StringView program_name) -{ - String path = getenv("PATH"); - if (!path.is_empty()) { - auto directories = path.split(':'); - for (auto const& directory : directories) { - Core::DirIterator programs(directory.characters(), Core::DirIterator::SkipDots); - while (programs.has_next()) { - auto program = programs.next_path(); - auto program_path = String::formatted("{}/{}", directory, program); - if (access(program_path.characters(), X_OK) != 0) - continue; - if (program == program_name) - return program_path; - } - } - } - - return {}; -} - void Shell::cache_path() { if (!m_is_interactive) @@ -1387,6 +1366,7 @@ void Shell::cache_path() cached_path.append({ RunnablePath::Kind::Alias, name }); } + // TODO: Can we make this rely on Core::File::resolve_executable_from_environment()? String path = getenv("PATH"); if (!path.is_empty()) { auto directories = path.split(':'); diff --git a/Userland/Shell/Shell.h b/Userland/Shell/Shell.h index 53d1c4d9c9..57b2f7ceff 100644 --- a/Userland/Shell/Shell.h +++ b/Userland/Shell/Shell.h @@ -157,8 +157,6 @@ public: String resolve_path(String) const; String resolve_alias(StringView) const; - static String find_in_path(StringView program_name); - static bool has_history_event(StringView); RefPtr get_argument(size_t) const; diff --git a/Userland/Utilities/pledge.cpp b/Userland/Utilities/pledge.cpp index 5d4b5e4653..90f41b357b 100644 --- a/Userland/Utilities/pledge.cpp +++ b/Userland/Utilities/pledge.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -12,12 +13,12 @@ static ErrorOr is_dynamically_linked_executable(StringView filename) { - String exec_filename = filename; - if (!filename.contains('/')) { - exec_filename = TRY(Core::System::find_file_in_path(filename)); - } + auto maybe_executable = Core::File::resolve_executable_from_environment(filename); - auto file = TRY(Core::MappedFile::map(exec_filename)); + if (!maybe_executable.has_value()) + return ENOENT; + + auto file = TRY(Core::MappedFile::map(maybe_executable.release_value())); ELF::Image elf_image(file->bytes()); return elf_image.is_dynamic(); } diff --git a/Userland/Utilities/which.cpp b/Userland/Utilities/which.cpp index aace9ea20b..96cceb0958 100644 --- a/Userland/Utilities/which.cpp +++ b/Userland/Utilities/which.cpp @@ -5,7 +5,7 @@ */ #include -#include +#include #include #include #include @@ -20,12 +20,12 @@ ErrorOr serenity_main(Main::Arguments arguments) args_parser.add_positional_argument(filename, "Name of executable", "executable"); args_parser.parse(arguments); - auto fullpath = Core::find_executable_in_path(filename); - if (fullpath.is_empty()) { + auto fullpath = Core::File::resolve_executable_from_environment({ filename, strlen(filename) }); + if (!fullpath.has_value()) { warnln("no '{}' in path", filename); return 1; } - outln("{}", fullpath); + outln("{}", fullpath.release_value()); return 0; }