From 381ce8a66658ac9d02c44e96c860cd05bcb6a5f8 Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Mon, 7 Sep 2020 13:01:49 -0400 Subject: refactor(tvix): Make static strings constexpr string_views Make all static std::strings constexpr std::string_views, and replace concatenation with absl::StrCat where necessary. Technically all of these are constant, so they really don't need to be top-level statics - and since I'm trying to get rid of as much global state as possible in preparation for making the nix daemon properly multithreaded I figured I'd knock these out while I was at it. Change-Id: Ibd3ad9ef68f0a0eacb135541b39fdb13dae042e1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/1939 Tested-by: BuildkiteCI Reviewed-by: tazjin --- third_party/nix/src/libstore/binary-cache-store.cc | 2 +- third_party/nix/src/libstore/build.cc | 6 +++--- third_party/nix/src/libstore/gc.cc | 25 +++++++++++----------- third_party/nix/src/libstore/legacy-ssh-store.cc | 10 +++++---- third_party/nix/src/libstore/rpc-store.cc | 4 ++-- third_party/nix/src/libstore/ssh-store.cc | 23 +++++++++++--------- 6 files changed, 38 insertions(+), 32 deletions(-) (limited to 'third_party/nix/src/libstore') diff --git a/third_party/nix/src/libstore/binary-cache-store.cc b/third_party/nix/src/libstore/binary-cache-store.cc index 91da7e22659d..e5b7ef2cdc3e 100644 --- a/third_party/nix/src/libstore/binary-cache-store.cc +++ b/third_party/nix/src/libstore/binary-cache-store.cc @@ -30,7 +30,7 @@ BinaryCacheStore::BinaryCacheStore(const Params& params) : Store(params) { } StringSink sink; - sink << narVersionMagic1; + sink << std::string(kNarVersionMagic1); narMagic = *sink.s; } diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc index a009651c9d96..e50f4cfa9901 100644 --- a/third_party/nix/src/libstore/build.cc +++ b/third_party/nix/src/libstore/build.cc @@ -81,7 +81,7 @@ namespace nix { -static std::string pathNullDevice = "/dev/null"; +constexpr std::string_view kPathNullDevice = "/dev/null"; /* Forward definition. */ class Worker; @@ -448,9 +448,9 @@ static void commonChildInit(Pipe& logPipe) { } /* Reroute stdin to /dev/null. */ - int fdDevNull = open(pathNullDevice.c_str(), O_RDWR); + int fdDevNull = open(kPathNullDevice.begin(), O_RDWR); if (fdDevNull == -1) { - throw SysError(format("cannot open '%1%'") % pathNullDevice); + throw SysError(format("cannot open '%1%'") % kPathNullDevice); } if (dup2(fdDevNull, STDIN_FILENO) == -1) { throw SysError("cannot dup null device into stdin"); diff --git a/third_party/nix/src/libstore/gc.cc b/third_party/nix/src/libstore/gc.cc index 50c52bb009bb..07dc10629a91 100644 --- a/third_party/nix/src/libstore/gc.cc +++ b/third_party/nix/src/libstore/gc.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -22,8 +23,8 @@ namespace nix { -static std::string gcLockName = "gc.lock"; -static std::string gcRootsDir = "gcroots"; +constexpr std::string_view kGcLockName = "gc.lock"; +constexpr std::string_view kGcRootsDir = "gcroots"; /* Acquire the global GC lock. This is used to prevent new Nix processes from starting after the temporary root files have been @@ -31,7 +32,7 @@ static std::string gcRootsDir = "gcroots"; file, they will block until the garbage collector has finished / yielded the GC lock. */ AutoCloseFD LocalStore::openGCLock(LockType lockType) { - Path fnGCLock = (format("%1%/%2%") % stateDir % gcLockName).str(); + Path fnGCLock = absl::StrCat(stateDir.get(), "/", kGcLockName); DLOG(INFO) << "acquiring global GC lock " << fnGCLock; @@ -73,8 +74,8 @@ void LocalStore::syncWithGC() { AutoCloseFD fdGCLock = openGCLock(ltRead); } void LocalStore::addIndirectRoot(const Path& path) { std::string hash = hashString(htSHA1, path).to_string(Base32, false); - Path realRoot = canonPath( - (format("%1%/%2%/auto/%3%") % stateDir % gcRootsDir % hash).str()); + Path realRoot = + canonPath(absl::StrCat(stateDir.get(), "/", kGcRootsDir, "/auto/", hash)); makeSymlink(realRoot, path); } @@ -105,8 +106,7 @@ Path LocalFSStore::addPermRoot(const Path& _storePath, const Path& _gcRoot, else { if (!allowOutsideRootsDir) { - Path rootsDir = - canonPath((format("%1%/%2%") % stateDir % gcRootsDir).str()); + Path rootsDir = canonPath(absl::StrCat(stateDir.get(), "/", kGcRootsDir)); if (std::string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/") { throw Error(format("path '%1%' is not a valid garbage collector root; " @@ -195,7 +195,7 @@ void LocalStore::addTempRoot(const Path& path) { lockFile(state->fdTempRoots.get(), ltRead, true); } -static std::string censored = "{censored}"; +constexpr std::string_view kCensored = "{censored}"; void LocalStore::findTempRoots(FDs& fds, Roots& tempRoots, bool censor) { /* Read the `temproots' directory for per-process temporary root @@ -247,7 +247,7 @@ void LocalStore::findTempRoots(FDs& fds, Roots& tempRoots, bool censor) { Path root(contents, pos, end - pos); DLOG(INFO) << "got temporary root " << root; assertStorePath(root); - tempRoots[root].emplace(censor ? censored : fmt("{temp:%d}", pid)); + tempRoots[root].emplace(censor ? kCensored : fmt("{temp:%d}", pid)); pos = end + 1; } @@ -287,7 +287,8 @@ void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) { else { target = absPath(target, dirOf(path)); if (!pathExists(target)) { - if (isInDir(path, stateDir + "/" + gcRootsDir + "/auto")) { + if (isInDir(path, absl::StrCat(stateDir.get(), "/", kGcRootsDir, + "/auto"))) { LOG(INFO) << "removing stale link from '" << path << "' to '" << target << "'"; unlink(path.c_str()); @@ -326,7 +327,7 @@ void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) { void LocalStore::findRootsNoTemp(Roots& roots, bool censor) { /* Process direct roots in {gcroots,profiles}. */ - findRoots(stateDir + "/" + gcRootsDir, DT_UNKNOWN, roots); + findRoots(absl::StrCat(stateDir.get(), "/", kGcRootsDir), DT_UNKNOWN, roots); findRoots(stateDir + "/profiles", DT_UNKNOWN, roots); /* Add additional roots returned by different platforms-specific @@ -465,7 +466,7 @@ void LocalStore::findRuntimeRoots(Roots& roots, bool censor) { if (isStorePath(path) && isValidPath(path)) { DLOG(INFO) << "got additional root " << path; if (censor) { - roots[path].insert(censored); + roots[path].insert(std::string(kCensored)); } else { roots[path].insert(links.begin(), links.end()); } diff --git a/third_party/nix/src/libstore/legacy-ssh-store.cc b/third_party/nix/src/libstore/legacy-ssh-store.cc index 343801870a96..19603115a81c 100644 --- a/third_party/nix/src/libstore/legacy-ssh-store.cc +++ b/third_party/nix/src/libstore/legacy-ssh-store.cc @@ -1,3 +1,5 @@ +#include +#include #include #include "libstore/derivations.hh" @@ -11,7 +13,7 @@ namespace nix { -static std::string uriScheme = "ssh://"; +constexpr std::string_view kUriScheme = "ssh://"; struct LegacySSHStore : public Store { const Setting maxConnections{ @@ -86,7 +88,7 @@ struct LegacySSHStore : public Store { return conn; }; - std::string getUri() override { return uriScheme + host; } + std::string getUri() override { return absl::StrCat(kUriScheme, host); } void queryPathInfoUncached( const Path& path, @@ -269,11 +271,11 @@ struct LegacySSHStore : public Store { static RegisterStoreImplementation regStore( [](const std::string& uri, const Store::Params& params) -> std::shared_ptr { - if (std::string(uri, 0, uriScheme.size()) != uriScheme) { + if (!absl::StartsWith(uri, kUriScheme)) { return nullptr; } return std::make_shared( - std::string(uri, uriScheme.size()), params); + std::string(uri, kUriScheme.size()), params); }); } // namespace nix diff --git a/third_party/nix/src/libstore/rpc-store.cc b/third_party/nix/src/libstore/rpc-store.cc index bc1fcc73d818..02975341fd40 100644 --- a/third_party/nix/src/libstore/rpc-store.cc +++ b/third_party/nix/src/libstore/rpc-store.cc @@ -531,13 +531,13 @@ unsigned int RpcStore::getProtocol() { return PROTOCOL_VERSION; } } // namespace store -static std::string uriScheme = "unix://"; +constexpr std::string_view kUriScheme = "unix://"; // TODO(grfn): Make this a function that we call from main rather than... this static RegisterStoreImplementation regStore([](const std::string& uri, const Store::Params& params) -> std::shared_ptr { - if (std::string(uri, 0, uriScheme.size()) != uriScheme) { + if (std::string(uri, 0, kUriScheme.size()) != kUriScheme) { return nullptr; } auto channel = grpc::CreateChannel(uri, grpc::InsecureChannelCredentials()); diff --git a/third_party/nix/src/libstore/ssh-store.cc b/third_party/nix/src/libstore/ssh-store.cc index 48fea858a393..96adb3660d42 100644 --- a/third_party/nix/src/libstore/ssh-store.cc +++ b/third_party/nix/src/libstore/ssh-store.cc @@ -1,3 +1,5 @@ +#include + #include "libstore/remote-fs-accessor.hh" #include "libstore/remote-store.hh" #include "libstore/ssh.hh" @@ -8,7 +10,7 @@ namespace nix { -static std::string uriScheme = "ssh-ng://"; +constexpr std::string_view kUriScheme = "ssh-ng://"; class SSHStore : public RemoteStore { public: @@ -25,7 +27,7 @@ class SSHStore : public RemoteStore { // Use SSH master only if using more than 1 connection. connections->capacity() > 1, compress) {} - std::string getUri() override { return uriScheme + host; } + std::string getUri() override { return absl::StrCat(kUriScheme, host); } bool sameMachine() override { return false; } @@ -74,13 +76,14 @@ ref SSHStore::openConnection() { return conn; } -static RegisterStoreImplementation regStore([](const std::string& uri, - const Store::Params& params) - -> std::shared_ptr { - if (std::string(uri, 0, uriScheme.size()) != uriScheme) { - return nullptr; - } - return std::make_shared(std::string(uri, uriScheme.size()), params); -}); +static RegisterStoreImplementation regStore( + [](const std::string& uri, + const Store::Params& params) -> std::shared_ptr { + if (std::string(uri, 0, kUriScheme.size()) != kUriScheme) { + return nullptr; + } + return std::make_shared(std::string(uri, kUriScheme.size()), + params); + }); } // namespace nix -- cgit 1.4.1