Skip to content

Commit

Permalink
Separate PosixError from SysError
Browse files Browse the repository at this point in the history
Most of this is a big `throw SysError` -> `throw PosixError` sed. This
is a rather pure-churn change I would like to get out of the way. **The
intersting part is `src/libutil/error.hh`.**

On Unix, we will only throw the `PosixError` concrete class, which has
the same constructors that `SysError` used to have.

On Windows, we will throw `WinError` *and* `PosixError`. `WinError`
(which will be created in a later PR), will use a `DWORD` instead of
`int` error value, and `GetLastError()`, which is the Windows equivalent
of the `errno` machinery. Windows will *also* use `PosixError` because
Window's "libc" (MSVCRT) implements the POSIX interface, and we use it
too.

As the docs describe, while we *throw* one of the 3 choices above (2
concrete classes or the alias), we should always *catch* `SysError`.
This ensures no matter how the implementation changes for Windows (e.g.
between `PosixError` and `WinError`) the catching logic stays the same
and stays correct.

Co-Authored-By volth <volth@volth.com>
  • Loading branch information
Ericson2314 authored and EButlerIV committed Jan 12, 2024
1 parent 8450267 commit 0da2e75
Show file tree
Hide file tree
Showing 49 changed files with 322 additions and 298 deletions.
8 changes: 4 additions & 4 deletions src/libcmd/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,19 +318,19 @@ bool NixRepl::getLine(std::string & input, const std::string & prompt)
sigfillset(&act.sa_mask);
act.sa_flags = 0;
if (sigaction(SIGINT, &act, &old))
throw SysError("installing handler for SIGINT");
throw PosixError("installing handler for SIGINT");

sigemptyset(&set);
sigaddset(&set, SIGINT);
if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask))
throw SysError("unblocking SIGINT");
throw PosixError("unblocking SIGINT");
};
auto restoreSignals = [&]() {
if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr))
throw SysError("restoring signals");
throw PosixError("restoring signals");

if (sigaction(SIGINT, &old, 0))
throw SysError("restoring handler for SIGINT");
throw PosixError("restoring handler for SIGINT");
};

setupSignals();
Expand Down
26 changes: 13 additions & 13 deletions src/libmain/shared.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,30 +130,30 @@ void initNix()

act.sa_handler = SIG_DFL;
if (sigaction(SIGCHLD, &act, 0))
throw SysError("resetting SIGCHLD");
throw PosixError("resetting SIGCHLD");

/* Install a dummy SIGUSR1 handler for use with pthread_kill(). */
act.sa_handler = sigHandler;
if (sigaction(SIGUSR1, &act, 0)) throw SysError("handling SIGUSR1");
if (sigaction(SIGUSR1, &act, 0)) throw PosixError("handling SIGUSR1");

#if __APPLE__
/* HACK: on darwin, we need can’t use sigprocmask with SIGWINCH.
* Instead, add a dummy sigaction handler, and signalHandlerThread
* can handle the rest. */
act.sa_handler = sigHandler;
if (sigaction(SIGWINCH, &act, 0)) throw SysError("handling SIGWINCH");
if (sigaction(SIGWINCH, &act, 0)) throw PosixError("handling SIGWINCH");

/* Disable SA_RESTART for interrupts, so that system calls on this thread
* error with EINTR like they do on Linux.
* Most signals on BSD systems default to SA_RESTART on, but Nix
* expects EINTR from syscalls to properly exit. */
act.sa_handler = SIG_DFL;
if (sigaction(SIGINT, &act, 0)) throw SysError("handling SIGINT");
if (sigaction(SIGTERM, &act, 0)) throw SysError("handling SIGTERM");
if (sigaction(SIGHUP, &act, 0)) throw SysError("handling SIGHUP");
if (sigaction(SIGPIPE, &act, 0)) throw SysError("handling SIGPIPE");
if (sigaction(SIGQUIT, &act, 0)) throw SysError("handling SIGQUIT");
if (sigaction(SIGTRAP, &act, 0)) throw SysError("handling SIGTRAP");
if (sigaction(SIGINT, &act, 0)) throw PosixError("handling SIGINT");
if (sigaction(SIGTERM, &act, 0)) throw PosixError("handling SIGTERM");
if (sigaction(SIGHUP, &act, 0)) throw PosixError("handling SIGHUP");
if (sigaction(SIGPIPE, &act, 0)) throw PosixError("handling SIGPIPE");
if (sigaction(SIGQUIT, &act, 0)) throw PosixError("handling SIGQUIT");
if (sigaction(SIGTRAP, &act, 0)) throw PosixError("handling SIGTRAP");
#endif

/* Register a SIGSEGV handler to detect stack overflows.
Expand Down Expand Up @@ -310,7 +310,7 @@ void showManPage(const std::string & name)
restoreProcessContext();
setenv("MANPATH", settings.nixManDir.c_str(), 1);
execlp("man", "man", name.c_str(), nullptr);
throw SysError("command 'man %1%' failed", name.c_str());
throw PosixError("command 'man %1%' failed", name.c_str());
}


Expand Down Expand Up @@ -367,7 +367,7 @@ RunPager::RunPager()

pid = startProcess([&]() {
if (dup2(toPager.readSide.get(), STDIN_FILENO) == -1)
throw SysError("dupping stdin");
throw PosixError("dupping stdin");
if (!getenv("LESS"))
setenv("LESS", "FRSXMK", 1);
restoreProcessContext();
Expand All @@ -376,13 +376,13 @@ RunPager::RunPager()
execlp("pager", "pager", nullptr);
execlp("less", "less", nullptr);
execlp("more", "more", nullptr);
throw SysError("executing '%1%'", pager);
throw PosixError("executing '%1%'", pager);
});

pid.setKillSignal(SIGINT);
std_out = fcntl(STDOUT_FILENO, F_DUPFD_CLOEXEC, 0);
if (dup2(toPager.writeSide.get(), STDOUT_FILENO) == -1)
throw SysError("dupping standard output");
throw PosixError("dupping standard output");
}


Expand Down
4 changes: 2 additions & 2 deletions src/libmain/stack.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ void detectStackOverflow()
stack.ss_sp = stackBuf->data();
if (!stack.ss_sp) throw Error("cannot allocate alternative stack");
stack.ss_flags = 0;
if (sigaltstack(&stack, 0) == -1) throw SysError("cannot set alternative stack");
if (sigaltstack(&stack, 0) == -1) throw PosixError("cannot set alternative stack");

struct sigaction act;
sigfillset(&act.sa_mask);
act.sa_sigaction = sigsegvHandler;
act.sa_flags = SA_SIGINFO | SA_ONSTACK;
if (sigaction(SIGSEGV, &act, 0))
throw SysError("resetting SIGSEGV");
throw PosixError("resetting SIGSEGV");
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion src/libstore/binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ AutoCloseFD openFile(const Path & path)
{
auto fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
if (!fd)
throw SysError("opening file '%1%'", path);
throw PosixError("opening file '%1%'", path);
return fd;
}

Expand Down
8 changes: 4 additions & 4 deletions src/libstore/build/child.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ void commonChildInit()
that e.g. ssh cannot open /dev/tty) and it doesn't receive
terminal signals. */
if (setsid() == -1)
throw SysError("creating a new session");
throw PosixError("creating a new session");

/* Dup stderr to stdout. */
if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1)
throw SysError("cannot dup stderr into stdout");
throw PosixError("cannot dup stderr into stdout");

/* Reroute stdin to /dev/null. */
int fdDevNull = open(pathNullDevice.c_str(), O_RDWR);
if (fdDevNull == -1)
throw SysError("cannot open '%1%'", pathNullDevice);
throw PosixError("cannot open '%1%'", pathNullDevice);
if (dup2(fdDevNull, STDIN_FILENO) == -1)
throw SysError("cannot dup null device into stdin");
throw PosixError("cannot dup null device into stdin");
close(fdDevNull);
}

Expand Down
6 changes: 3 additions & 3 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ void DerivationGoal::tryLocalBuild() {
static void chmod_(const Path & path, mode_t mode)
{
if (chmod(path.c_str(), mode) == -1)
throw SysError("setting permissions on '%s'", path);
throw PosixError("setting permissions on '%s'", path);
}


Expand Down Expand Up @@ -1188,7 +1188,7 @@ HookReply DerivationGoal::tryBuildHook()
else if (reply != "accept")
throw Error("bad hook reply '%s'", reply);

} catch (SysError & e) {
} catch (PosixError & e) {
if (e.errNo == EPIPE) {
printError(
"build hook died unexpectedly: %s",
Expand Down Expand Up @@ -1274,7 +1274,7 @@ Path DerivationGoal::openLogFile()
settings.compressLog ? ".bz2" : "");

fdLogFile = open(logFileName.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0666);
if (!fdLogFile) throw SysError("creating log file '%1%'", logFileName);
if (!fdLogFile) throw PosixError("creating log file '%1%'", logFileName);

logFileSink = std::make_shared<FdSink>(fdLogFile.get());

Expand Down
12 changes: 6 additions & 6 deletions src/libstore/build/hook-instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,28 @@ HookInstance::HookInstance()
pid = startProcess([&]() {

if (dup2(fromHook.writeSide.get(), STDERR_FILENO) == -1)
throw SysError("cannot pipe standard error into log file");
throw PosixError("cannot pipe standard error into log file");

commonChildInit();

if (chdir("/") == -1) throw SysError("changing into /");
if (chdir("/") == -1) throw PosixError("changing into /");

/* Dup the communication pipes. */
if (dup2(toHook.readSide.get(), STDIN_FILENO) == -1)
throw SysError("dupping to-hook read side");
throw PosixError("dupping to-hook read side");

/* Use fd 4 for the builder's stdout/stderr. */
if (dup2(builderOut.writeSide.get(), 4) == -1)
throw SysError("dupping builder's stdout/stderr");
throw PosixError("dupping builder's stdout/stderr");

/* Hack: pass the read side of that fd to allow build-remote
to read SSH error messages. */
if (dup2(builderOut.readSide.get(), 5) == -1)
throw SysError("dupping builder's stdout/stderr");
throw PosixError("dupping builder's stdout/stderr");

execv(buildHook.c_str(), stringsToCharPtrs(args).data());

throw SysError("executing '%s'", buildHook);
throw PosixError("executing '%s'", buildHook);
});

pid.setSeparatePG(true);
Expand Down
Loading

0 comments on commit 0da2e75

Please sign in to comment.