From 98299da0fda612b42ab933c47f18163cfef5fa71 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Mon, 25 May 2020 01:19:02 +0100 Subject: refactor(3p/nix/libutil): Replace string2Int & trim functions Replaces these functions with corresponding functions from Abseil, namely absl::StripAsciiWhitespace and absl::SimpleAtoi. In the course of doing this some minor things I encountered along the way were also refactored. This also changes the signatures of the various custom readFile functions to use absl::string_view types. --- third_party/nix/src/libexpr/attr-path.cc | 4 ++- third_party/nix/src/libexpr/get-drvs.cc | 3 ++- third_party/nix/src/libexpr/names.cc | 6 +++-- third_party/nix/src/libmain/shared.hh | 3 ++- third_party/nix/src/libstore/binary-cache-store.cc | 13 +++++++--- third_party/nix/src/libstore/build.cc | 6 +++-- third_party/nix/src/libstore/download.cc | 16 +++++++----- third_party/nix/src/libstore/globals.cc | 3 ++- third_party/nix/src/libstore/local-store.cc | 3 ++- third_party/nix/src/libstore/machines.cc | 5 ++-- third_party/nix/src/libstore/nar-info.cc | 6 +++-- third_party/nix/src/libstore/profiles.cc | 29 ++++++++++++---------- third_party/nix/src/libstore/store-api.cc | 5 ++-- third_party/nix/src/libutil/args.hh | 4 ++- third_party/nix/src/libutil/config.cc | 3 ++- third_party/nix/src/libutil/meson.build | 2 +- third_party/nix/src/libutil/util.cc | 19 +++++--------- third_party/nix/src/libutil/util.hh | 19 +++----------- third_party/nix/src/nix-env/nix-env.cc | 7 +++--- 19 files changed, 84 insertions(+), 72 deletions(-) diff --git a/third_party/nix/src/libexpr/attr-path.cc b/third_party/nix/src/libexpr/attr-path.cc index 628b58c1b1..5f14fca214 100644 --- a/third_party/nix/src/libexpr/attr-path.cc +++ b/third_party/nix/src/libexpr/attr-path.cc @@ -1,5 +1,7 @@ #include "attr-path.hh" +#include + #include "eval-inline.hh" #include "util.hh" @@ -50,7 +52,7 @@ Value* findAlongAttrPath(EvalState& state, const std::string& attrPath, /* Is i an index (integer) or a normal attribute name? */ enum { apAttr, apIndex } apType = apAttr; unsigned int attrIndex; - if (string2Int(attr, attrIndex)) { + if (absl::SimpleAtoi(attr, &attrIndex)) { apType = apIndex; } diff --git a/third_party/nix/src/libexpr/get-drvs.cc b/third_party/nix/src/libexpr/get-drvs.cc index bc40ec3fef..968108e78c 100644 --- a/third_party/nix/src/libexpr/get-drvs.cc +++ b/third_party/nix/src/libexpr/get-drvs.cc @@ -4,6 +4,7 @@ #include #include +#include #include #include "derivations.hh" @@ -243,7 +244,7 @@ NixInt DrvInfo::queryMetaInt(const std::string& name, NixInt def) { /* Backwards compatibility with before we had support for integer meta fields. */ NixInt n; - if (string2Int(v->string.s, n)) { + if (absl::SimpleAtoi(v->string.s, &n)) { return n; } } diff --git a/third_party/nix/src/libexpr/names.cc b/third_party/nix/src/libexpr/names.cc index 20c326776c..769f9e99db 100644 --- a/third_party/nix/src/libexpr/names.cc +++ b/third_party/nix/src/libexpr/names.cc @@ -2,6 +2,8 @@ #include +#include + #include "util.hh" namespace nix { @@ -68,8 +70,8 @@ std::string nextComponent(std::string::const_iterator& p, static bool componentsLT(const std::string& c1, const std::string& c2) { int n1; int n2; - bool c1Num = string2Int(c1, n1); - bool c2Num = string2Int(c2, n2); + bool c1Num = absl::SimpleAtoi(c1, &n1); + bool c2Num = absl::SimpleAtoi(c2, &n2); if (c1Num && c2Num) { return n1 < n2; diff --git a/third_party/nix/src/libmain/shared.hh b/third_party/nix/src/libmain/shared.hh index edd0a5159a..60d7918931 100644 --- a/third_party/nix/src/libmain/shared.hh +++ b/third_party/nix/src/libmain/shared.hh @@ -2,6 +2,7 @@ #include +#include #include #include "args.hh" @@ -78,7 +79,7 @@ N getIntArg(const std::string& opt, Strings::iterator& i, } } N n; - if (!string2Int(s, n)) + if (!absl::SimpleAtoi(s, &n)) throw UsageError(format("'%1%' requires an integer argument") % opt); return n * multiplier; } diff --git a/third_party/nix/src/libstore/binary-cache-store.cc b/third_party/nix/src/libstore/binary-cache-store.cc index 37d1c1a440..ea36078435 100644 --- a/third_party/nix/src/libstore/binary-cache-store.cc +++ b/third_party/nix/src/libstore/binary-cache-store.cc @@ -4,6 +4,9 @@ #include #include +#include +#include + #include "archive.hh" #include "compression.hh" #include "derivations.hh" @@ -21,7 +24,8 @@ namespace nix { BinaryCacheStore::BinaryCacheStore(const Params& params) : Store(params) { if (secretKeyFile != "") { - secretKey = std::make_unique(readFile(secretKeyFile)); + const std::string& secret_key_file = secretKeyFile; + secretKey = std::make_unique(readFile(secret_key_file)); } StringSink sink; @@ -43,7 +47,8 @@ void BinaryCacheStore::init() { continue; } auto name = line.substr(0, colon); - auto value = trim(line.substr(colon + 1, std::string::npos)); + auto value = + absl::StripAsciiWhitespace(line.substr(colon + 1, std::string::npos)); if (name == "StoreDir") { if (value != storeDir) { throw Error(format("binary cache '%s' is for Nix stores with prefix " @@ -53,7 +58,9 @@ void BinaryCacheStore::init() { } else if (name == "WantMassQuery") { wantMassQuery_ = value == "1"; } else if (name == "Priority") { - string2Int(value, priority); + if (!absl::SimpleAtoi(value, &priority)) { + LOG(WARNING) << "Invalid 'Priority' value: " << value; + } } } } diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc index 49204a72a8..c0fa0074d6 100644 --- a/third_party/nix/src/libstore/build.cc +++ b/third_party/nix/src/libstore/build.cc @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -2412,7 +2413,7 @@ void DerivationGoal::startBuilder() { userNamespaceSync.readSide = -1; pid_t tmp; - if (!string2Int(readLine(builderOut.readSide.get()), tmp)) { + if (!absl::SimpleAtoi(readLine(builderOut.readSide.get()), &tmp)) { abort(); } pid = tmp; @@ -2805,7 +2806,8 @@ void DerivationGoal::runChild() { std::string netrcData; try { if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") { - netrcData = readFile(settings.netrcFile); + const std::string& netrc_file = settings.netrcFile; + netrcData = readFile(netrc_file); } } catch (SysError&) { } diff --git a/third_party/nix/src/libstore/download.cc b/third_party/nix/src/libstore/download.cc index a83599715c..bebe0d1bf8 100644 --- a/third_party/nix/src/libstore/download.cc +++ b/third_party/nix/src/libstore/download.cc @@ -1,6 +1,7 @@ #include "download.hh" #include +#include #include "archive.hh" #include "compression.hh" @@ -174,7 +175,8 @@ struct CurlDownloader : public Downloader { size_t headerCallback(void* contents, size_t size, size_t nmemb) { size_t realSize = size * nmemb; std::string line((char*)contents, realSize); - DLOG(INFO) << "got header for '" << request.uri << "': " << trim(line); + DLOG(INFO) << "got header for '" << request.uri + << "': " << absl::StripAsciiWhitespace(line); if (line.compare(0, 5, "HTTP/") == 0) { // new response starts result.etag = ""; auto ss = tokenizeString>(line, " "); @@ -186,9 +188,10 @@ struct CurlDownloader : public Downloader { } else { auto i = line.find(':'); if (i != std::string::npos) { - std::string name = toLower(trim(std::string(line, 0, i))); + std::string name = absl::AsciiStrToLower( + absl::StripAsciiWhitespace(std::string(line, 0, i))); if (name == "etag") { - result.etag = trim(std::string(line, i + 1)); + result.etag = absl::StripAsciiWhitespace(std::string(line, i + 1)); /* Hack to work around a GitHub bug: it sends ETags, but ignores If-None-Match. So if we get the expected ETag on a 200 response, then shut @@ -200,9 +203,10 @@ struct CurlDownloader : public Downloader { return 0; } } else if (name == "content-encoding") { - encoding = trim(std::string(line, i + 1)); + encoding = absl::StripAsciiWhitespace(std::string(line, i + 1)); } else if (name == "accept-ranges" && - toLower(trim(std::string(line, i + 1))) == "bytes") { + absl::AsciiStrToLower(absl::StripAsciiWhitespace( + std::string(line, i + 1))) == "bytes") { acceptRanges = true; } } @@ -893,7 +897,7 @@ CachedDownloadResult Downloader::downloadCached( tokenizeString>(readFile(dataFile), "\n"); if (ss.size() >= 3 && ss[0] == url) { time_t lastChecked; - if (string2Int(ss[2], lastChecked) && + if (absl::SimpleAtoi(ss[2], &lastChecked) && (uint64_t)lastChecked + request.ttl >= (uint64_t)time(nullptr)) { skip = true; result.effectiveUri = request.uri; diff --git a/third_party/nix/src/libstore/globals.cc b/third_party/nix/src/libstore/globals.cc index 61293dffed..7307f03527 100644 --- a/third_party/nix/src/libstore/globals.cc +++ b/third_party/nix/src/libstore/globals.cc @@ -4,6 +4,7 @@ #include #include +#include #include #include "archive.hh" @@ -163,7 +164,7 @@ void BaseSetting::convertToArg(Args& args, void MaxBuildJobsSetting::set(const std::string& str) { if (str == "auto") { value = std::max(1U, std::thread::hardware_concurrency()); - } else if (!string2Int(str, value)) { + } else if (!absl::SimpleAtoi(str, &value)) { throw UsageError( "configuration setting '%s' should be 'auto' or an integer", name); } diff --git a/third_party/nix/src/libstore/local-store.cc b/third_party/nix/src/libstore/local-store.cc index 84055740de..faea0fbce5 100644 --- a/third_party/nix/src/libstore/local-store.cc +++ b/third_party/nix/src/libstore/local-store.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -295,7 +296,7 @@ int LocalStore::getSchema() { int curSchema = 0; if (pathExists(schemaPath)) { std::string s = readFile(schemaPath); - if (!string2Int(s, curSchema)) { + if (!absl::SimpleAtoi(s, &curSchema)) { throw Error(format("'%1%' is corrupt") % schemaPath); } } diff --git a/third_party/nix/src/libstore/machines.cc b/third_party/nix/src/libstore/machines.cc index 2f1a7289bd..6472e037d1 100644 --- a/third_party/nix/src/libstore/machines.cc +++ b/third_party/nix/src/libstore/machines.cc @@ -2,6 +2,8 @@ #include +#include +#include #include #include "globals.hh" @@ -48,14 +50,13 @@ bool Machine::mandatoryMet(const std::set& features) const { void parseMachines(const std::string& s, Machines& machines) { for (auto line : tokenizeString>(s, "\n;")) { - trim(line); line.erase(std::find(line.begin(), line.end(), '#'), line.end()); if (line.empty()) { continue; } if (line[0] == '@') { - auto file = trim(std::string(line, 1)); + auto file = absl::StripAsciiWhitespace(std::string(line, 1)); try { parseMachines(readFile(file), machines); } catch (const SysError& e) { diff --git a/third_party/nix/src/libstore/nar-info.cc b/third_party/nix/src/libstore/nar-info.cc index 5a217d1b61..9f4c53e19b 100644 --- a/third_party/nix/src/libstore/nar-info.cc +++ b/third_party/nix/src/libstore/nar-info.cc @@ -1,5 +1,7 @@ #include "nar-info.hh" +#include + #include "globals.hh" namespace nix { @@ -47,13 +49,13 @@ NarInfo::NarInfo(const Store& store, const std::string& s, } else if (name == "FileHash") { fileHash = parseHashField(value); } else if (name == "FileSize") { - if (!string2Int(value, fileSize)) { + if (!absl::SimpleAtoi(value, &fileSize)) { corrupt(); } } else if (name == "NarHash") { narHash = parseHashField(value); } else if (name == "NarSize") { - if (!string2Int(value, narSize)) { + if (!absl::SimpleAtoi(value, &narSize)) { corrupt(); } } else if (name == "References") { diff --git a/third_party/nix/src/libstore/profiles.cc b/third_party/nix/src/libstore/profiles.cc index 7c802d7549..3516954b50 100644 --- a/third_party/nix/src/libstore/profiles.cc +++ b/third_party/nix/src/libstore/profiles.cc @@ -3,6 +3,9 @@ #include #include +#include +#include +#include #include #include #include @@ -17,22 +20,22 @@ static bool cmpGensByNumber(const Generation& a, const Generation& b) { return a.number < b.number; } -/* Parse a generation name of the format - `--link'. */ -static int parseName(const std::string& profileName, const std::string& name) { - if (std::string(name, 0, profileName.size() + 1) != profileName + "-") { - return -1; - } - std::string s = std::string(name, profileName.size() + 1); - std::string::size_type p = s.find("-link"); - if (p == std::string::npos) { +// Parse a generation out of the format +// `--link'. +static int parseName(absl::string_view profileName, absl::string_view name) { + // Consume the `-' prefix and and `-link' suffix. + if (!(absl::ConsumePrefix(&name, profileName) && + absl::ConsumePrefix(&name, "-") && + absl::ConsumeSuffix(&name, "-link"))) { return -1; } + int n; - if (string2Int(std::string(s, 0, p), n) && n >= 0) { - return n; + if (!absl::SimpleAtoi(name, &n) && n < 0) { + return -1; } - return -1; + + return n; } Generations findGenerations(const Path& profile, int& curGen) { @@ -218,7 +221,7 @@ void deleteGenerationsOlderThan(const Path& profile, std::string strDays = std::string(timeSpec, 0, timeSpec.size() - 1); int days; - if (!string2Int(strDays, days) || days < 1) { + if (!absl::SimpleAtoi(strDays, &days) || days < 1) { throw Error(format("invalid number of days specifier '%1%'") % timeSpec); } diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc index e6891395cd..54214e6f4e 100644 --- a/third_party/nix/src/libstore/store-api.cc +++ b/third_party/nix/src/libstore/store-api.cc @@ -3,6 +3,7 @@ #include #include +#include #include #include "crypto.hh" @@ -718,7 +719,7 @@ ValidPathInfo decodeValidPathInfo(std::istream& str, bool hashGiven) { getline(str, s); info.narHash = Hash(s, htSHA256); getline(str, s); - if (!string2Int(s, info.narSize)) { + if (!absl::SimpleAtoi(s, &info.narSize)) { throw Error("number expected"); } } @@ -726,7 +727,7 @@ ValidPathInfo decodeValidPathInfo(std::istream& str, bool hashGiven) { std::string s; int n; getline(str, s); - if (!string2Int(s, n)) { + if (!absl::SimpleAtoi(s, &n)) { throw Error("number expected"); } while ((n--) != 0) { diff --git a/third_party/nix/src/libutil/args.hh b/third_party/nix/src/libutil/args.hh index 20a6379fec..2c352a0436 100644 --- a/third_party/nix/src/libutil/args.hh +++ b/third_party/nix/src/libutil/args.hh @@ -4,6 +4,8 @@ #include #include +#include + #include "util.hh" namespace nix { @@ -179,7 +181,7 @@ class Args { .arity(1) .handler([=](std::vector ss) { I n; - if (!string2Int(ss[0], n)) + if (!absl::SimpleAtoi(ss[0], &n)) throw UsageError("flag '--%s' requires a integer argument", longName); fun(n); diff --git a/third_party/nix/src/libutil/config.cc b/third_party/nix/src/libutil/config.cc index c7c952abf1..bc81ce77ce 100644 --- a/third_party/nix/src/libutil/config.cc +++ b/third_party/nix/src/libutil/config.cc @@ -3,6 +3,7 @@ #include +#include #include #include "args.hh" @@ -214,7 +215,7 @@ std::string BaseSetting::to_string() { template void BaseSetting::set(const std::string& str) { static_assert(std::is_integral::value, "Integer required."); - if (!string2Int(str, value)) { + if (!absl::SimpleAtoi(str, &value)) { throw UsageError("setting '%s' has invalid value '%s'", name, str); } } diff --git a/third_party/nix/src/libutil/meson.build b/third_party/nix/src/libutil/meson.build index a6494e6eac..50043a8fad 100644 --- a/third_party/nix/src/libutil/meson.build +++ b/third_party/nix/src/libutil/meson.build @@ -47,7 +47,7 @@ libutil_dep_list = [ openssl_dep, pthread_dep, libsodium_dep, -] +] + absl_deps libutil_link_list = [] libutil_link_args = [] diff --git a/third_party/nix/src/libutil/util.cc b/third_party/nix/src/libutil/util.cc index 696a8ff236..8dca97af98 100644 --- a/third_party/nix/src/libutil/util.cc +++ b/third_party/nix/src/libutil/util.cc @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -306,16 +307,17 @@ std::string readFile(int fd) { return std::string((char*)buf.data(), st.st_size); } -std::string readFile(const Path& path, bool drain) { - AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); +std::string readFile(absl::string_view path, bool drain) { + AutoCloseFD fd = open(std::string(path).c_str(), O_RDONLY | O_CLOEXEC); if (!fd) { throw SysError(format("opening file '%1%'") % path); } return drain ? drainFD(fd.get()) : readFile(fd.get()); } -void readFile(const Path& path, Sink& sink) { - AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); +void readFile(absl::string_view path, Sink& sink) { + // TODO(tazjin): use stdlib functions for this stuff + AutoCloseFD fd = open(std::string(path).c_str(), O_RDONLY | O_CLOEXEC); if (!fd) { throw SysError("opening file '%s'", path); } @@ -1213,15 +1215,6 @@ std::string concatStringsSep(const std::string& sep, const StringSet& ss) { return s; } -std::string trim(const std::string& s, const std::string& whitespace) { - auto i = s.find_first_not_of(whitespace); - if (i == std::string::npos) { - return ""; - } - auto j = s.find_last_not_of(whitespace); - return std::string(s, i, j == std::string::npos ? j : j - i + 1); -} - std::string replaceStrings(const std::string& s, const std::string& from, const std::string& to) { if (from.empty()) { diff --git a/third_party/nix/src/libutil/util.hh b/third_party/nix/src/libutil/util.hh index 7d10df50bf..0f3752dfde 100644 --- a/third_party/nix/src/libutil/util.hh +++ b/third_party/nix/src/libutil/util.hh @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -97,8 +98,8 @@ unsigned char getFileType(const Path& path); /* Read the contents of a file into a string. */ std::string readFile(int fd); -std::string readFile(const Path& path, bool drain = false); -void readFile(const Path& path, Sink& sink); +std::string readFile(absl::string_view path, bool drain = false); +void readFile(absl::string_view path, Sink& sink); /* Write a string to a file. */ void writeFile(const Path& path, const std::string& s, mode_t mode = 0666); @@ -325,10 +326,6 @@ C tokenizeString(const std::string& s, std::string concatStringsSep(const std::string& sep, const Strings& ss); std::string concatStringsSep(const std::string& sep, const StringSet& ss); -/* Remove whitespace from the start and end of a string. */ -std::string trim(const std::string& s, - const std::string& whitespace = " \n\r\t"); - /* Replace all occurrences of a string inside another string. */ std::string replaceStrings(const std::string& s, const std::string& from, const std::string& to); @@ -339,16 +336,6 @@ std::string statusToString(int status); bool statusOk(int status); -/* Parse a string into an integer. */ -template -bool string2Int(const std::string& s, N& n) { - if (std::string(s, 0, 1) == "-" && !std::numeric_limits::is_signed) - return false; - std::istringstream str(s); - str >> n; - return str && str.get() == EOF; -} - /* Parse a string into a float. */ template bool string2Float(const std::string& s, N& n) { diff --git a/third_party/nix/src/nix-env/nix-env.cc b/third_party/nix/src/nix-env/nix-env.cc index e827f31e5d..d17bde34f3 100644 --- a/third_party/nix/src/nix-env/nix-env.cc +++ b/third_party/nix/src/nix-env/nix-env.cc @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -1303,7 +1304,7 @@ static void opSwitchGeneration(Globals& globals, Strings opFlags, } int dstGen; - if (!string2Int(opArgs.front(), dstGen)) { + if (!absl::SimpleAtoi(opArgs.front(), &dstGen)) { throw UsageError(format("expected a generation number")); } @@ -1369,7 +1370,7 @@ static void opDeleteGenerations(Globals& globals, Strings opFlags, } std::string str_max = std::string(opArgs.front(), 1, opArgs.front().size()); int max; - if (!string2Int(str_max, max) || max == 0) { + if (!absl::SimpleAtoi(str_max, &max) || max == 0) { throw Error(format("invalid number of generations to keep ‘%1%’") % opArgs.front()); } @@ -1378,7 +1379,7 @@ static void opDeleteGenerations(Globals& globals, Strings opFlags, std::set gens; for (auto& i : opArgs) { unsigned int n; - if (!string2Int(i, n)) { + if (!absl::SimpleAtoi(i, &n)) { throw UsageError(format("invalid generation number '%1%'") % i); } gens.insert(n); -- cgit 1.4.1