From 949a5615405d24294abab4d3dfb05958d87c32d6 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 5 Apr 2025 21:57:20 +0200 Subject: [PATCH] libutil: explicitly mark ref(shared_ptr) as unsafe we'll also assert instead of throwing an exception because that just seems more useful. this should never *ever* happen, and it is on the callers of such conversions to ensure that the conversion *is* safe. Change-Id: Ib0696af4f037046f2d45bf5b1b255393ea9b5f05 --- lix/legacy/nix-store.cc | 8 ++++---- lix/libcmd/command.cc | 2 +- lix/libexpr/eval-cache.cc | 4 ++-- lix/libexpr/eval.cc | 2 +- lix/libexpr/parser/parser-impl1.inc.cc | 2 +- lix/libstore/build/local-derivation-goal.cc | 10 +++++++--- lix/libstore/local-fs-store.cc | 2 +- lix/libstore/store-api.cc | 6 +++--- lix/libutil/pool.hh | 2 +- lix/libutil/ref.hh | 17 +++++++++++------ perl/lib/Nix/Store.xs | 4 ++-- 11 files changed, 34 insertions(+), 25 deletions(-) diff --git a/lix/legacy/nix-store.cc b/lix/legacy/nix-store.cc index dcc5e2be2..370894ffe 100644 --- a/lix/legacy/nix-store.cc +++ b/lix/legacy/nix-store.cc @@ -47,7 +47,7 @@ ref ensureLocalStore() { auto store2 = std::dynamic_pointer_cast(store); if (!store2) throw Error("you don't have sufficient rights to use this command"); - return ref(store2); + return ref::unsafeFromPtr(store2); } @@ -159,7 +159,7 @@ static void opRealise(AsyncIoRoot & aio, Strings opFlags, Strings opArgs) if (settings.printMissing) { aio.blockOn(printMissing( - ref(store), willBuild, willSubstitute, unknown, downloadSize, narSize + ref::unsafeFromPtr(store), willBuild, willSubstitute, unknown, downloadSize, narSize )); } @@ -451,7 +451,7 @@ static void opQuery(AsyncIoRoot & aio, Strings opFlags, Strings opArgs) for (auto & i : opArgs) for (auto & j : aio.blockOn(maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise))) roots.insert(j); - aio.blockOn(printDotGraph(ref(store), std::move(roots))); + aio.blockOn(printDotGraph(ref::unsafeFromPtr(store), std::move(roots))); break; } @@ -460,7 +460,7 @@ static void opQuery(AsyncIoRoot & aio, Strings opFlags, Strings opArgs) for (auto & i : opArgs) for (auto & j : aio.blockOn(maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise))) roots.insert(j); - aio.blockOn(printGraphML(ref(store), std::move(roots))); + aio.blockOn(printGraphML(ref::unsafeFromPtr(store), std::move(roots))); break; } diff --git a/lix/libcmd/command.cc b/lix/libcmd/command.cc index c5093614e..7f77c19be 100644 --- a/lix/libcmd/command.cc +++ b/lix/libcmd/command.cc @@ -118,7 +118,7 @@ ref EvalCommand::getEvaluator() evalState->repair = repair; } - return ref(evalState); + return ref::unsafeFromPtr(evalState); } MixOperateOnOptions::MixOperateOnOptions() diff --git a/lix/libexpr/eval-cache.cc b/lix/libexpr/eval-cache.cc index ad6e0a069..e4e5dabee 100644 --- a/lix/libexpr/eval-cache.cc +++ b/lix/libexpr/eval-cache.cc @@ -526,7 +526,7 @@ ref AttrCursor::getAttr(EvalState & state, const std::string & name) auto p = maybeGetAttr(state, name); if (!p) throw Error("attribute '%s' does not exist", getAttrPathStr(state, name)); - return ref(p); + return ref::unsafeFromPtr(p); } OrSuggestions> AttrCursor::findAlongAttrPath(EvalState & state, const std::vector & attrPath) @@ -540,7 +540,7 @@ OrSuggestions> AttrCursor::findAlongAttrPath(EvalState & state, } res = child; } - return ref(res); + return ref::unsafeFromPtr(res); } std::string AttrCursor::getString(EvalState & state) diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 490759924..3e3490ec4 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -333,7 +333,7 @@ Evaluator::Evaluator( , builtins(mem, symbols, paths.searchPath(), store->config().storeDir) , repair(NoRepair) , store(store) - , buildStore(buildStore ? ref(buildStore) : store) + , buildStore(buildStore ? ref::unsafeFromPtr(buildStore) : store) , debug{ debugRepl ? std::make_unique( positions, diff --git a/lix/libexpr/parser/parser-impl1.inc.cc b/lix/libexpr/parser/parser-impl1.inc.cc index 300515dc3..62f12e34e 100644 --- a/lix/libexpr/parser/parser-impl1.inc.cc +++ b/lix/libexpr/parser/parser-impl1.inc.cc @@ -387,7 +387,7 @@ template<> struct BuildAST : change_head { if (s.from != nullptr) { if (!b.attrs.inheritFromExprs) b.attrs.inheritFromExprs = std::make_unique>>(); - auto fromExpr = ref(std::move(s.from)); + auto fromExpr = ref::unsafeFromPtr(std::move(s.from)); b.attrs.inheritFromExprs->push_back(fromExpr); for (auto & i : s.attrs) { if (attrs.find(i.symbol) != attrs.end()) diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index d1e1f2cfb..709ec8de7 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -1310,9 +1310,13 @@ void LocalDerivationGoal::startDaemon() params["root"] = *optRoot; params["state"] = "/no-such-path"; params["log"] = "/no-such-path"; - auto store = make_ref(params, - ref(std::dynamic_pointer_cast(worker.store.shared_from_this())), - *this); + auto store = make_ref( + params, + ref::unsafeFromPtr( + std::dynamic_pointer_cast(worker.store.shared_from_this()) + ), + *this + ); addedPaths.clear(); diff --git a/lix/libstore/local-fs-store.cc b/lix/libstore/local-fs-store.cc index 62c3035ca..fc56fb6fe 100644 --- a/lix/libstore/local-fs-store.cc +++ b/lix/libstore/local-fs-store.cc @@ -80,7 +80,7 @@ struct LocalStoreAccessor : public FSAccessor ref LocalFSStore::getFSAccessor() { - return make_ref(ref( + return make_ref(ref::unsafeFromPtr( std::dynamic_pointer_cast(shared_from_this()))); } diff --git a/lix/libstore/store-api.cc b/lix/libstore/store-api.cc index 01e410bba..043515563 100644 --- a/lix/libstore/store-api.cc +++ b/lix/libstore/store-api.cc @@ -715,7 +715,7 @@ try { stats.narInfoReadAverted++; if (!res->didExist()) throw InvalidPath("path '%s' does not exist in the store", printStorePath(storePath)); - co_return ref(res->value); + co_return ref::unsafeFromPtr(res->value); } } @@ -730,7 +730,7 @@ try { if (res.first == NarInfoDiskCache::oInvalid) throw InvalidPath("path '%s' does not exist in the store", printStorePath(storePath)); } - co_return ref(res.second); + co_return ref::unsafeFromPtr(res.second); } } @@ -754,7 +754,7 @@ try { throw InvalidPath("path '%s' does not exist in the store", printStorePath(storePath)); } - co_return ref(info); + co_return ref::unsafeFromPtr(info); } catch (...) { co_return result::current_exception(); } diff --git a/lix/libutil/pool.hh b/lix/libutil/pool.hh index c92552f3d..a07ce492b 100644 --- a/lix/libutil/pool.hh +++ b/lix/libutil/pool.hh @@ -132,7 +132,7 @@ public: { auto state_(pool.state.lock()); if (!bad) - state_->idle.push_back(ref(r)); + state_->idle.push_back(ref::unsafeFromPtr(r)); assert(state_->inUse); state_->inUse--; state_->notify(); diff --git a/lix/libutil/ref.hh b/lix/libutil/ref.hh index c65abf376..ab7e60e7b 100644 --- a/lix/libutil/ref.hh +++ b/lix/libutil/ref.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include #include #include #include @@ -20,17 +21,21 @@ private: std::shared_ptr p; + explicit ref(const std::shared_ptr & p) + : p(p) + { + assert(p); + } + public: ref(const ref & r) : p(r.p) { } - explicit ref(const std::shared_ptr & p) - : p(p) + static ref unsafeFromPtr(const std::shared_ptr & p) { - if (!p) - throw std::invalid_argument("null pointer cast to ref"); + return ref(p); } template> T2> @@ -62,7 +67,7 @@ public: std::optional> try_cast() const { if (auto d = std::dynamic_pointer_cast(p)) { - return ref(d); + return ref::unsafeFromPtr(d); } else { return std::nullopt; } @@ -77,7 +82,7 @@ public: template operator ref () const { - return ref((std::shared_ptr) p); + return ref::unsafeFromPtr((std::shared_ptr) p); } ref & operator=(ref const & rhs) = default; diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 1acecc72c..356c44674 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -34,7 +34,7 @@ static AsyncIoRoot & aio() static ref store() { - static std::shared_ptr _store; + static std::optional> _store; if (!_store) { try { initLibStore(); @@ -43,7 +43,7 @@ static ref store() croak("%s", e.what()); } } - return ref(_store); + return *_store; }