libutil: Get rid of restartableSourceFromFactory

Instead we can just seek back in the file - duh. Also this makes use
of the anonymous temp file facility, since that is much safer (no need
window where the we don't have an open file descriptor for it).

Author: xokdvium

src/libstore/binary-cache-store.cc

@@ -79,8 +79,8 @@ std::optional<std::string> BinaryCacheStore::getNixCacheInfo()
 void BinaryCacheStore::upsertFile(
     const std::string & path, std::string && data, const std::string & mimeType, uint64_t sizeHint)
 {
-    auto source = restartableSourceFromFactory([data = std::move(data)]() { return make_unique<StringSource>(data); });
-    upsertFile(path, *source, mimeType, sizeHint);
+    StringSource source{data};
+    upsertFile(path, source, mimeType, sizeHint);
 }
 
 void BinaryCacheStore::getFile(const std::string & path, Callback<std::optional<std::string>> callback) noexcept
@@ -140,9 +140,7 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
 ref<const ValidPathInfo> BinaryCacheStore::addToStoreCommon(
     Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, std::function<ValidPathInfo(HashResult)> mkInfo)
 {
-    auto [fdTemp, fnTemp] = createTempFile();
-
-    AutoDelete autoDelete(fnTemp);
+    auto fdTemp = createAnonymousTempFile();
 
     auto now1 = std::chrono::steady_clock::now();
 
@@ -272,19 +270,10 @@ ref<const ValidPathInfo> BinaryCacheStore::addToStoreCommon(
 
     /* Atomically write the NAR file. */
     if (repair || !fileExists(narInfo->url)) {
-        auto source = restartableSourceFromFactory([fnTemp]() {
-            struct AutoCloseFDSource : AutoCloseFD, FdSource
-            {
-                AutoCloseFDSource(AutoCloseFD fd)
-                    : AutoCloseFD(std::move(fd))
-                    , FdSource(get())
-                {
-                }
-            };
-            return std::make_unique<AutoCloseFDSource>(toDescriptor(open(fnTemp.c_str(), O_RDONLY)));
-        });
+        FdSource source{fdTemp.get()};
+        source.restart(); /* Seek back to the start of the file. */
         stats.narWrite++;
-        upsertFile(narInfo->url, *source, "application/x-nix-nar", narInfo->fileSize);
+        upsertFile(narInfo->url, source, "application/x-nix-nar", narInfo->fileSize);
     } else
         stats.narWriteAverted++;
 

src/libutil-tests/file-system.cc

@@ -398,4 +398,20 @@ TEST(createAnonymousTempFile, works)
     EXPECT_EQ(source.drain(), "testtest");
 }
 
+/* ----------------------------------------------------------------------------
+ * FdSource
+ * --------------------------------------------------------------------------*/
+
+TEST(FdSource, restartWorks)
+{
+    auto fd = createAnonymousTempFile();
+    writeFull(fd.get(), "hello world");
+    lseek(fd.get(), 0, SEEK_SET);
+    FdSource source{fd.get()};
+    EXPECT_EQ(source.drain(), "hello world");
+    source.restart();
+    EXPECT_EQ(source.drain(), "hello world");
+    EXPECT_EQ(source.drain(), "");
+}
+
 } // namespace nix

src/libutil/include/nix/util/serialise.hh

@@ -105,7 +105,7 @@ struct Source
  * A buffered abstract source. Warning: a BufferedSource should not be
  * used from multiple threads concurrently.
  */
-struct BufferedSource : Source
+struct BufferedSource : virtual Source
 {
     size_t bufSize, bufPosIn, bufPosOut;
     std::unique_ptr<char[]> buffer;
@@ -132,6 +132,14 @@ protected:
     virtual size_t readUnbuffered(char * data, size_t len) = 0;
 };
 
+/**
+ * Source type that can be restarted.
+ */
+struct RestartableSource : virtual Source
+{
+    virtual void restart() = 0;
+};
+
 /**
  * A sink that writes data to a file descriptor.
  */
@@ -174,7 +182,7 @@ private:
 /**
  * A source that reads data from a file descriptor.
  */
-struct FdSource : BufferedSource
+struct FdSource : BufferedSource, RestartableSource
 {
     Descriptor fd;
     size_t read = 0;
@@ -196,6 +204,7 @@ struct FdSource : BufferedSource
     FdSource & operator=(FdSource && s) = default;
 
     bool good() override;
+    void restart() override;
 
     /**
      * Return true if the buffer is not empty after a non-blocking
@@ -230,14 +239,6 @@ struct StringSink : Sink
     void operator()(std::string_view data) override;
 };
 
-/**
- * Source type that can be restarted.
- */
-struct RestartableSource : Source
-{
-    virtual void restart() = 0;
-};
-
 /**
  * A source that reads data from a string.
  */
@@ -316,15 +317,6 @@ public:
     }
 };
 
-/**
- * Create a restartable Source from a factory function.
- *
- * @param factory Factory function that returns a fresh instance of the Source. Gets
- * called for each source restart.
- * @pre factory must return an equivalent source for each invocation.
- */
-std::unique_ptr<RestartableSource> restartableSourceFromFactory(std::function<std::unique_ptr<Source>()> factory);
-
 /**
  * A sink that writes all incoming data to two other sinks.
  */

src/libutil/serialise.cc

@@ -201,6 +201,16 @@ bool FdSource::hasData()
     }
 }
 
+void FdSource::restart()
+{
+    if (!isSeekable)
+        throw Error("can't seek to the start of a file");
+    buffer.reset();
+    read = bufPosOut = bufPosOut = 0;
+    if (lseek(fd, 0, SEEK_SET) == -1)
+        throw SysError("seeking to the start of a file");
+}
+
 void FdSource::skip(size_t len)
 {
     /* Discard data in the buffer. */
@@ -527,41 +537,4 @@ size_t ChainSource::read(char * data, size_t len)
     }
 }
 
-std::unique_ptr<RestartableSource> restartableSourceFromFactory(std::function<std::unique_ptr<Source>()> factory)
-{
-    struct RestartableSourceImpl : RestartableSource
-    {
-        RestartableSourceImpl(decltype(factory) factory_)
-            : factory_(std::move(factory_))
-            , impl(this->factory_())
-        {
-        }
-
-        decltype(factory) factory_;
-        std::unique_ptr<Source> impl = factory_();
-
-        size_t read(char * data, size_t len) override
-        {
-            return impl->read(data, len);
-        }
-
-        bool good() override
-        {
-            return impl->good();
-        }
-
-        void skip(size_t len) override
-        {
-            return impl->skip(len);
-        }
-
-        void restart() override
-        {
-            impl = factory_();
-        }
-    };
-
-    return std::make_unique<RestartableSourceImpl>(std::move(factory));
-}
-
 } // namespace nix