From b43a2e84c4b2fa7cb1167693652702e6dac95f53 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 5 Apr 2024 20:35:13 +0200 Subject: [PATCH] libutil: make Pid -> pid_t operations explicit Change-Id: I3137cc140590001fe7ba542844e735944a0a9255 --- src/libmain/shared.cc | 2 +- src/libstore/build/hook-instance.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 16 ++++++++-------- src/libstore/ssh.cc | 2 +- src/libutil/processes.cc | 6 ------ src/libutil/processes.hh | 3 ++- 6 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index f99777a20..29538a9ca 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -378,7 +378,7 @@ RunPager::RunPager() RunPager::~RunPager() { try { - if (pid != -1) { + if (pid) { std::cout.flush(); dup2(std_out, STDOUT_FILENO); pid.wait(); diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index 86f72486e..9a93646f4 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -79,7 +79,7 @@ HookInstance::~HookInstance() { try { toHook.writeSide.reset(); - if (pid != -1) pid.kill(); + if (pid) pid.kill(); } catch (...) { ignoreException(); } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4e616c838..eb6229c56 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -137,7 +137,7 @@ LocalStore & LocalDerivationGoal::getLocalStore() void LocalDerivationGoal::killChild() { - if (pid != -1) { + if (pid) { worker.childTerminated(this); /* If we're using a build user, then there is a tricky race @@ -145,7 +145,7 @@ void LocalDerivationGoal::killChild() done its setuid() to the build user uid, then it won't be killed, and we'll potentially lock up in pid.wait(). So also send a conventional kill to the child. */ - ::kill(-pid, SIGKILL); /* ignore the result */ + ::kill(-pid.get(), SIGKILL); /* ignore the result */ killSandbox(true); @@ -961,13 +961,13 @@ void LocalDerivationGoal::startBuilder() uid_t hostGid = buildUser ? buildUser->getGID() : getgid(); uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1; - writeFile("/proc/" + std::to_string(pid) + "/uid_map", + writeFile("/proc/" + std::to_string(pid.get()) + "/uid_map", fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); if (!buildUser || buildUser->getUIDCount() == 1) - writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny"); + writeFile("/proc/" + std::to_string(pid.get()) + "/setgroups", "deny"); - writeFile("/proc/" + std::to_string(pid) + "/gid_map", + writeFile("/proc/" + std::to_string(pid.get()) + "/gid_map", fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); } else { debug("note: not using a user namespace"); @@ -990,19 +990,19 @@ void LocalDerivationGoal::startBuilder() /* Save the mount- and user namespace of the child. We have to do this *before* the child does a chroot. */ - sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY)}; + sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", pid.get()).c_str(), O_RDONLY)}; if (sandboxMountNamespace.get() == -1) throw SysError("getting sandbox mount namespace"); if (usingUserNamespace) { - sandboxUserNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY)}; + sandboxUserNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/user", pid.get()).c_str(), O_RDONLY)}; if (sandboxUserNamespace.get() == -1) throw SysError("getting sandbox user namespace"); } /* Move the child into its own cgroup. */ if (cgroup) - writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid)); + writeFile(*cgroup + "/cgroup.procs", fmt("%d", pid.get())); /* Signal the builder that we've updated its user namespace. */ writeFull(userNamespaceSync.writeSide.get(), "1"); diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 932ebaa52..0d7bfa01d 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -131,7 +131,7 @@ Path SSHMaster::startMaster() auto state(state_.lock()); - if (state->sshMaster != -1) return state->socketPath; + if (state->sshMaster) return state->socketPath; state->socketPath = (Path) *state->tmpDir + "/ssh.sock"; diff --git a/src/libutil/processes.cc b/src/libutil/processes.cc index 8639a77f8..3bb419e45 100644 --- a/src/libutil/processes.cc +++ b/src/libutil/processes.cc @@ -55,12 +55,6 @@ void Pid::operator =(pid_t pid) } -Pid::operator pid_t() -{ - return pid; -} - - int Pid::kill() { assert(pid != -1); diff --git a/src/libutil/processes.hh b/src/libutil/processes.hh index b84fc7c4b..2079e1b36 100644 --- a/src/libutil/processes.hh +++ b/src/libutil/processes.hh @@ -29,13 +29,14 @@ public: Pid(pid_t pid); ~Pid() noexcept(false); void operator =(pid_t pid); - operator pid_t(); + explicit operator bool() const { return pid != -1; } int kill(); int wait(); void setSeparatePG(bool separatePG); void setKillSignal(int signal); pid_t release(); + pid_t get() const { return pid; } }; /**