refactor: add tr_saveFile() (#2267)

* refactor: add tr_saveFile()

* refactor: add tr_ctorSaveContents()

cli/cli.cc

@@ -243,8 +243,6 @@ int tr_main(int argc, char* argv[])
     tr_torrent* tor = nullptr;
     tr_variant settings;
     char const* configDir;
-    uint8_t* fileContents;
-    size_t fileLength;
 
     tr_formatter_mem_init(MEM_K, MEM_K_STR, MEM_M_STR, MEM_G_STR, MEM_T_STR);
     tr_formatter_size_init(DISK_K, DISK_K_STR, DISK_M_STR, DISK_G_STR, DISK_T_STR);
@@ -304,12 +302,11 @@ int tr_main(int argc, char* argv[])
 
     ctor = tr_ctorNew(h);
 
-    fileContents = tr_loadFile(torrentPath, &fileLength, nullptr);
     tr_ctorSetPaused(ctor, TR_FORCE, false);
 
-    if (fileContents != nullptr)
+    if (tr_sys_path_exists(torrentPath, nullptr))
     {
-        tr_ctorSetMetainfo(ctor, fileContents, fileLength);
+        tr_ctorSetMetainfoFromFile(ctor, torrentPath);
     }
     else if (memcmp(torrentPath, "magnet:?", 8) == 0)
     {
@@ -334,8 +331,6 @@ int tr_main(int argc, char* argv[])
         return EXIT_FAILURE;
     }
 
-    tr_free(fileContents);
-
     tor = tr_torrentNew(ctor, nullptr, nullptr);
     tr_ctorFree(ctor);
 

gtk/Session.cc

@@ -1148,7 +1148,7 @@ void Session::Impl::add_file_async_callback(
         {
             g_message(_("Couldn't read \"%s\""), file->get_parse_name().c_str());
         }
-        else if (tr_ctorSetMetainfo(ctor, (uint8_t const*)contents, length) == 0)
+        else if (tr_ctorSetMetainfo(ctor, contents, length) == 0)
         {
             add_ctor(ctor, do_prompt, do_notify);
         }

libtransmission/torrent-ctor.cc

@@ -12,6 +12,8 @@
 #include <vector>
 
 #include "transmission.h"
+
+#include "error.h"
 #include "file.h"
 #include "magnet-metainfo.h"
 #include "session.h"
@@ -88,12 +90,11 @@ static int parseMetainfoContents(tr_ctor* ctor)
     return ctor->isSet_metainfo ? 0 : EILSEQ;
 }
 
-int tr_ctorSetMetainfo(tr_ctor* ctor, void const* metainfo, size_t len)
+int tr_ctorSetMetainfo(tr_ctor* ctor, char const* metainfo, size_t len)
 {
     clearMetainfo(ctor);
 
-    ctor->contents.resize(len);
-    std::copy_n(static_cast<char const*>(metainfo), len, std::begin(ctor->contents));
+    ctor->contents.assign(metainfo, metainfo + len);
 
     return parseMetainfoContents(ctor);
 }
@@ -115,7 +116,7 @@ int tr_ctorSetMetainfoFromMagnetLink(tr_ctor* ctor, char const* magnet_link)
     mm.toVariant(&tmp);
     auto len = size_t{};
     char* const str = tr_variantToStr(&tmp, TR_VARIANT_FMT_BENC, &len);
-    auto const err = tr_ctorSetMetainfo(ctor, (uint8_t const*)str, len);
+    auto const err = tr_ctorSetMetainfo(ctor, str, len);
     tr_free(str);
     tr_variantFree(&tmp);
 
@@ -164,6 +165,20 @@ int tr_ctorSetMetainfoFromFile(tr_ctor* ctor, char const* filename)
     return 0;
 }
 
+bool tr_ctorSaveContents(tr_ctor const* ctor, char const* filename, tr_error** error)
+{
+    TR_ASSERT(ctor != nullptr);
+    TR_ASSERT(filename != nullptr);
+
+    if (std::empty(ctor->contents))
+    {
+        tr_error_set_literal(error, EINVAL, "torrent ctor has no contents to save");
+        return false;
+    }
+
+    return tr_saveFile(filename, { std::data(ctor->contents), std::size(ctor->contents) }, error);
+}
+
 /***
 ****
 ***/

libtransmission/torrent.cc

@@ -763,17 +763,12 @@ static void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
     /* maybe save our own copy of the metainfo */
     if (tr_ctorGetSave(ctor))
     {
-        tr_variant const* val = nullptr;
-        if (tr_ctorGetMetainfo(ctor, &val))
+        tr_error* error = nullptr;
+        if (!tr_ctorSaveContents(ctor, tor->info.torrent, &error))
         {
-            char const* path = tor->info.torrent;
-            int const err = tr_variantToFile(val, TR_VARIANT_FMT_BENC, path);
-
-            if (err != 0)
-            {
-                tr_torrentSetLocalError(tor, "Unable to save torrent file: %s", tr_strerror(err));
-            }
+            tr_torrentSetLocalError(tor, "Unable to save torrent file: %s (%d)", error->message, error->code);
         }
+        tr_error_clear(&error);
     }
 
     tor->tiers = tr_announcerAddTorrent(tor, onTrackerResponse, nullptr);

libtransmission/torrent.h

@@ -52,6 +52,14 @@ void tr_ctorInitTorrentPriorities(tr_ctor const* ctor, tr_torrent* tor);
 
 void tr_ctorInitTorrentWanted(tr_ctor const* ctor, tr_torrent* tor);
 
+bool tr_ctorSaveContents(tr_ctor const* ctor, char const* filename, tr_error** error);
+
+bool tr_ctorGetMetainfo(tr_ctor const* ctor, tr_variant const** setme);
+
+tr_session* tr_ctorGetSession(tr_ctor const* ctor);
+
+bool tr_ctorGetIncompleteDir(tr_ctor const* ctor, char const** setmeIncompleteDir);
+
 /**
 ***
 **/
@@ -240,7 +248,7 @@ public:
 
     /// WANTED
 
-    bool pieceIsWanted(tr_piece_index_t piece) const final
+    bool pieceIsWanted(tr_piece_index_t piece) const final override
     {
         return files_wanted_.pieceWanted(piece);
     }

libtransmission/transmission.h

@@ -860,7 +860,7 @@ void tr_ctorSetDeleteSource(tr_ctor* ctor, bool doDelete);
 int tr_ctorSetMetainfoFromMagnetLink(tr_ctor* ctor, char const* magnet);
 
 /** @brief Set the constructor's metainfo from a raw benc already in memory */
-int tr_ctorSetMetainfo(tr_ctor* ctor, void const* metainfo, size_t len);
+int tr_ctorSetMetainfo(tr_ctor* ctor, char const* metainfo, size_t len);
 
 /** @brief Set the constructor's metainfo from a local .torrent file */
 int tr_ctorSetMetainfoFromFile(tr_ctor* ctor, char const* filename);
@@ -902,18 +902,9 @@ bool tr_ctorGetPaused(tr_ctor const* ctor, tr_ctorMode mode, bool* setmeIsPaused
 /** @brief Get the download path from this peer constructor */
 bool tr_ctorGetDownloadDir(tr_ctor const* ctor, tr_ctorMode mode, char const** setmeDownloadDir);
 
-/** @brief Get the incomplete directory from this peer constructor */
-bool tr_ctorGetIncompleteDir(tr_ctor const* ctor, char const** setmeIncompleteDir);
-
-/** @brief Get the metainfo from this peer constructor */
-bool tr_ctorGetMetainfo(tr_ctor const* ctor, struct tr_variant const** setme);
-
 /** @brief Get the "delete .torrent file" flag from this peer constructor */
 bool tr_ctorGetDeleteSource(tr_ctor const* ctor, bool* setmeDoDelete);
 
-/** @brief Get the tr_session poiner from this peer constructor */
-tr_session* tr_ctorGetSession(tr_ctor const* ctor);
-
 /** @brief Get the .torrent file that this ctor's metainfo came from,
            or nullptr if tr_ctorSetMetainfoFromFile() wasn't used */
 char const* tr_ctorGetSourceFile(tr_ctor const* ctor);

libtransmission/utils.cc

@@ -378,6 +378,51 @@ bool tr_loadFile(std::vector<char>& setme, char const* path, tr_error** error)
     return true;
 }
 
+bool tr_saveFile(char const* filename_in, std::string_view contents, tr_error** error)
+{
+    auto filename = std::string{ filename_in };
+
+    // follow symlinks to find the "real" file, to make sure the temporary
+    // we build with tr_sys_file_open_temp() is created on the right partition
+    char* real_filename = tr_sys_path_resolve(filename.c_str(), nullptr);
+    if (real_filename != nullptr)
+    {
+        filename = real_filename;
+        tr_free(real_filename);
+    }
+
+    // Write it to a temp file first.
+    // This is a safeguard against edge cases, e.g. disk full, crash while writing, etc.
+    auto tmp = tr_strvJoin(filename, ".tmp.XXXXXX"sv);
+    auto const fd = tr_sys_file_open_temp(std::data(tmp), error);
+    if (fd == TR_BAD_SYS_FILE)
+    {
+        return false;
+    }
+
+    // Save the contents. This might take >1 pass.
+    auto ok = bool{ true };
+    while (!std::empty(contents))
+    {
+        auto n_written = uint64_t{};
+        if (!tr_sys_file_write(fd, std::data(contents), std::size(contents), &n_written, error))
+        {
+            ok = false;
+            break;
+        }
+        contents.remove_prefix(n_written);
+    }
+
+    // If we saved it to disk successfully, move it from '.tmp' to the correct filename
+    if (!tr_sys_file_close(fd, error) || !ok || !tr_sys_path_rename(tmp.c_str(), filename.c_str(), error))
+    {
+        return false;
+    }
+
+    tr_logAddInfo(_("Saved \"%s\""), filename.c_str());
+    return true;
+}
+
 char* tr_buildPath(char const* first_element, ...)
 {
 

libtransmission/utils.h

@@ -81,6 +81,8 @@ uint8_t* tr_loadFile(char const* filename, size_t* size, struct tr_error** error
 
 bool tr_loadFile(std::vector<char>& setme, char const* filename, tr_error** error = nullptr);
 
+bool tr_saveFile(char const* filename_in, std::string_view contents, tr_error** error = nullptr);
+
 /** @brief build a filename from a series of elements using the
            platform's correct directory separator. */
 char* tr_buildPath(char const* first_element, ...) TR_GNUC_NULL_TERMINATED TR_GNUC_MALLOC;

libtransmission/variant.cc

@@ -1205,87 +1205,18 @@ char* tr_variantToStr(tr_variant const* v, tr_variant_fmt fmt, size_t* len)
     return evbuffer_free_to_str(buf, len);
 }
 
-static int writeVariantToFd(tr_variant const* v, tr_variant_fmt fmt, tr_sys_file_t fd, tr_error** error)
-{
-    int err = 0;
-    struct evbuffer* buf = tr_variantToBuf(v, fmt);
-    char const* walk = (char const*)evbuffer_pullup(buf, -1);
-    uint64_t nleft = evbuffer_get_length(buf);
-
-    while (nleft > 0)
-    {
-        uint64_t n = 0;
-
-        tr_error* tmperr = nullptr;
-        if (!tr_sys_file_write(fd, walk, nleft, &n, &tmperr))
-        {
-            err = tmperr->code;
-            tr_error_propagate(error, &tmperr);
-            break;
-        }
-
-        nleft -= n;
-        walk += n;
-    }
-
-    evbuffer_free(buf);
-    return err;
-}
-
 int tr_variantToFile(tr_variant const* v, tr_variant_fmt fmt, char const* filename)
 {
-    /* follow symlinks to find the "real" file, to make sure the temporary
-     * we build with tr_sys_file_open_temp() is created on the right partition */
-    char* real_filename = tr_sys_path_resolve(filename, nullptr);
-    if (real_filename != nullptr)
-    {
-        filename = real_filename;
-    }
-
-    /* if the file already exists, try to move it out of the way & keep it as a backup */
-    char* const tmp = tr_strdup_printf("%s.tmp.XXXXXX", filename);
+    auto contents_len = size_t{};
+    auto const* contents = tr_variantToStr(v, fmt, &contents_len);
     tr_error* error = nullptr;
-    tr_sys_file_t const fd = tr_sys_file_open_temp(tmp, &error);
-
-    int err = 0;
-    if (fd != TR_BAD_SYS_FILE)
+    auto const saved = tr_saveFile(filename, { contents, contents_len }, &error);
+    if (error != nullptr)
     {
-        err = writeVariantToFd(v, fmt, fd, &error);
-        tr_sys_file_close(fd, nullptr);
-
-        if (err)
-        {
-            tr_logAddError(_("Couldn't save temporary file \"%1$s\": %2$s"), tmp, error->message);
-            tr_sys_path_remove(tmp, nullptr);
-            tr_error_free(error);
-        }
-        else
-        {
-            tr_error_clear(&error);
-
-            if (tr_sys_path_rename(tmp, filename, &error))
-            {
-                tr_logAddInfo(_("Saved \"%s\""), filename);
-            }
-            else
-            {
-                err = error->code;
-                tr_logAddError(_("Couldn't save file \"%1$s\": %2$s"), filename, error->message);
-                tr_sys_path_remove(tmp, nullptr);
-                tr_error_free(error);
-            }
-        }
+        tr_logAddError(_("Error saving \"%s\": %s (%d)"), filename, error->message, error->code);
+        tr_error_clear(&error);
     }
-    else
-    {
-        err = error->code;
-        tr_logAddError(_("Couldn't save temporary file \"%1$s\": %2$s"), tmp, error->message);
-        tr_error_free(error);
-    }
-
-    tr_free(tmp);
-    tr_free(real_filename);
-    return err;
+    return saved ? 0 : -1;
 }
 
 /***

tests/libtransmission/metainfo-test.cc

@@ -8,7 +8,9 @@
 
 #include "transmission.h"
 
+#include "error.h"
 #include "metainfo.h"
+#include "torrent.h"
 #include "utils.h"
 
 #include "gtest/gtest.h"
@@ -176,13 +178,42 @@ TEST(Metainfo, sanitize)
 
 TEST(Metainfo, AndroidTorrent)
 {
-    auto* ctor = tr_ctorNew(nullptr);
+    auto const filename = tr_strvJoin(LIBTRANSMISSION_TEST_ASSETS_DIR, "/Android-x86 8.1 r6 iso.torrent"sv);
 
-    auto filename = std::string{ LIBTRANSMISSION_TEST_ASSETS_DIR };
-    filename += '/'; // FIXME
-    filename += "Android-x86 8.1 r6 iso.torrent";
+    auto* ctor = tr_ctorNew(nullptr);
     auto const err = tr_ctorSetMetainfoFromFile(ctor, filename.c_str());
     EXPECT_EQ(0, err);
-
+    tr_ctorFree(ctor);
+}
+
+TEST(Metainfo, ctorSaveContents)
+{
+    auto const src_filename = tr_strvJoin(LIBTRANSMISSION_TEST_ASSETS_DIR, "/Android-x86 8.1 r6 iso.torrent"sv);
+    auto const tgt_filename = tr_strvJoin(::testing::TempDir(), "save-contents-test.torrent");
+
+    // try saving without passing any metainfo.
+    auto* ctor = tr_ctorNew(nullptr);
+    tr_error* error = nullptr;
+    EXPECT_FALSE(tr_ctorSaveContents(ctor, tgt_filename.c_str(), &error));
+    ASSERT_NE(nullptr, error);
+    EXPECT_EQ(EINVAL, error->code);
+    tr_error_clear(&error);
+
+    // now try saving _with_ metainfo
+    EXPECT_EQ(0, tr_ctorSetMetainfoFromFile(ctor, src_filename.c_str()));
+    EXPECT_TRUE(tr_ctorSaveContents(ctor, tgt_filename.c_str(), &error));
+    EXPECT_EQ(nullptr, error);
+
+    // the saved contents should match the source file's contents
+    auto src_contents = std::vector<char>{};
+    EXPECT_TRUE(tr_loadFile(src_contents, src_filename.c_str(), &error));
+    auto tgt_contents = std::vector<char>{};
+    EXPECT_TRUE(tr_loadFile(tgt_contents, tgt_filename.c_str(), &error));
+    EXPECT_EQ(src_contents, tgt_contents);
+
+    // cleanup
+    EXPECT_TRUE(tr_sys_path_remove(tgt_filename.c_str(), &error));
+    EXPECT_EQ(nullptr, error);
+    tr_error_clear(&error);
     tr_ctorFree(ctor);
 }

tests/libtransmission/rename-test.cc

@@ -73,7 +73,7 @@ protected:
         auto* metainfo = static_cast<char*>(tr_base64_decode_str(metainfo_base64, &metainfo_len));
         EXPECT_NE(nullptr, metainfo);
         EXPECT_LT(size_t(0), metainfo_len);
-        tr_ctorSetMetainfo(ctor, reinterpret_cast<uint8_t const*>(metainfo), metainfo_len);
+        tr_ctorSetMetainfo(ctor, metainfo, metainfo_len);
         tr_ctorSetPaused(ctor, TR_FORCE, true);
 
         // create the torrent

tests/libtransmission/test-fixtures.h

@@ -374,11 +374,11 @@ protected:
 
         // create the torrent ctor
         auto metainfo_len = size_t{};
-        auto* metainfo = tr_base64_decode_str(metainfo_base64, &metainfo_len);
+        auto* const metainfo = tr_base64_decode_str(metainfo_base64, &metainfo_len);
         EXPECT_NE(nullptr, metainfo);
         EXPECT_LT(size_t{ 0 }, metainfo_len);
         auto* ctor = tr_ctorNew(session_);
-        tr_ctorSetMetainfo(ctor, reinterpret_cast<uint8_t*>(metainfo), metainfo_len);
+        tr_ctorSetMetainfo(ctor, static_cast<char const*>(metainfo), metainfo_len);
         tr_ctorSetPaused(ctor, TR_FORCE, true);
         tr_free(metainfo);
 

tests/libtransmission/utils-test.cc

@@ -484,3 +484,31 @@ TEST_F(UtilsTest, mimeTypes)
     EXPECT_EQ("video/x-msvideo"sv, tr_get_mime_type_for_filename("/path/to/FILENAME.AVI"sv));
     EXPECT_EQ("application/octet-stream"sv, tr_get_mime_type_for_filename("music.ajoijfeisfe"sv));
 }
+
+TEST_F(UtilsTest, saveFile)
+{
+    // save a file to GoogleTest's temp dir
+    auto filename = tr_strvJoin(::testing::TempDir(), "filename.txt");
+    auto contents = "these are the contents"sv;
+    tr_error* error = nullptr;
+    EXPECT_TRUE(tr_saveFile(filename.c_str(), contents, &error));
+    EXPECT_EQ(nullptr, error);
+
+    // now read the file back in and confirm the contents are the same
+    auto buf = std::vector<char>{};
+    EXPECT_TRUE(tr_loadFile(buf, filename.c_str(), &error));
+    EXPECT_EQ(nullptr, error);
+    auto sv = std::string_view{ std::data(buf), std::size(buf) };
+    EXPECT_EQ(contents, sv);
+
+    // remove the tempfile
+    EXPECT_TRUE(tr_sys_path_remove(filename.c_str(), &error));
+    EXPECT_EQ(nullptr, error);
+
+    // try saving a file to a path that doesn't exist
+    filename = "/this/path/does/not/exist/foo.txt";
+    EXPECT_FALSE(tr_saveFile(filename.c_str(), contents, &error));
+    ASSERT_NE(nullptr, error);
+    EXPECT_NE(0, error->code);
+    tr_error_clear(&error);
+}