From: Joel Rosdahl Date: Sat, 11 Jan 2025 14:40:17 +0000 (+0100) Subject: refactor: Simplify code for executing Windows processes X-Git-Tag: v4.11~20 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ba120a63882f1b7a946ce4af317c2811a67cde7c;p=thirdparty%2Fccache.git refactor: Simplify code for executing Windows processes --- diff --git a/src/ccache/execute.cpp b/src/ccache/execute.cpp index d9d222e4..80e28a72 100644 --- a/src/ccache/execute.cpp +++ b/src/ccache/execute.cpp @@ -56,8 +56,7 @@ namespace fs = util::filesystem; #ifdef _WIN32 -static int win32execute(const char* path, - const char* const* argv, +static int win32execute(const char* const* argv, int doreturn, int fd_stdout, int fd_stderr, @@ -71,8 +70,7 @@ execute(Context& ctx, { LOG("Executing {}", util::format_argv_for_logging(argv)); - return win32execute(argv[0], - argv, + return win32execute(argv, 1, fd_out.release(), fd_err.release(), @@ -82,7 +80,7 @@ execute(Context& ctx, void execute_noreturn(const char* const* argv, const fs::path& temp_dir) { - win32execute(argv[0], argv, 0, -1, -1, util::pstr(temp_dir).c_str()); + win32execute(argv, 0, -1, -1, util::pstr(temp_dir).c_str()); } std::string @@ -114,8 +112,7 @@ win32getshell(const std::string& path) } int -win32execute(const char* path, - const char* const* argv, +win32execute(const char* const* argv, int doreturn, int fd_stdout, int fd_stderr, @@ -198,11 +195,6 @@ win32execute(const char* path, STARTUPINFO si; memset(&si, 0x00, sizeof(si)); - std::string sh = win32getshell(path); - if (!sh.empty()) { - path = sh.c_str(); - } - si.cb = sizeof(STARTUPINFO); if (fd_stdout != -1) { si.hStdOutput = (HANDLE)_get_osfhandle(fd_stdout); @@ -225,10 +217,9 @@ win32execute(const char* path, } } - std::string args = util::format_argv_as_win32_command_string(argv, sh); - std::string full_path = util::add_exe_suffix(path); - fs::path tmp_file_path; + std::string args = util::format_argv_as_win32_command_string(argv); + fs::path tmp_file_path; DEFER([&] { if (!tmp_file_path.empty()) { util::remove(tmp_file_path); @@ -238,12 +229,17 @@ win32execute(const char* path, if (args.length() > 8192) { auto tmp_file = util::value_or_throw( util::TemporaryFile::create(FMT("{}/cmd_args", temp_dir))); - args = util::format_argv_as_win32_command_string(argv + 1, sh, true); + args = util::format_argv_as_win32_command_string(argv + 1, true); util::write_fd(*tmp_file.fd, args.data(), args.length()); - args = FMT(R"("{}" "@{}")", full_path, tmp_file.path); + args = FMT(R"("{}" "@{}")", argv[0], tmp_file.path); tmp_file_path = tmp_file.path; LOG("Arguments from {}", tmp_file.path); } + + std::string sh = win32getshell(argv[0]); + if (!sh.empty()) { + args = FMT(R"("{}" {})", sh, args); + } BOOL ret = CreateProcess(nullptr, const_cast(args.c_str()), nullptr, @@ -261,7 +257,7 @@ win32execute(const char* path, if (ret == 0) { DWORD error = GetLastError(); LOG("failed to execute {}: {} ({})", - full_path, + argv[0], util::win32_error_message(error), error); return -1; @@ -272,8 +268,8 @@ win32execute(const char* path, TerminateProcess(pi.hProcess, 1); DWORD error = GetLastError(); - LOG("failed to assign process to job object {}: {} ({})", - full_path, + LOG("failed to assign process to job object for {}: {} ({})", + argv[0], util::win32_error_message(error), error); return -1; diff --git a/src/ccache/hashutil.cpp b/src/ccache/hashutil.cpp index 880bedee..c8bed7e5 100644 --- a/src/ccache/hashutil.cpp +++ b/src/ccache/hashutil.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2009-2024 Joel Rosdahl and other contributors +// Copyright (C) 2009-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -367,6 +367,13 @@ hash_command_output(Hash& hash, using_cmd_exe = false; } Args args = Args::from_string(adjusted_command); + { + auto full_path = + find_executable_in_path(args[0], util::getenv_path_list("PATH")).string(); + if (!full_path.empty()) { + args[0] = full_path; + } + } #else Args args = Args::from_string(command); #endif @@ -388,16 +395,6 @@ hash_command_output(Hash& hash, STARTUPINFO si; memset(&si, 0x00, sizeof(si)); - auto path = - find_executable_in_path(args[0], util::getenv_path_list("PATH")).string(); - if (path.empty()) { - path = args[0]; - } - std::string sh = win32getshell(path); - if (!sh.empty()) { - path = sh; - } - si.cb = sizeof(STARTUPINFO); HANDLE pipe_out[2]; @@ -409,14 +406,18 @@ hash_command_output(Hash& hash, si.hStdInput = GetStdHandle(STD_INPUT_HANDLE); si.dwFlags = STARTF_USESTDHANDLES; - std::string win32args; + std::string commandline; if (using_cmd_exe) { - win32args = adjusted_command; // quoted + commandline = adjusted_command; // quoted } else { - win32args = util::format_argv_as_win32_command_string(argv, sh); + commandline = util::format_argv_as_win32_command_string(argv); + std::string sh = win32getshell(args[0]); + if (!sh.empty()) { + commandline = FMT(R"("{}" {})", sh, commandline); + } } - BOOL ret = CreateProcess(path.c_str(), - const_cast(win32args.c_str()), + BOOL ret = CreateProcess(nullptr, + const_cast(commandline.c_str()), nullptr, nullptr, 1, diff --git a/src/ccache/util/string.cpp b/src/ccache/util/string.cpp index 1597647e..54874ffb 100644 --- a/src/ccache/util/string.cpp +++ b/src/ccache/util/string.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Joel Rosdahl and other contributors +// Copyright (C) 2021-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -51,14 +51,12 @@ namespace util { std::string format_argv_as_win32_command_string(const char* const* argv, - const std::string& prefix, bool escape_backslashes) { std::string result; - size_t i = 0; - const char* arg = prefix.empty() ? argv[i++] : prefix.c_str(); - do { + for (size_t i = 0; argv[i]; ++i) { + const char* arg = argv[i]; int bs = 0; result += '"'; for (size_t j = 0; arg[j]; ++j) { @@ -88,7 +86,7 @@ format_argv_as_win32_command_string(const char* const* argv, --bs; } result += "\" "; - } while ((arg = argv[i++])); + } result.resize(result.length() - 1); return result; diff --git a/src/ccache/util/string.hpp b/src/ccache/util/string.hpp index b1c3ae26..f78ee085 100644 --- a/src/ccache/util/string.hpp +++ b/src/ccache/util/string.hpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Joel Rosdahl and other contributors +// Copyright (C) 2021-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -50,7 +50,6 @@ bool ends_with(std::string_view string, std::string_view suffix); // is not at the end of `argv[i]` either. std::string format_argv_as_win32_command_string(const char* const* argv, - const std::string& prefix, bool escape_backslashes = false); // Format `argv` as a simple string for logging purposes. That is, the result is diff --git a/unittest/test_util_string.cpp b/unittest/test_util_string.cpp index 6714d93e..25fbb8b8 100644 --- a/unittest/test_util_string.cpp +++ b/unittest/test_util_string.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Joel Rosdahl and other contributors +// Copyright (C) 2021-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -43,34 +43,30 @@ TEST_CASE("util::format_argv_as_win32_command_string") { { const char* const argv[] = {"a", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "") == R"("a")"); - } - { - const char* const argv[] = {"a", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "p") == R"("p" "a")"); + CHECK(util::format_argv_as_win32_command_string(argv) == R"("a")"); } { const char* const argv[] = {"a", "b c", "\"d\"", "'e'", "\\\"h", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "") + CHECK(util::format_argv_as_win32_command_string(argv) == R"("a" "b c" "\"d\"" "'e'" "\\\"h")"); } { const char* const argv[] = {"a\\b\\c", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "") == R"("a\b\c")"); + CHECK(util::format_argv_as_win32_command_string(argv) == R"("a\b\c")"); } { const char* const argv[] = {"a\\b\\c", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "", true) + CHECK(util::format_argv_as_win32_command_string(argv, true) == R"("a\\b\\c")"); } { const char* const argv[] = {R"(a\b \"c\" \)", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "") + CHECK(util::format_argv_as_win32_command_string(argv) == R"("a\b \\\"c\\\" \\")"); } { const char* const argv[] = {R"(a\b \"c\" \)", nullptr}; - CHECK(util::format_argv_as_win32_command_string(argv, "", true) + CHECK(util::format_argv_as_win32_command_string(argv, true) == R"("a\\b \\\"c\\\" \\")"); } }