From b490742a511dd03afc43f5143d6d61edaeeb8091 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 19 May 2020 17:38:04 +0100 Subject: style(3p/nix): Enforce braces around loops and conditionals This change was generated with: fd -e cc -e hh | xargs -I{} clang-tidy {} -p ~/projects/nix-build/ \ --checks='-*,readability-braces-around-statements' --fix \ -fix-errors Some manual fixes were applied because some convoluted unbraced statements couldn't be untangled by clang-tidy. This commit still includes invalid files, but I decided to clean them up in a subsequent commit so that it becomes more obvious where clang-tidy failed. Maybe this will allow for a bug-report to clang-tidy. --- third_party/nix/src/libstore/build.cc | 58 ++++++++++++++++------ third_party/nix/src/libstore/derivations.cc | 19 ++++--- third_party/nix/src/libstore/download.cc | 19 +++++-- third_party/nix/src/libstore/gc.cc | 12 +++-- .../nix/src/libstore/http-binary-cache-store.cc | 3 +- third_party/nix/src/libstore/local-fs-store.cc | 4 +- third_party/nix/src/libstore/local-store.cc | 35 +++++++------ third_party/nix/src/libstore/optimise-store.cc | 4 +- third_party/nix/src/libstore/references.cc | 4 +- third_party/nix/src/libstore/remote-store.cc | 7 ++- third_party/nix/src/libstore/sqlite.cc | 31 ++++++++---- third_party/nix/src/libstore/store-api.cc | 9 ++-- 12 files changed, 138 insertions(+), 67 deletions(-) (limited to 'third_party/nix/src/libstore') diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc index 7da262206abd..8257d7aff158 100644 --- a/third_party/nix/src/libstore/build.cc +++ b/third_party/nix/src/libstore/build.cc @@ -350,12 +350,17 @@ void Goal::waiteeDone(GoalPtr waitee, ExitCode result) { trace(format("waitee '%1%' done; %2% left") % waitee->name % waitees.size()); if (result == ecFailed || result == ecNoSubstituters || - result == ecIncompleteClosure) + result == ecIncompleteClosure) { ++nrFailed; + } - if (result == ecNoSubstituters) ++nrNoSubstituters; + if (result == ecNoSubstituters) { + ++nrNoSubstituters; + } - if (result == ecIncompleteClosure) ++nrIncompleteClosure; + if (result == ecIncompleteClosure) { + ++nrIncompleteClosure; + } if (waitees.empty() || (result == ecFailed && !settings.keepGoing)) { /* If we failed and keepGoing is not set, we remove all @@ -1147,7 +1152,9 @@ void DerivationGoal::outputsSubstituted() { /* If the substitutes form an incomplete closure, then we should build the dependencies of this derivation, but after that, we can still use the substitutes for this derivation itself. */ - if (nrIncompleteClosure > 0) retrySubstitution = true; + if (nrIncompleteClosure > 0) { + retrySubstitution = true; + } nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; @@ -1659,7 +1666,9 @@ MakeError(NotDeterministic, BuildError) } HookReply DerivationGoal::tryBuildHook() { - if (!worker.tryBuildHook || !useDerivation) return rpDecline; + if (!worker.tryBuildHook || !useDerivation) { + return rpDecline; + } if (!worker.hook) worker.hook = std::make_unique(); @@ -2231,7 +2240,9 @@ void DerivationGoal::startBuilder() { us. */ - if (!fixedOutput) privateNetwork = true; + if (!fixedOutput) { + privateNetwork = true; + } userNamespaceSync.create(); @@ -3117,7 +3128,9 @@ void DerivationGoal::registerOutputs() { bool allValid = true; for (auto& i : drv->outputs) if (!worker.store.isValidPath(i.second.path)) allValid = false; - if (allValid) return; + if (allValid) { + return; + } } std::map infos; @@ -3345,7 +3358,9 @@ void DerivationGoal::registerOutputs() { infos[i.first] = info; } - if (buildMode == bmCheck) return; + if (buildMode == bmCheck) { + return; + } /* Apply output checks. */ checkOutputs(infos); @@ -4095,9 +4110,10 @@ GoalPtr Worker::makeDerivationGoal(const Path& path, std::make_shared(path, wantedOutputs, *this, buildMode); derivationGoals[path] = goal; wakeUp(goal); - } else + } else { (dynamic_cast(goal.get())) ->addWantedOutputs(wantedOutputs); + } return goal; } @@ -4167,7 +4183,9 @@ void Worker::childStarted(GoalPtr goal, const set& fds, bool inBuildSlot, child.inBuildSlot = inBuildSlot; child.respectTimeouts = respectTimeouts; children.emplace_back(child); - if (inBuildSlot) nrLocalBuilds++; + if (inBuildSlot) { + nrLocalBuilds++; + } } void Worker::childTerminated(Goal* goal, bool wakeSleepers) { @@ -4413,14 +4431,22 @@ unsigned int Worker::exitStatus() { */ unsigned int mask = 0; bool buildFailure = permanentFailure || timedOut || hashMismatch; - if (buildFailure) mask |= 0x04; // 100 - if (timedOut) mask |= 0x01; // 101 - if (hashMismatch) mask |= 0x02; // 102 + if (buildFailure) { + mask |= 0x04; // 100 + } + if (timedOut) { + mask |= 0x01; // 101 + } + if (hashMismatch) { + mask |= 0x02; // 102 + } if (checkMismatch) { mask |= 0x08; // 104 } - if (mask) mask |= 0x60; + if (mask) { + mask |= 0x60; + } return mask ? mask : 1; } @@ -4430,9 +4456,9 @@ bool Worker::pathContentsGood(const Path& path) { LOG(INFO) << "checking path '" << path << "'..."; auto info = store.queryPathInfo(path); bool res; - if (!pathExists(path)) + if (!pathExists(path)) { res = false; - else { + } else { HashResult current = hashPath(info->narHash.type, path); Hash nullHash(htSHA256); res = info->narHash == nullHash || info->narHash == current.first; diff --git a/third_party/nix/src/libstore/derivations.cc b/third_party/nix/src/libstore/derivations.cc index 28f775dee1e9..454f483d1613 100644 --- a/third_party/nix/src/libstore/derivations.cc +++ b/third_party/nix/src/libstore/derivations.cc @@ -185,19 +185,21 @@ Derivation Store::derivationFromPath(const Path& drvPath) { static void printString(string& res, const string& s) { res += '"'; - for (const char* i = s.c_str(); *i; i++) + for (const char* i = s.c_str(); *i; i++) { if (*i == '\"' || *i == '\\') { res += "\\"; res += *i; - } else if (*i == '\n') + } else if (*i == '\n') { res += "\\n"; - else if (*i == '\r') + } else if (*i == '\r') { res += "\\r"; - else if (*i == '\t') + } else if (*i == '\t') { res += "\\t"; - else + } else { res += *i; - res += '"'; + } + res += '"'; + } } template @@ -205,10 +207,11 @@ static void printStrings(string& res, ForwardIterator i, ForwardIterator j) { res += '['; bool first = true; for (; i != j; ++i) { - if (first) + if (first) { first = false; - else + } else { res += ','; + } printString(res, *i); } res += ']'; diff --git a/third_party/nix/src/libstore/download.cc b/third_party/nix/src/libstore/download.cc index 16719ad7b68f..d408a208db76 100644 --- a/third_party/nix/src/libstore/download.cc +++ b/third_party/nix/src/libstore/download.cc @@ -108,10 +108,14 @@ struct CurlDownloader : public Downloader { ~DownloadItem() { if (req) { - if (active) curl_multi_remove_handle(downloader.curlm, req); + if (active) { + curl_multi_remove_handle(downloader.curlm, req); + } curl_easy_cleanup(req); } - if (requestHeaders) curl_slist_free_all(requestHeaders); + if (requestHeaders) { + curl_slist_free_all(requestHeaders); + } try { if (!done) fail(DownloadError( @@ -242,7 +246,9 @@ struct CurlDownloader : public Downloader { } void init() { - if (!req) req = curl_easy_init(); + if (!req) { + req = curl_easy_init(); + } curl_easy_reset(req); @@ -316,8 +322,9 @@ struct CurlDownloader : public Downloader { settings.netrcFile.get().c_str()); curl_easy_setopt(req, CURLOPT_NETRC, CURL_NETRC_OPTIONAL); - if (writtenToSink) + if (writtenToSink) { curl_easy_setopt(req, CURLOPT_RESUME_FROM_LARGE, writtenToSink); + } result.data = std::make_shared(); result.bodySize = 0; @@ -505,7 +512,9 @@ struct CurlDownloader : public Downloader { workerThread.join(); - if (curlm) curl_multi_cleanup(curlm); + if (curlm) { + curl_multi_cleanup(curlm); + } } void stopWorkerThread() { diff --git a/third_party/nix/src/libstore/gc.cc b/third_party/nix/src/libstore/gc.cc index 44c118bed448..19de00143c8b 100644 --- a/third_party/nix/src/libstore/gc.cc +++ b/third_party/nix/src/libstore/gc.cc @@ -630,7 +630,9 @@ void LocalStore::tryToDelete(GCState& state, const Path& path) { ‘nix-store --delete’ doesn't have the unexpected effect of recursing into derivations and outputs. */ state.dead.insert(visited.begin(), visited.end()); - if (state.shouldDelete) deletePathRecursive(state, path); + if (state.shouldDelete) { + deletePathRecursive(state, path); + } } } @@ -704,7 +706,9 @@ void LocalStore::collectGarbage(const GCOptions& options, GCResults& results) { state.shouldDelete = options.action == GCOptions::gcDeleteDead || options.action == GCOptions::gcDeleteSpecific; - if (state.shouldDelete) deletePath(reservedPath); + if (state.shouldDelete) { + deletePath(reservedPath); + } /* Acquire the global GC root. This prevents a) New roots from being added. @@ -737,7 +741,9 @@ void LocalStore::collectGarbage(const GCOptions& options, GCResults& results) { that is not reachable from `roots' is garbage. */ if (state.shouldDelete) { - if (pathExists(trashDir)) deleteGarbage(state, trashDir); + if (pathExists(trashDir)) { + deleteGarbage(state, trashDir); + } try { createDirs(trashDir); } catch (SysError& e) { diff --git a/third_party/nix/src/libstore/http-binary-cache-store.cc b/third_party/nix/src/libstore/http-binary-cache-store.cc index 1bbdc46d3296..cdef7ca03837 100644 --- a/third_party/nix/src/libstore/http-binary-cache-store.cc +++ b/third_party/nix/src/libstore/http-binary-cache-store.cc @@ -77,8 +77,9 @@ class HttpBinaryCacheStore : public BinaryCacheStore { } catch (DownloadError& e) { /* S3 buckets return 403 if a file doesn't exist and the bucket is unlistable, so treat 403 as 404. */ - if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden) + if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden) { return false; + } maybeDisable(); throw; } diff --git a/third_party/nix/src/libstore/local-fs-store.cc b/third_party/nix/src/libstore/local-fs-store.cc index 0361dbd9774d..3dd63498b2dc 100644 --- a/third_party/nix/src/libstore/local-fs-store.cc +++ b/third_party/nix/src/libstore/local-fs-store.cc @@ -96,10 +96,10 @@ std::shared_ptr LocalFSStore::getBuildLog(const Path& path_) { : fmt("%s/%s/%s", logDir, drvsLogDir, baseName); Path logBz2Path = logPath + ".bz2"; - if (pathExists(logPath)) + if (pathExists(logPath)) { return std::make_shared(readFile(logPath)); - else if (pathExists(logBz2Path)) { + } else if (pathExists(logBz2Path)) { try { return decompress("bzip2", readFile(logBz2Path)); } catch (Error&) { diff --git a/third_party/nix/src/libstore/local-store.cc b/third_party/nix/src/libstore/local-store.cc index 4c57c8a32a2b..1e49ab88c260 100644 --- a/third_party/nix/src/libstore/local-store.cc +++ b/third_party/nix/src/libstore/local-store.cc @@ -157,19 +157,17 @@ LocalStore::LocalStore(const Params& params) /* Check the current database schema and if necessary do an upgrade. */ int curSchema = getSchema(); - if (curSchema > nixSchemaVersion) + if (curSchema > nixSchemaVersion) { throw Error( format( "current Nix store schema is version %1%, but I only support %2%") % curSchema % nixSchemaVersion); - else if (curSchema == 0) { /* new store */ + } else if (curSchema == 0) { /* new store */ curSchema = nixSchemaVersion; openDB(*state, true); writeFile(schemaPath, (format("%1%") % nixSchemaVersion).str()); - } - - else if (curSchema < nixSchemaVersion) { + } else if (curSchema < nixSchemaVersion) { if (curSchema < 5) throw Error( "Your Nix store has a database in Berkeley DB format,\n" @@ -219,10 +217,9 @@ LocalStore::LocalStore(const Params& params) writeFile(schemaPath, (format("%1%") % nixSchemaVersion).str()); lockFile(globalLock.get(), ltRead, true); - } - - else + } else { openDB(*state, false); + } /* Prepare SQL statements. */ state->stmtRegisterValidPath.create( @@ -325,8 +322,9 @@ void LocalStore::openDB(State& state, bool create) { SetDllDirectoryW(L""); #endif - if (sqlite3_busy_timeout(db, 60 * 60 * 1000) != SQLITE_OK) + if (sqlite3_busy_timeout(db, 60 * 60 * 1000) != SQLITE_OK) { throwSQLiteError(db, "setting timeout"); + } db.exec("pragma foreign_keys = 1"); @@ -347,8 +345,9 @@ void LocalStore::openDB(State& state, bool create) { { SQLiteStmt stmt; stmt.create(db, "pragma main.journal_mode;"); - if (sqlite3_step(stmt) != SQLITE_ROW) + if (sqlite3_step(stmt) != SQLITE_ROW) { throwSQLiteError(db, "querying journal mode"); + } prevMode = string((const char*)sqlite3_column_text(stmt, 0)); } if (prevMode != mode && @@ -622,7 +621,9 @@ uint64_t LocalStore::addValidPath(State& state, const ValidPathInfo& info, derivations). Note that if this throws an error, then the DB transaction is rolled back, so the path validity registration above is undone. */ - if (checkOutputs) checkDerivationOutputs(info.path, drv); + if (checkOutputs) { + checkDerivationOutputs(info.path, drv); + } for (auto& i : drv.outputs) { state.stmtAddDerivationOutput.use()(id)(i.first)(i.second.path).exec(); @@ -1046,8 +1047,9 @@ Path LocalStore::addToStoreFromDump(const string& dump, const string& name, if (recursive) { StringSource source(dump); restorePath(realPath, source); - } else + } else { writeFile(realPath, dump); + } canonicalisePathMetaData(realPath, -1); @@ -1059,8 +1061,9 @@ Path LocalStore::addToStoreFromDump(const string& dump, const string& name, if (recursive) { hash.first = hashAlgo == htSHA256 ? h : hashString(htSHA256, dump); hash.second = dump.size(); - } else + } else { hash = hashPath(htSHA256, realPath); + } optimisePath(realPath); // FIXME: combine with hashPath() @@ -1297,14 +1300,16 @@ void LocalStore::verifyPath(const Path& path, const PathSet& store, } else { LOG(ERROR) << "path '" << path << "' disappeared, but it still has valid referrers!"; - if (repair) try { + if (repair) { + try { repairPath(path); } catch (Error& e) { LOG(WARNING) << e.msg(); errors = true; } - else + } else { errors = true; + } } return; diff --git a/third_party/nix/src/libstore/optimise-store.cc b/third_party/nix/src/libstore/optimise-store.cc index 215eec94fa2e..d5629c880573 100644 --- a/third_party/nix/src/libstore/optimise-store.cc +++ b/third_party/nix/src/libstore/optimise-store.cc @@ -206,7 +206,9 @@ retry: the store itself (we don't want or need to mess with its permissions). */ bool mustToggle = dirOf(path) != realStoreDir; - if (mustToggle) makeWritable(dirOf(path)); + if (mustToggle) { + makeWritable(dirOf(path)); + } /* When we're done, make the directory read-only again and reset its timestamp back to 0. */ diff --git a/third_party/nix/src/libstore/references.cc b/third_party/nix/src/libstore/references.cc index d8c4620a16c9..63cd284732a6 100644 --- a/third_party/nix/src/libstore/references.cc +++ b/third_party/nix/src/libstore/references.cc @@ -18,7 +18,9 @@ static void search(const unsigned char* s, size_t len, StringSet& hashes, static bool initialised = false; static bool isBase32[256]; if (!initialised) { - for (unsigned int i = 0; i < 256; ++i) isBase32[i] = false; + for (unsigned int i = 0; i < 256; ++i) { + isBase32[i] = false; + } for (unsigned int i = 0; i < base32Chars.size(); ++i) isBase32[(unsigned char)base32Chars[i]] = true; initialised = true; diff --git a/third_party/nix/src/libstore/remote-store.cc b/third_party/nix/src/libstore/remote-store.cc index 39c908afed3a..a4cb98fe7f10 100644 --- a/third_party/nix/src/libstore/remote-store.cc +++ b/third_party/nix/src/libstore/remote-store.cc @@ -33,7 +33,9 @@ Path readStorePath(Store& store, Source& from) { template T readStorePaths(Store& store, Source& from) { T paths = readStrings(from); - for (auto& i : paths) store.assertStorePath(i); + for (auto& i : paths) { + store.assertStorePath(i); + } return paths; } @@ -600,10 +602,11 @@ void RemoteStore::queryMissing(const PathSet& targets, PathSet& willBuild, unsigned long long& narSize) { { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 19) + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 19) { // Don't hold the connection handle in the fallback case // to prevent a deadlock. goto fallback; + } conn->to << wopQueryMissing << targets; conn.processStderr(); willBuild = readStorePaths(*this, conn->from); diff --git a/third_party/nix/src/libstore/sqlite.cc b/third_party/nix/src/libstore/sqlite.cc index 85d159d620ec..81c3ffcd9d05 100644 --- a/third_party/nix/src/libstore/sqlite.cc +++ b/third_party/nix/src/libstore/sqlite.cc @@ -14,15 +14,19 @@ namespace nix { int exterr = sqlite3_extended_errcode(db); auto path = sqlite3_db_filename(db, nullptr); - if (!path) path = "(in-memory)"; + if (!path) { + path = "(in-memory)"; + } if (err == SQLITE_BUSY || err == SQLITE_PROTOCOL) { throw SQLiteBusy( err == SQLITE_PROTOCOL ? fmt("SQLite database '%s' is busy (SQLITE_PROTOCOL)", path) : fmt("SQLite database '%s' is busy", path)); - } else - throw SQLiteError("%s: %s (in '%s')", fs.s, sqlite3_errstr(exterr), path); + } else { + throw + } + SQLiteError("%s: %s (in '%s')", fs.s, sqlite3_errstr(exterr), path); } SQLite::SQLite(const Path& path) { @@ -34,8 +38,9 @@ SQLite::SQLite(const Path& path) { SQLite::~SQLite() { try { - if (db && sqlite3_close(db) != SQLITE_OK) + if (db && sqlite3_close(db) != SQLITE_OK) { throwSQLiteError(db, "closing database"); + } } catch (...) { ignoreException(); } @@ -81,8 +86,9 @@ SQLiteStmt::Use& SQLiteStmt::Use::operator()(const std::string& value, if (sqlite3_bind_text(stmt, curArg++, value.c_str(), -1, SQLITE_TRANSIENT) != SQLITE_OK) throwSQLiteError(stmt.db, "binding argument"); - } else + } else { bind(); + } return *this; } @@ -90,14 +96,16 @@ SQLiteStmt::Use& SQLiteStmt::Use::operator()(int64_t value, bool notNull) { if (notNull) { if (sqlite3_bind_int64(stmt, curArg++, value) != SQLITE_OK) throwSQLiteError(stmt.db, "binding argument"); - } else + } else { bind(); + } return *this; } SQLiteStmt::Use& SQLiteStmt::Use::bind() { - if (sqlite3_bind_null(stmt, curArg++) != SQLITE_OK) + if (sqlite3_bind_null(stmt, curArg++) != SQLITE_OK) { throwSQLiteError(stmt.db, "binding argument"); + } return *this; } @@ -134,21 +142,24 @@ bool SQLiteStmt::Use::isNull(int col) { SQLiteTxn::SQLiteTxn(sqlite3* db) { this->db = db; - if (sqlite3_exec(db, "begin;", 0, 0, 0) != SQLITE_OK) + if (sqlite3_exec(db, "begin;", 0, 0, 0) != SQLITE_OK) { throwSQLiteError(db, "starting transaction"); + } active = true; } void SQLiteTxn::commit() { - if (sqlite3_exec(db, "commit;", 0, 0, 0) != SQLITE_OK) + if (sqlite3_exec(db, "commit;", 0, 0, 0) != SQLITE_OK) { throwSQLiteError(db, "committing transaction"); + } active = false; } SQLiteTxn::~SQLiteTxn() { try { - if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK) + if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK) { throwSQLiteError(db, "aborting transaction"); + } } catch (...) { ignoreException(); } diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc index 2ea478c1bddf..736970e8db99 100644 --- a/third_party/nix/src/libstore/store-api.cc +++ b/third_party/nix/src/libstore/store-api.cc @@ -42,7 +42,9 @@ Path Store::toStorePath(const Path& path) const { Path Store::followLinksToStore(const Path& _path) const { Path path = absPath(_path); while (!isInStore(path)) { - if (!isLink(path)) break; + if (!isLink(path)) { + break; + } string target = readLink(path); path = absPath(target, dirOf(path)); } @@ -828,10 +830,11 @@ StoreType getStoreType(const std::string& uri, const std::string& stateDir) { } else if (uri == "" || uri == "auto") { if (access(stateDir.c_str(), R_OK | W_OK) == 0) return tLocal; - else if (pathExists(settings.nixDaemonSocketFile)) + else if (pathExists(settings.nixDaemonSocketFile)) { return tDaemon; - else + } else { return tLocal; + } } else { return tOther; } -- cgit 1.4.1