From 77ebbc9f5425d84e1fed71c8b060c7cc1a57ae79 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 4 Oct 2021 14:29:42 +0200 Subject: [PATCH] Add a test for RefScanSink and clean up the code Issue #5322. --- Makefile | 1 + src/libstore/references.cc | 56 +++++++++++++++----------------- src/libstore/references.hh | 18 ++++++++++ src/libstore/tests/local.mk | 15 +++++++++ src/libstore/tests/references.cc | 45 +++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 src/libstore/tests/local.mk create mode 100644 src/libstore/tests/references.cc diff --git a/Makefile b/Makefile index c7d8967c8..5040d2884 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ makefiles = \ src/libutil/local.mk \ src/libutil/tests/local.mk \ src/libstore/local.mk \ + src/libstore/tests/local.mk \ src/libfetchers/local.mk \ src/libmain/local.mk \ src/libexpr/local.mk \ diff --git a/src/libstore/references.cc b/src/libstore/references.cc index eb70844a2..c369b14ac 100644 --- a/src/libstore/references.cc +++ b/src/libstore/references.cc @@ -11,11 +11,13 @@ namespace nix { -static unsigned int refLength = 32; /* characters */ +static size_t refLength = 32; /* characters */ -static void search(const unsigned char * s, size_t len, - StringSet & hashes, StringSet & seen) +static void search( + std::string_view s, + StringSet & hashes, + StringSet & seen) { static std::once_flag initialised; static bool isBase32[256]; @@ -25,7 +27,7 @@ static void search(const unsigned char * s, size_t len, isBase32[(unsigned char) base32Chars[i]] = true; }); - for (size_t i = 0; i + refLength <= len; ) { + for (size_t i = 0; i + refLength <= s.size(); ) { int j; bool match = true; for (j = refLength - 1; j >= 0; --j) @@ -35,7 +37,7 @@ static void search(const unsigned char * s, size_t len, break; } if (!match) continue; - string ref((const char *) s + i, refLength); + std::string ref(s.substr(i, refLength)); if (hashes.erase(ref)) { debug(format("found reference to '%1%' at offset '%2%'") % ref % i); @@ -46,30 +48,23 @@ static void search(const unsigned char * s, size_t len, } -struct RefScanSink : Sink +void RefScanSink::operator () (std::string_view data) { - StringSet hashes; - StringSet seen; + /* It's possible that a reference spans the previous and current + fragment, so search in the concatenation of the tail of the + previous fragment and the start of the current fragment. */ + auto s = tail; + s.append(data.data(), refLength); + search(s, hashes, seen); - string tail; + search(data, hashes, seen); - RefScanSink() { } - - void operator () (std::string_view data) override - { - /* It's possible that a reference spans the previous and current - fragment, so search in the concatenation of the tail of the - previous fragment and the start of the current fragment. */ - string s = tail + std::string(data, 0, refLength); - search((const unsigned char *) s.data(), s.size(), hashes, seen); - - search((const unsigned char *) data.data(), data.size(), hashes, seen); - - size_t tailLen = data.size() <= refLength ? data.size() : refLength; - tail = std::string(tail, tail.size() < refLength - tailLen ? 0 : tail.size() - (refLength - tailLen)); - tail.append({data.data() + data.size() - tailLen, tailLen}); - } -}; + auto tailLen = std::min(data.size(), refLength); + auto rest = refLength - tailLen; + if (rest < tail.size()) + tail = tail.substr(tail.size() - rest); + tail.append(data.data() + data.size() - tailLen, tailLen); +} std::pair scanForReferences( @@ -87,23 +82,24 @@ StorePathSet scanForReferences( const Path & path, const StorePathSet & refs) { - RefScanSink refsSink; - TeeSink sink { refsSink, toTee }; + StringSet hashes; std::map backMap; for (auto & i : refs) { std::string hashPart(i.hashPart()); auto inserted = backMap.emplace(hashPart, i).second; assert(inserted); - refsSink.hashes.insert(hashPart); + hashes.insert(hashPart); } /* Look for the hashes in the NAR dump of the path. */ + RefScanSink refsSink(std::move(hashes)); + TeeSink sink { refsSink, toTee }; dumpPath(path, sink); /* Map the hashes found back to their store paths. */ StorePathSet found; - for (auto & i : refsSink.seen) { + for (auto & i : refsSink.getResult()) { auto j = backMap.find(i); assert(j != backMap.end()); found.insert(j->second); diff --git a/src/libstore/references.hh b/src/libstore/references.hh index 73e674109..a6119c861 100644 --- a/src/libstore/references.hh +++ b/src/libstore/references.hh @@ -9,6 +9,24 @@ std::pair scanForReferences(const Path & path, const S StorePathSet scanForReferences(Sink & toTee, const Path & path, const StorePathSet & refs); +class RefScanSink : public Sink +{ + StringSet hashes; + StringSet seen; + + std::string tail; + +public: + + RefScanSink(StringSet && hashes) : hashes(hashes) + { } + + StringSet & getResult() + { return seen; } + + void operator () (std::string_view data) override; +}; + struct RewritingSink : Sink { std::string from, to, prev; diff --git a/src/libstore/tests/local.mk b/src/libstore/tests/local.mk new file mode 100644 index 000000000..6ae9a0285 --- /dev/null +++ b/src/libstore/tests/local.mk @@ -0,0 +1,15 @@ +check: libstore-tests_RUN + +programs += libstore-tests + +libstore-tests_DIR := $(d) + +libstore-tests_INSTALL_DIR := + +libstore-tests_SOURCES := $(wildcard $(d)/*.cc) + +libstore-tests_CXXFLAGS += -I src/libstore -I src/libutil + +libstore-tests_LIBS = libstore + +libstore-tests_LDFLAGS := $(GTEST_LIBS) diff --git a/src/libstore/tests/references.cc b/src/libstore/tests/references.cc new file mode 100644 index 000000000..d91d1cedd --- /dev/null +++ b/src/libstore/tests/references.cc @@ -0,0 +1,45 @@ +#include "references.hh" + +#include + +namespace nix { + +TEST(references, scan) +{ + std::string hash1 = "dc04vv14dak1c1r48qa0m23vr9jy8sm0"; + std::string hash2 = "zc842j0rz61mjsp3h3wp5ly71ak6qgdn"; + + { + RefScanSink scanner(StringSet{hash1}); + auto s = "foobar"; + scanner(s); + ASSERT_EQ(scanner.getResult(), StringSet{}); + } + + { + RefScanSink scanner(StringSet{hash1}); + auto s = "foobar" + hash1 + "xyzzy"; + scanner(s); + ASSERT_EQ(scanner.getResult(), StringSet{hash1}); + } + + { + RefScanSink scanner(StringSet{hash1, hash2}); + auto s = "foobar" + hash1 + "xyzzy" + hash2; + scanner(((std::string_view) s).substr(0, 10)); + scanner(((std::string_view) s).substr(10, 5)); + scanner(((std::string_view) s).substr(15, 5)); + scanner(((std::string_view) s).substr(20)); + ASSERT_EQ(scanner.getResult(), StringSet({hash1, hash2})); + } + + { + RefScanSink scanner(StringSet{hash1, hash2}); + auto s = "foobar" + hash1 + "xyzzy" + hash2; + for (auto & i : s) + scanner(std::string(1, i)); + ASSERT_EQ(scanner.getResult(), StringSet({hash1, hash2})); + } +} + +}