diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 772a20e81a4..da042d7fb58 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -425,7 +425,7 @@ path_start /* Absolute paths are always interpreted relative to the root filesystem accessor, rather than the accessor of the current Nix expression. */ - Path path(canonPath(literal)); + auto path = canonPath(literal).string(); /* add back in the trailing '/' to the first segment */ if (literal.size() > 1 && literal.back() == '/') path += '/'; @@ -442,7 +442,7 @@ path_start }); auto basePath = std::filesystem::path(state->basePath.path.abs()); - Path path(absPath(literal, &basePath).string()); + auto path = absPath(literal, &basePath).string(); /* add back in the trailing '/' to the first segment */ if (literal.size() > 1 && literal.back() == '/') path += '/'; diff --git a/src/libutil/include/nix/util/error.hh b/src/libutil/include/nix/util/error.hh index 30af3270f7f..37c8417ba2d 100644 --- a/src/libutil/include/nix/util/error.hh +++ b/src/libutil/include/nix/util/error.hh @@ -19,6 +19,7 @@ #include "nix/util/fmt.hh" #include "nix/util/config.hh" +#include #include #include #include @@ -287,32 +288,60 @@ protected: /** * Just here to allow derived classes to use the right constructor * (the protected one). + * + * This one indicates the prebuilt `HintFmt` one with the explicit `errorDetails` */ - struct Disambig + struct DisambigHintFmt {}; + /** + * Just here to allow derived classes to use the right constructor + * (the protected one). + * + * This one indicates the varargs one to build the `HintFmt` with the explicit `errorDetails` + */ + struct DisambigVarArgs + {}; + + /** + * Protected constructor that takes a pre-built HintFmt. + * Use this when the error message needs to be constructed before + * capturing errno/GetLastError(). + */ + SystemError(DisambigHintFmt, std::error_code errorCode, std::string_view errorDetails, const HintFmt & hf) + : CloneableError(HintFmt{"%s: %s", Uncolored(hf.str()), errorDetails}) + , errorCode(errorCode) + , errorDetails(errorDetails) + { + } + /** * Protected constructor for subclasses that provide their own error message. * The error message is appended to the formatted hint. */ template - SystemError(Disambig, std::error_code errorCode, std::string_view errorDetails, Args &&... args) - : CloneableError("") - , errorCode(errorCode) - , errorDetails(errorDetails) + SystemError(DisambigVarArgs, std::error_code errorCode, std::string_view errorDetails, Args &&... args) + : SystemError(DisambigHintFmt{}, errorCode, errorDetails, HintFmt{std::forward(args)...}) { - auto hf = HintFmt(std::forward(args)...); - err.msg = HintFmt("%s: %s", Uncolored(hf.str()), errorDetails); } public: + /** + * Construct with an error code. The error code's message is automatically + * appended to the error message. + */ + SystemError(std::error_code errorCode, const HintFmt & hf) + : SystemError(DisambigHintFmt{}, errorCode, errorCode.message(), hf) + { + } + /** * Construct with an error code. The error code's message is automatically * appended to the error message. */ template SystemError(std::error_code errorCode, Args &&... args) - : SystemError(Disambig{}, errorCode, errorCode.message(), std::forward(args)...) + : SystemError(DisambigVarArgs{}, errorCode, errorCode.message(), std::forward(args)...) { } @@ -355,7 +384,7 @@ public: template SysError(int errNo, Args &&... args) : CloneableError( - Disambig{}, + DisambigVarArgs{}, std::make_error_code(static_cast(errNo)), strerror(errNo), std::forward(args)...) @@ -363,6 +392,19 @@ public: { } + /** + * Construct using the explicitly-provided error number. `strerror` + * will be used to try to add additional information to the message. + * + * Unlike above, the `HintFmt` already exists rather than being made on + * the spot. + */ + SysError(int errNo, const HintFmt & hf) + : CloneableError(DisambigHintFmt{}, std::make_error_code(static_cast(errNo)), strerror(errNo), hf) + , errNo(errNo) + { + } + /** * Construct using the ambient `errno`. * @@ -374,6 +416,34 @@ public: : SysError(errno, std::forward(args)...) { } + + /** + * Construct using the ambient `errno` and a function that produces + * a `HintFmt`. errno is read first, then the function is called, so + * the function is safe to modify `errno`. + */ + SysError(auto && mkHintFmt) + requires std::invocable && std::same_as, HintFmt> + : SysError(captureErrno(std::forward(mkHintFmt))) + { + } + +private: + /** + * Helper to ensure errno is captured before mkHintFmt is called. + * C++ argument evaluation order is unspecified, so we can't rely on + * `SysError(errno, mkHintFmt())` evaluating errno first. + */ + static std::pair captureErrno(auto && mkHintFmt) + { + int e = errno; + return {e, mkHintFmt()}; + } + + SysError(std::pair && p) + : SysError(p.first, std::move(p.second)) + { + } }; /** @@ -437,7 +507,7 @@ public: template WinError(DWORD lastError, Args &&... args) : CloneableError( - Disambig{}, + DisambigVarArgs{}, std::error_code(lastError, std::system_category()), renderError(lastError), std::forward(args)...) @@ -445,6 +515,21 @@ public: { } + /** + * Construct using the explicitly-provided error number. + * `FormatMessageA` will be used to try to add additional + * information to the message. + * + * Unlike above, the `HintFmt` already exists rather than being made on + * the spot. + */ + WinError(DWORD lastError, const HintFmt & hf) + : CloneableError( + DisambigHintFmt{}, std::error_code(lastError, std::system_category()), renderError(lastError), hf) + , lastError(lastError) + { + } + /** * Construct using `GetLastError()` and the ambient "last error". * @@ -457,7 +542,33 @@ public: { } + /** + * Construct using `GetLastError()` and a function that produces a + * `HintFmt`. `GetLastError()` is called first, then the function is + * called, so the function is safe to modify the last error. + */ + WinError(auto && mkHintFmt) + requires std::invocable && std::same_as, HintFmt> + : WinError(captureLastError(std::forward(mkHintFmt))) + { + } + private: + /** + * Helper to ensure GetLastError() is captured before mkHintFmt is called. + * C++ argument evaluation order is unspecified, so we can't rely on + * `WinError(GetLastError(), mkHintFmt())` evaluating GetLastError() first. + */ + static std::pair captureLastError(auto && mkHintFmt) + { + DWORD e = GetLastError(); + return {e, mkHintFmt()}; + } + + WinError(std::pair && p) + : WinError(p.first, std::move(p.second)) + { + } static std::string renderError(DWORD lastError); }; diff --git a/src/libutil/unix/file-system-at.cc b/src/libutil/unix/file-system-at.cc index e50596c4de5..739cac0177c 100644 --- a/src/libutil/unix/file-system-at.cc +++ b/src/libutil/unix/file-system-at.cc @@ -72,8 +72,7 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t if (errno == ENOSYS) fchmodat2Unsupported.test_and_set(); else { - auto savedErrno = errno; - throw SysError(savedErrno, "fchmodat2 %s", PathFmt(descriptorToPath(dirFd) / path.rel())); + throw SysError([&] { return HintFmt("fchmodat2 %s", PathFmt(descriptorToPath(dirFd) / path.rel())); }); } } else return; @@ -83,11 +82,11 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t #ifdef __linux__ AutoCloseFD pathFd = ::openat(dirFd, path.rel_c_str(), O_PATH | O_NOFOLLOW | O_CLOEXEC); if (!pathFd) { - auto savedErrno = errno; - throw SysError( - savedErrno, - "opening %s to get an O_PATH file descriptor (fchmodat2 is unsupported)", - PathFmt(descriptorToPath(dirFd) / path.rel())); + throw SysError([&] { + return HintFmt( + "opening %s to get an O_PATH file descriptor (fchmodat2 is unsupported)", + PathFmt(descriptorToPath(dirFd) / path.rel())); + }); } struct ::stat st; @@ -107,9 +106,9 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t if (errno == ENOENT) dontHaveProc.test_and_set(); else { - auto savedErrno = errno; - throw SysError( - savedErrno, "chmod %s (%s)", selfProcFdPath, PathFmt(descriptorToPath(dirFd) / path.rel())); + throw SysError([&] { + return HintFmt("chmod %s (%s)", selfProcFdPath, PathFmt(descriptorToPath(dirFd) / path.rel())); + }); } } else return; @@ -131,8 +130,7 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t ); if (res == -1) { - auto savedErrno = errno; - throw SysError(savedErrno, "fchmodat %s", PathFmt(descriptorToPath(dirFd) / path.rel())); + throw SysError([&] { return HintFmt("fchmodat %s", PathFmt(descriptorToPath(dirFd) / path.rel())); }); } } @@ -217,8 +215,8 @@ OsString readLinkAt(Descriptor dirFd, const CanonPath & path) buf.resize(bufSize); ssize_t rlSize = ::readlinkat(dirFd, path.rel_c_str(), buf.data(), bufSize); if (rlSize == -1) { - auto savedErrno = errno; - throw SysError(savedErrno, "reading symbolic link %1%", PathFmt(descriptorToPath(dirFd) / path.rel())); + throw SysError( + [&] { return HintFmt("reading symbolic link %1%", PathFmt(descriptorToPath(dirFd) / path.rel())); }); } else if (rlSize < bufSize) return {buf.data(), static_cast(rlSize)}; } diff --git a/src/libutil/windows/current-process.cc b/src/libutil/windows/current-process.cc index d4392e2279d..3d252257e19 100644 --- a/src/libutil/windows/current-process.cc +++ b/src/libutil/windows/current-process.cc @@ -16,8 +16,7 @@ std::chrono::microseconds getCpuUserTime() FILETIME userTime; if (!GetProcessTimes(GetCurrentProcess(), &creationTime, &exitTime, &kernelTime, &userTime)) { - auto lastError = GetLastError(); - throw windows::WinError(lastError, "failed to get CPU time"); + throw windows::WinError("failed to get CPU time"); } ULARGE_INTEGER uLargeInt; diff --git a/src/libutil/windows/file-descriptor.cc b/src/libutil/windows/file-descriptor.cc index 2e8d35ed6e6..72685913519 100644 --- a/src/libutil/windows/file-descriptor.cc +++ b/src/libutil/windows/file-descriptor.cc @@ -20,8 +20,7 @@ std::make_unsigned_t getFileSize(Descriptor fd) { LARGE_INTEGER li; if (!GetFileSizeEx(fd, &li)) { - auto lastError = GetLastError(); - throw WinError(lastError, "getting size of file %s", PathFmt(descriptorToPath(fd))); + throw WinError([&] { return HintFmt("getting size of file %s", PathFmt(descriptorToPath(fd))); }); } return li.QuadPart; } @@ -49,13 +48,10 @@ size_t readOffset(Descriptor fd, off_t offset, std::span buffer) ov.OffsetHigh = static_cast(offset >> 32); DWORD n; if (!ReadFile(fd, buffer.data(), static_cast(buffer.size()), &n, &ov)) { - auto lastError = GetLastError(); - throw WinError( - lastError, - "reading %1% bytes at offset %2% from %3%", - buffer.size(), - offset, - PathFmt(descriptorToPath(fd))); + throw WinError([&] { + return HintFmt( + "reading %1% bytes at offset %2% from %3%", buffer.size(), offset, PathFmt(descriptorToPath(fd))); + }); } return static_cast(n); } @@ -66,8 +62,8 @@ size_t write(Descriptor fd, std::span buffer, bool allowInterru checkInterrupt(); // For consistency with unix DWORD n; if (!WriteFile(fd, buffer.data(), static_cast(buffer.size()), &n, NULL)) { - auto lastError = GetLastError(); - throw WinError(lastError, "writing %1% bytes to %2%", buffer.size(), PathFmt(descriptorToPath(fd))); + throw WinError( + [&] { return HintFmt("writing %1% bytes to %2%", buffer.size(), PathFmt(descriptorToPath(fd))); }); } return static_cast(n); } @@ -125,8 +121,7 @@ off_t lseek(HANDLE h, off_t offset, int whence) void syncDescriptor(Descriptor fd) { if (!::FlushFileBuffers(fd)) { - auto lastError = GetLastError(); - throw WinError(lastError, "flushing file %s", PathFmt(descriptorToPath(fd))); + throw WinError([&] { return HintFmt("flushing file %s", PathFmt(descriptorToPath(fd))); }); } } diff --git a/src/libutil/windows/muxable-pipe.cc b/src/libutil/windows/muxable-pipe.cc index f258f0f414b..c005118007b 100644 --- a/src/libutil/windows/muxable-pipe.cc +++ b/src/libutil/windows/muxable-pipe.cc @@ -7,15 +7,17 @@ namespace nix { +using namespace nix::windows; + void MuxablePipePollState::poll(HANDLE ioport, std::optional timeout) { /* We are on at least Windows Vista / Server 2008 and can get many (countof(oentries)) statuses in one API call. */ if (!GetQueuedCompletionStatusEx( ioport, oentries, sizeof(oentries) / sizeof(*oentries), &removed, timeout ? *timeout : INFINITE, false)) { - windows::WinError winError("GetQueuedCompletionStatusEx"); - if (winError.lastError != WAIT_TIMEOUT) - throw winError; + auto lastError = GetLastError(); + if (lastError != WAIT_TIMEOUT) + throw WinError(lastError, "GetQueuedCompletionStatusEx"); assert(removed == 0); } else { assert(0 < removed && removed <= sizeof(oentries) / sizeof(*oentries)); @@ -52,12 +54,12 @@ void MuxablePipePollState::iterate( // here is possible (but not obligatory) to call // `handleRead` and repeat ReadFile immediately } else { - windows::WinError winError("ReadFile(%s, ..)", (*p)->readSide.get()); - if (winError.lastError == ERROR_BROKEN_PIPE) { + auto lastError = GetLastError(); + if (lastError == ERROR_BROKEN_PIPE) { handleEOF((*p)->readSide.get()); nextp = channels.erase(p); // no need to maintain `channels` ? - } else if (winError.lastError != ERROR_IO_PENDING) - throw winError; + } else if (lastError != ERROR_IO_PENDING) + throw WinError(lastError, "ReadFile(%s, ..)", (*p)->readSide.get()); } } break;