From ce6cb14995e869cfea395570ccb300b0369c72dc Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 5 Apr 2024 21:15:22 +0200 Subject: [PATCH] libutil: return Pid from startProcess, not pid_t Change-Id: Icc8a15090c77f54ea7d9220aadedcd4a19922814 --- src/libmain/shared.cc | 4 ++-- src/libstore/build/hook-instance.cc | 4 ++-- src/libstore/build/local-derivation-goal.cc | 14 +++++++------- src/libstore/ssh.cc | 8 ++++---- src/libutil/namespaces.cc | 6 +++--- src/libutil/processes.cc | 4 ++-- src/libutil/processes.hh | 3 ++- src/nix/daemon.cc | 2 +- .../repl_characterization/test-session.cc | 4 ++-- .../repl_characterization/test-session.hh | 3 ++- 10 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 5dabd0c8e..29538a9ca 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -354,7 +354,7 @@ RunPager::RunPager() Pipe toPager; toPager.create(); - pid = Pid{startProcess([&]() { + pid = startProcess([&]() { if (dup2(toPager.readSide.get(), STDIN_FILENO) == -1) throw SysError("dupping stdin"); if (!getenv("LESS")) @@ -366,7 +366,7 @@ RunPager::RunPager() execlp("less", "less", nullptr); execlp("more", "more", nullptr); throw SysError("executing '%1%'", pager); - })}; + }); pid.setKillSignal(SIGINT); std_out = fcntl(STDOUT_FILENO, F_DUPFD_CLOEXEC, 0); diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index b8d8fe7a4..9a93646f4 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -35,7 +35,7 @@ HookInstance::HookInstance() builderOut.create(); /* Fork the hook. */ - pid = Pid{startProcess([&]() { + pid = startProcess([&]() { if (dup2(fromHook.writeSide.get(), STDERR_FILENO) == -1) throw SysError("cannot pipe standard error into log file"); @@ -60,7 +60,7 @@ HookInstance::HookInstance() execv(buildHook.c_str(), stringsToCharPtrs(args).data()); throw SysError("executing '%s'", buildHook); - })}; + }); pid.setSeparatePG(true); fromHook.writeSide.reset(); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index a2ff495bf..efba648a4 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -906,7 +906,7 @@ void LocalDerivationGoal::startBuilder() Pipe sendPid; sendPid.create(); - Pid helper{startProcess([&]() { + Pid helper = startProcess([&]() { sendPid.readSide.close(); /* We need to open the slave early, before @@ -930,11 +930,11 @@ void LocalDerivationGoal::startBuilder() if (usingUserNamespace) options.cloneFlags |= CLONE_NEWUSER; - pid_t child = startProcess([&]() { runChild(); }, options); + pid_t child = startProcess([&]() { runChild(); }, options).release(); writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); _exit(0); - })}; + }); sendPid.writeSide.close(); @@ -1010,10 +1010,10 @@ void LocalDerivationGoal::startBuilder() } else #endif { - pid = Pid{startProcess([&]() { + pid = startProcess([&]() { openSlave(); runChild(); - })}; + }); } /* parent */ @@ -1570,7 +1570,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path) entering its mount namespace, which is not possible in multithreaded programs. So we do this in a child process.*/ - Pid child(startProcess([&]() { + Pid child = startProcess([&]() { if (usingUserNamespace && (setns(sandboxUserNamespace.get(), 0) == -1)) throw SysError("entering sandbox user namespace"); @@ -1581,7 +1581,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path) doBind(source, target); _exit(0); - })); + }); int status = child.wait(); if (status != 0) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index c40eba894..0d7bfa01d 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -70,7 +70,7 @@ std::unique_ptr SSHMaster::startCommand(const std::string } Finally cleanup = [&]() { logger->resume(); }; - conn->sshPid = Pid{startProcess([&]() { + conn->sshPid = startProcess([&]() { restoreProcessContext(); close(in.writeSide.get()); @@ -99,7 +99,7 @@ std::unique_ptr SSHMaster::startCommand(const std::string // could not exec ssh/bash throw SysError("unable to execute '%s'", args.front()); - }, options)}; + }, options); in.readSide.reset(); @@ -147,7 +147,7 @@ Path SSHMaster::startMaster() if (isMasterRunning()) return state->socketPath; - state->sshMaster = Pid{startProcess([&]() { + state->sshMaster = startProcess([&]() { restoreProcessContext(); close(out.readSide.get()); @@ -160,7 +160,7 @@ Path SSHMaster::startMaster() execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); throw SysError("unable to execute '%s'", args.front()); - }, options)}; + }, options); out.writeSide.reset(); diff --git a/src/libutil/namespaces.cc b/src/libutil/namespaces.cc index 4b7a1b577..247fba2c4 100644 --- a/src/libutil/namespaces.cc +++ b/src/libutil/namespaces.cc @@ -94,7 +94,7 @@ bool userNamespacesSupported() static auto res = [&]() -> bool { try { - Pid pid{startProcess([&]() { _exit(0); }, {.cloneFlags = CLONE_NEWUSER})}; + Pid pid = startProcess([&]() { _exit(0); }, {.cloneFlags = CLONE_NEWUSER}); auto r = pid.wait(); assert(!r); @@ -115,7 +115,7 @@ bool mountAndPidNamespacesSupported() { try { - Pid pid{startProcess([&]() { + Pid pid = startProcess([&]() { /* Make sure we don't remount the parent's /proc. */ if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1) _exit(1); @@ -130,7 +130,7 @@ bool mountAndPidNamespacesSupported() _exit(0); }, { .cloneFlags = CLONE_NEWNS | CLONE_NEWPID | (userNamespacesSupported() ? CLONE_NEWUSER : 0) - })}; + }); if (pid.wait()) { debug("PID namespaces do not work on this system: cannot remount /proc"); diff --git a/src/libutil/processes.cc b/src/libutil/processes.cc index 9c99f047c..e8af12fbd 100644 --- a/src/libutil/processes.cc +++ b/src/libutil/processes.cc @@ -183,7 +183,7 @@ static int childEntry(void * arg) #endif -pid_t startProcess(std::function fun, const ProcessOptions & options) +Pid startProcess(std::function fun, const ProcessOptions & options) { std::function wrapper = [&]() { logger = makeSimpleLogger(); @@ -227,7 +227,7 @@ pid_t startProcess(std::function fun, const ProcessOptions & options) if (pid == -1) throw SysError("unable to fork"); - return pid; + return Pid{pid}; } std::string runProgram(Path program, bool searchPath, const Strings & args, diff --git a/src/libutil/processes.hh b/src/libutil/processes.hh index 8fc241ab1..001e1fa12 100644 --- a/src/libutil/processes.hh +++ b/src/libutil/processes.hh @@ -62,7 +62,8 @@ struct ProcessOptions int cloneFlags = 0; }; -pid_t startProcess(std::function fun, const ProcessOptions & options = ProcessOptions()); +[[nodiscard]] +Pid startProcess(std::function fun, const ProcessOptions & options = ProcessOptions()); /** diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index f1cc1ee9c..a9211d64a 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -369,7 +369,7 @@ static void daemonLoop(std::optional forceTrustClientOpt) processConnection(openUncachedStore(), from, to, trusted, NotRecursive); exit(0); - }, options); + }, options).release(); } catch (Interrupted & e) { return; diff --git a/tests/functional/repl_characterization/test-session.cc b/tests/functional/repl_characterization/test-session.cc index e59064fc5..96f101cdd 100644 --- a/tests/functional/repl_characterization/test-session.cc +++ b/tests/functional/repl_characterization/test-session.cc @@ -22,7 +22,7 @@ RunningProcess RunningProcess::start(std::string executable, Strings args) procStdout.create(); // This is separate from runProgram2 because we have different IO requirements - pid_t pid = startProcess([&]() { + auto pid = startProcess([&]() { if (dup2(procStdout.writeSide.get(), STDOUT_FILENO) == -1) { throw SysError("dupping stdout"); } @@ -42,7 +42,7 @@ RunningProcess RunningProcess::start(std::string executable, Strings args) procStdin.readSide.close(); return RunningProcess{ - .pid = pid, + .pid = std::move(pid), .procStdin = std::move(procStdin), .procStdout = std::move(procStdout), }; diff --git a/tests/functional/repl_characterization/test-session.hh b/tests/functional/repl_characterization/test-session.hh index c77cce6d5..a9ba9cf0c 100644 --- a/tests/functional/repl_characterization/test-session.hh +++ b/tests/functional/repl_characterization/test-session.hh @@ -7,13 +7,14 @@ #include #include "file-descriptor.hh" +#include "processes.hh" #include "tests/terminal-code-eater.hh" namespace nix { struct RunningProcess { - pid_t pid; + Pid pid; Pipe procStdin; Pipe procStdout;