Separate SystemError from SysError

Most of this is a `catch SysError` -> `catch SystemError` 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 `SysError` concrete class, which has
the same constructors that `SystemError` used to have.

On Windows, we will throw `WinError` *and* `SysError`. `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 `SysError` 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* `SystemError`.
This ensures no matter how the implementation changes for Windows (e.g.
between `SysError` and `WinError`) the catching logic stays the same
and stays correct.

Co-Authored-By volth <volth@volth.com>
Co-Authored-By Eugene Butler <eugene@eugene4.com>
This commit is contained in:
John Ericson 2023-12-01 17:03:28 -05:00
parent 0d55d660d5
commit 6208ca7209
19 changed files with 59 additions and 27 deletions

View file

@ -254,7 +254,7 @@ void NixRepl::mainLoop()
rl_readline_name = "nix-repl";
try {
createDirs(dirOf(historyFile));
} catch (SysError & e) {
} catch (SystemError & e) {
logWarning(e.info());
}
#ifndef USE_READLINE

View file

@ -1495,7 +1495,7 @@ void LocalDerivationGoal::startDaemon()
daemon::processConnection(store, from, to,
NotTrusted, daemon::Recursive);
debug("terminated daemon connection");
} catch (SysError &) {
} catch (SystemError &) {
ignoreException();
}
});
@ -1707,7 +1707,7 @@ void LocalDerivationGoal::runChild()
try {
if (drv->isBuiltin() && drv->builder == "builtin:fetchurl")
netrcData = readFile(settings.netrcFile);
} catch (SysError &) { }
} catch (SystemError &) { }
#if __linux__
if (useChroot) {

View file

@ -413,7 +413,7 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
auto env_end = std::sregex_iterator{};
for (auto i = std::sregex_iterator{envString.begin(), envString.end(), storePathRegex}; i != env_end; ++i)
unchecked[i->str()].emplace(envFile);
} catch (SysError & e) {
} catch (SystemError & e) {
if (errno == ENOENT || errno == EACCES || errno == ESRCH)
continue;
throw;

View file

@ -118,7 +118,7 @@ void loadConfFile()
try {
std::string contents = readFile(path);
globalConfig.applyConfig(contents, path);
} catch (SysError &) { }
} catch (SystemError &) { }
};
applyConfigFile(settings.nixConfDir + "/nix.conf");

View file

@ -19,7 +19,7 @@ PublicKeys getDefaultPublicKeys()
try {
SecretKey secretKey(readFile(secretKeyFile));
publicKeys.emplace(secretKey.name, secretKey.toPublicKey());
} catch (SysError & e) {
} catch (SystemError & e) {
/* Ignore unreadable key files. That's normal in a
multi-user installation. */
}

View file

@ -276,7 +276,7 @@ LocalStore::LocalStore(const Params & params)
[[gnu::unused]] auto res2 = ftruncate(fd.get(), settings.reservedSize);
}
}
} catch (SysError & e) { /* don't care about errors */
} catch (SystemError & e) { /* don't care about errors */
}
/* Acquire the big fat lock in shared mode to make sure that no

View file

@ -242,7 +242,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
/* Atomically replace the old file with the new hard link. */
try {
renameFile(tempLink, path);
} catch (SysError & e) {
} catch (SystemError & e) {
if (unlink(tempLink.c_str()) == -1)
printError("unable to unlink '%1%'", tempLink);
if (errno == EMLINK) {

View file

@ -87,13 +87,13 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
nars.emplace(storePath.hashPart(), narAccessor);
return {narAccessor, restPath};
} catch (SysError &) { }
} catch (SystemError &) { }
try {
auto narAccessor = makeNarAccessor(nix::readFile(cacheFile));
nars.emplace(storePath.hashPart(), narAccessor);
return {narAccessor, restPath};
} catch (SysError &) { }
} catch (SystemError &) { }
}
StringSink sink;

View file

@ -304,7 +304,7 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang)
for (auto pos = savedArgs.begin(); pos != savedArgs.end();pos++)
cmdline.push_back(*pos);
}
} catch (SysError &) { }
} catch (SystemError &) { }
}
for (auto pos = cmdline.begin(); pos != cmdline.end(); ) {

View file

@ -95,7 +95,7 @@ static CgroupStats destroyCgroup(const Path & cgroup, bool returnStats)
using namespace std::string_literals;
warn("killing stray builder process %d (%s)...",
pid, trim(replaceStrings(cmdline, "\0"s, " ")));
} catch (SysError &) {
} catch (SystemError &) {
}
}
// FIXME: pid wraparound

View file

@ -124,7 +124,7 @@ static void applyConfigInner(const std::string & contents, const std::string & p
try {
std::string includedContents = readFile(path);
applyConfigInner(includedContents, p, parsedContents);
} catch (SysError &) {
} catch (SystemError &) {
// TODO: Do we actually want to ignore this? Or is it better to fail?
}
} else if (!ignoreMissing) {

View file

@ -178,20 +178,50 @@ MakeError(Error, BaseError);
MakeError(UsageError, Error);
MakeError(UnimplementedError, Error);
class SysError : public Error
/**
* To use in catch-blocks.
*/
MakeError(SystemError, Error);
/**
* POSIX system error, created using `errno`, `strerror` friends.
*
* Throw this, but prefer not to catch this, and catch `SystemError`
* instead. This allows implementations to freely switch between this
* and `WinError` without breaking catch blocks.
*
* However, it is permissible to catch this and rethrow so long as
* certain conditions are not met (e.g. to catch only if `errNo =
* EFooBar`). In that case, try to also catch the equivalent `WinError`
* code.
*
* @todo Rename this to `PosixError` or similar. At this point Windows
* support is too WIP to justify the code churn, but if it is finished
* then a better identifier becomes moe worth it.
*/
class SysError : public SystemError
{
public:
int errNo;
/**
* Construct using the explicitly-provided error number. `strerror`
* will be used to try to add additional information to the message.
*/
template<typename... Args>
SysError(int errNo_, const Args & ... args)
: Error("")
SysError(int errNo, const Args & ... args)
: SystemError(""), errNo(errNo)
{
errNo = errNo_;
auto hf = hintfmt(args...);
err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo));
}
/**
* Construct using the ambient `errno`.
*
* Be sure to not perform another `errno`-modifying operation before
* calling this constructor!
*/
template<typename... Args>
SysError(const Args & ... args)
: SysError(errno, args ...)
@ -199,7 +229,9 @@ public:
}
};
/** Throw an exception for the purpose of checking that exception handling works; see 'initLibUtil()'.
/**
* Throw an exception for the purpose of checking that exception
* handling works; see 'initLibUtil()'.
*/
void throwExceptionSelfCheck();

View file

@ -231,7 +231,7 @@ void closeMostFDs(const std::set<int> & exceptions)
}
}
return;
} catch (SysError &) {
} catch (SystemError &) {
}
#endif

View file

@ -116,7 +116,7 @@ void writeToStderr(std::string_view s)
{
try {
writeFull(STDERR_FILENO, s, false);
} catch (SysError & e) {
} catch (SystemError & e) {
/* Ignore failing writes to stderr. We need to ignore write
errors to ensure that cleanup code that logs to stderr runs
to completion if the other side of stderr has been closed

View file

@ -53,7 +53,7 @@ void FdSink::writeUnbuffered(std::string_view data)
written += data.size();
try {
writeFull(fd, data);
} catch (SysError & e) {
} catch (SystemError & e) {
_good = false;
throw;
}

View file

@ -20,7 +20,7 @@ void initLibUtil() {
// When exception handling fails, the message tends to be printed by the
// C++ runtime, followed by an abort.
// For example on macOS we might see an error such as
// libc++abi: terminating with uncaught exception of type nix::SysError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded.
// libc++abi: terminating with uncaught exception of type nix::SystemError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded.
bool caught = false;
try {
throwExceptionSelfCheck();

View file

@ -148,7 +148,7 @@ static void main_nix_build(int argc, char * * argv)
args.push_back(word);
}
}
} catch (SysError &) { }
} catch (SystemError &) { }
}
struct MyArgs : LegacyArgs, MixEvalArgs

View file

@ -107,7 +107,7 @@ struct CmdConfigCheck : StoreCommand
if (profileDir.find("/profiles/") == std::string::npos)
dirs.insert(dir);
}
} catch (SysError &) {}
} catch (SystemError &) {}
}
if (!dirs.empty()) {

View file

@ -73,7 +73,7 @@ namespace nix {
}
TEST(logEI, picksUpSysErrorExitCode) {
TEST(logEI, picksUpSystemErrorExitCode) {
MakeError(TestError, Error);
ErrorInfo::programName = std::optional("error-unit-test");
@ -81,12 +81,12 @@ namespace nix {
try {
auto x = readFile(-1);
}
catch (SysError &e) {
catch (SystemError &e) {
testing::internal::CaptureStderr();
logError(e.info());
auto str = testing::internal::GetCapturedStderr();
ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n");
ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SystemError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n");
}
}