Repository: bitcoin/bitcoin
File: src/bitcoin.cpp (C++)

Code before:
//!
//!     strace -e trace=execve -s 10000 build/bin/bitcoin ...
//!     dtrace -n 'proc:::exec-success  /pid == $target/ { trace(curpsinfo->pr_psargs); }' -c ...
static void ExecCommand(const std::vector<const char*>& args, std::string_view wrapper_argv0)
{
    // Construct argument string for execvp
    std::vector<const char*> exec_args{args};
    exec_args.emplace_back(nullptr);

    // Try to call ExecVp with given exe path.
    auto try_exec = [&](fs::path exe_path, bool allow_notfound = true) {
        std::string exe_path_str{fs::PathToString(exe_path)};
        exec_args[0] = exe_path_str.c_str();
        if (util::ExecVp(exec_args[0], (char*const*)exec_args.data()) == -1) {
            if (allow_notfound && errno == ENOENT) return false;
            throw std::system_error(errno, std::system_category(), strprintf("execvp failed to execute '%s'", exec_args[0]));
        }
        throw std::runtime_error("execvp returned unexpectedly");
    };

    // Get the wrapper executable path.
    const fs::path wrapper_path{util::GetExePath(wrapper_argv0)};

    // Try to resolve any symlinks and figure out the directory containing the wrapper executable.
    std::error_code ec;
    fs::path wrapper_dir{fs::weakly_canonical(wrapper_path, ec)};
    if (wrapper_dir.empty()) wrapper_dir = wrapper_path; // Restore previous path if weakly_canonical failed.
    wrapper_dir = wrapper_dir.parent_path();

    // Get path of the executable to be invoked.
    const fs::path arg0{fs::PathFromString(args[0])};

    // Decide whether to fall back to the operating system to search for the
    // specified executable. Avoid doing this if it looks like the wrapper
    // executable was invoked by path, rather than by search, to avoid
    // unintentionally launching system executables in a local build.
    // (https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807)
    const bool fallback_os_search{!fs::PathFromString(std::string{wrapper_argv0}).has_parent_path()};

    // If wrapper is installed in a bin/ directory, look for target executable
    // in libexec/
    (wrapper_dir.filename() == "bin" && try_exec(fs::path{wrapper_dir.parent_path()} / "libexec" / arg0.filename())) ||
#ifdef WIN32
    // Otherwise check the "daemon" subdirectory in a windows install.
    (!wrapper_dir.empty() && try_exec(wrapper_dir / "daemon" / arg0.filename())) ||
#endif
    // Otherwise look for target executable next to current wrapper
    (!wrapper_dir.empty() && try_exec(wrapper_dir / arg0.filename(), fallback_os_search)) ||
    // Otherwise just look on the system path.
    (fallback_os_search && try_exec(arg0.filename(), false));
}

Review comment (bug):
nit in c864a4c1940d682f7eb6fdb3b91b18d638b59330: I understand that you probably used the std namespace here and then used `auto` to silence the linter? However, this seems to go in the wrong direction. If we think our `fs::` namespace should be preferred over the `std::` fs one, we should try to use it as much as possible and only call `std_path` when needed. I guess restoring `fs::path` here would require some overloads in the wrapper class, or restoring some `fs::path` constructors below. But this is just a nit or follow-up idea, and maybe this was intentional and I am missing something.

Code after:
//!
//!     strace -e trace=execve -s 10000 build/bin/bitcoin ...
//!     dtrace -n 'proc:::exec-success  /pid == $target/ { trace(curpsinfo->pr_psargs); }' -c ...
static void ExecCommand(const std::vector<const char*>& args, std::string_view wrapper_argv0)
{
    // Construct argument string for execvp
    std::vector<const char*> exec_args{args};
    exec_args.emplace_back(nullptr);

    // Try to call ExecVp with given exe path.
    auto try_exec = [&](fs::path exe_path, bool allow_notfound = true) {
        std::string exe_path_str{fs::PathToString(exe_path)};
        exec_args[0] = exe_path_str.c_str();
        if (util::ExecVp(exec_args[0], (char*const*)exec_args.data()) == -1) {
            if (allow_notfound && errno == ENOENT) return false;
            throw std::system_error(errno, std::system_category(), strprintf("execvp failed to execute '%s'", exec_args[0]));
        }
        throw std::runtime_error("execvp returned unexpectedly");
    };

    // Get the wrapper executable path.
    const fs::path wrapper_path{util::GetExePath(wrapper_argv0)};

    // Try to resolve any symlinks and figure out the directory containing the wrapper executable.
    std::error_code ec;
    auto wrapper_dir{fs::weakly_canonical(wrapper_path, ec)};
    if (wrapper_dir.empty()) wrapper_dir = wrapper_path; // Restore previous path if weakly_canonical failed.
    wrapper_dir = wrapper_dir.parent_path();

    // Get path of the executable to be invoked.
    const fs::path arg0{fs::PathFromString(args[0])};

    // Decide whether to fall back to the operating system to search for the
    // specified executable. Avoid doing this if it looks like the wrapper
    // executable was invoked by path, rather than by search, to avoid
    // unintentionally launching system executables in a local build.
    // (https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807)
    const bool fallback_os_search{!fs::PathFromString(std::string{wrapper_argv0}).has_parent_path()};

    // If wrapper is installed in a bin/ directory, look for target executable
    // in libexec/
    (wrapper_dir.filename() == "bin" && try_exec(wrapper_dir.parent_path() / "libexec" / arg0.filename())) ||
#ifdef WIN32
    // Otherwise check the "daemon" subdirectory in a windows install.
    (!wrapper_dir.empty() && try_exec(wrapper_dir / "daemon" / arg0.filename())) ||
#endif
    // Otherwise look for target executable next to current wrapper
    (!wrapper_dir.empty() && try_exec(wrapper_dir / arg0.filename(), fallback_os_search)) ||
    // Otherwise just look on the system path.
    (fallback_os_search && try_exec(arg0.filename(), false));
}