#5908: Check for `tr_loadFile` return value instead of `errno` in `tr_variantFromFile`

Seems like there could be a defect in uClibc making errno not
thread-local. Don't rely on errno value but check function return value
instead which is a better failure indicator.

Return errors from `tr_loadFile` and `tr_variantFromFile` via tr_error.
Fix `tr_sessionLoadSettings` to not fail on Windows if settings.json
does not exist.

diff --git a/cli/cli.c b/cli/cli.c
index 6b903d9acfdf1f68af7443165d12b16cd0136ee2..2c9f06a0942143c187354628a6e846544f407bf8 100644 (file)
--- a/cli/cli.c
+++ b/cli/cli.c
@@ -287,7 +287,7 @@ main (int argc, char ** argv)
 
   ctor = tr_ctorNew (h);
 
-  fileContents = tr_loadFile (torrentPath, &fileLength);
+  fileContents = tr_loadFile (torrentPath, &fileLength, NULL);
   tr_ctorSetPaused (ctor, TR_FORCE, false);
   if (fileContents != NULL)
     {

diff --git a/daemon/remote.c b/daemon/remote.c
index 4898d1341ae28cfad115f63302745c1c8b1ae902..ee5b88068582f1189b8cf5b3b3a925503fe0bacc 100644 (file)
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -532,7 +532,7 @@ getEncodedMetainfo (const char * filename)
 {
     size_t    len = 0;
     char *    b64 = NULL;
-    uint8_t * buf = tr_loadFile (filename, &len);
+    uint8_t * buf = tr_loadFile (filename, &len, NULL);
 
     if (buf)
     {

diff --git a/libtransmission/error-types.h b/libtransmission/error-types.h
index f3689c59e62ae3ec84adccb2000c51a34456dff6..476a8d49c3fd72ab344465268c5e42269913d3c4 100644 (file)
--- a/libtransmission/error-types.h
+++ b/libtransmission/error-types.h
@@ -14,17 +14,22 @@
 
 #include <windows.h>
 
+#define TR_ERROR_IS_ENOENT(code) ((code) == ERROR_FILE_NOT_FOUND || \
+                                  (code) == ERROR_PATH_NOT_FOUND)
 #define TR_ERROR_IS_ENOSPC(code) ((code) == ERROR_DISK_FULL)
 
 #define TR_ERROR_EINVAL ERROR_INVALID_PARAMETER
+#define TR_ERROR_EISDIR ERROR_DIRECTORY_NOT_SUPPORTED
 
 #else /* _WIN32 */
 
 #include <errno.h>
 
+#define TR_ERROR_IS_ENOENT(code) ((code) == ENOENT)
 #define TR_ERROR_IS_ENOSPC(code) ((code) == ENOSPC)
 
 #define TR_ERROR_EINVAL EINVAL
+#define TR_ERROR_EISDIR EISDIR
 
 #endif /* _WIN32 */
 

diff --git a/libtransmission/platform.c b/libtransmission/platform.c
index 92d8b60d682c09cc9234f9788a0955dedf9058a0..f4f9ca0ce48455e79afb5a536a678f95831beac7 100644 (file)
--- a/libtransmission/platform.c
+++ b/libtransmission/platform.c
@@ -375,7 +375,7 @@ tr_getDefaultDownloadDir (void)
       tr_free (config_home);
 
       /* read in user-dirs.dirs and look for the download dir entry */
-      content = (char *) tr_loadFile (config_file, &content_len);
+      content = (char *) tr_loadFile (config_file, &content_len, NULL);
       if (content && content_len>0)
         {
           const char * key = "XDG_DOWNLOAD_DIR=\"";

diff --git a/libtransmission/rename-test.c b/libtransmission/rename-test.c
index 94d4bc0b96484d44de5bd1d6c7d03a48724c8462..489ee4a847fc024d2aecccf5acb1fb83ba165f55 100644 (file)
--- a/libtransmission/rename-test.c
+++ b/libtransmission/rename-test.c
@@ -53,9 +53,10 @@ testFileExistsAndConsistsOfThisString (const tr_torrent * tor, tr_file_index_t f
 
       assert (tr_sys_path_exists (path, NULL));
 
-      contents = tr_loadFile (path, &contents_len);
+      contents = tr_loadFile (path, &contents_len, NULL);
 
-      success = (str_len == contents_len)
+      success = contents != NULL
+             && (str_len == contents_len)
              && (!memcmp (contents, str, contents_len));
 
       tr_free (contents);

diff --git a/libtransmission/resume.c b/libtransmission/resume.c
index 03131a77089d7d624dc6e04cbe6407239b2ca431..29902454f98ec6668f80bf7b65fdd56be3b99f72 100644 (file)
--- a/libtransmission/resume.c
+++ b/libtransmission/resume.c
@@ -11,6 +11,7 @@
 
 #include "transmission.h"
 #include "completion.h"
+#include "error.h"
 #include "file.h"
 #include "log.h"
 #include "metainfo.h" /* tr_metainfoGetBasename () */
@@ -706,14 +707,16 @@ loadFromFile (tr_torrent * tor, uint64_t fieldsToLoad)
   bool boolVal;
   uint64_t fieldsLoaded = 0;
   const bool wasDirty = tor->isDirty;
+  tr_error * error = NULL;
 
   assert (tr_isTorrent (tor));
 
   filename = getResumeFilename (tor);
 
-  if (tr_variantFromFile (&top, TR_VARIANT_FMT_BENC, filename))
+  if (!tr_variantFromFile (&top, TR_VARIANT_FMT_BENC, filename, &error))
     {
-      tr_logAddTorDbg (tor, "Couldn't read \"%s\"", filename);
+      tr_logAddTorDbg (tor, "Couldn't read \"%s\": %s", filename, error->message);
+      tr_error_free (error);
 
       tr_free (filename);
       return fieldsLoaded;

diff --git a/libtransmission/rpc-server.c b/libtransmission/rpc-server.c
index aa0cb5c37fb4a782f959d6511bd806084a69a8d7..e67cbdcd5e54495ff59fbbb1a084b86ba2a0f5e5 100644 (file)
--- a/libtransmission/rpc-server.c
+++ b/libtransmission/rpc-server.c
@@ -429,27 +429,27 @@ serve_file (struct evhttp_request  * req,
     {
       void * file;
       size_t file_len;
-      struct evbuffer * content;
-      const int error = errno;
+      tr_error * error = NULL;
 
-      errno = 0;
       file_len = 0;
-      file = tr_loadFile (filename, &file_len);
-      content = evbuffer_new ();
-      evbuffer_add_reference (content, file, file_len, evbuffer_ref_cleanup_tr_free, file);
+      file = tr_loadFile (filename, &file_len, &error);
 
-      if (errno)
+      if (file == NULL)
         {
-          char * tmp = tr_strdup_printf ("%s (%s)", filename, tr_strerror (errno));
+          char * tmp = tr_strdup_printf ("%s (%s)", filename, error->message);
           send_simple_response (req, HTTP_NOTFOUND, tmp);
           tr_free (tmp);
+          tr_error_free (error);
         }
       else
         {
+          struct evbuffer * content;
           struct evbuffer * out;
           const time_t now = tr_time ();
 
-          errno = error;
+          content = evbuffer_new ();
+          evbuffer_add_reference (content, file, file_len, evbuffer_ref_cleanup_tr_free, file);
+
           out = evbuffer_new ();
           evhttp_add_header (req->output_headers, "Content-Type", mimetype_guess (filename));
           add_time_header (req->output_headers, "Date", now);
@@ -458,9 +458,8 @@ serve_file (struct evhttp_request  * req,
           evhttp_send_reply (req, HTTP_OK, "OK", out);
 
           evbuffer_free (out);
+          evbuffer_free (content);
         }
-
-      evbuffer_free (content);
     }
 }
 

diff --git a/libtransmission/session.c b/libtransmission/session.c
index b259594b796960eb55c3ebf4e45311c61db27aec..8e83441d2e2909679d6abad755ba469fba28f63d 100644 (file)
--- a/libtransmission/session.c
+++ b/libtransmission/session.c
@@ -32,6 +32,8 @@
 #include "blocklist.h"
 #include "cache.h"
 #include "crypto-utils.h"
+#include "error.h"
+#include "error-types.h"
 #include "fdlimit.h"
 #include "file.h"
 #include "list.h"
@@ -456,12 +458,12 @@ tr_sessionGetSettings (tr_session * s, tr_variant * d)
 bool
 tr_sessionLoadSettings (tr_variant * dict, const char * configDir, const char * appName)
 {
-  int err = 0;
   char * filename;
   tr_variant fileSettings;
   tr_variant sessionDefaults;
   tr_variant tmp;
-  bool success = false;
+  bool success;
+  tr_error * error = NULL;
 
   assert (tr_variantIsDict (dict));
 
@@ -480,17 +482,21 @@ tr_sessionLoadSettings (tr_variant * dict, const char * configDir, const char *
 
   /* file settings override the defaults */
   filename = tr_buildPath (configDir, "settings.json", NULL);
-  err = tr_variantFromFile (&fileSettings, TR_VARIANT_FMT_JSON, filename);
-  if (!err)
+  if (tr_variantFromFile (&fileSettings, TR_VARIANT_FMT_JSON, filename, &error))
     {
       tr_variantMergeDicts (dict, &fileSettings);
       tr_variantFree (&fileSettings);
+      success = true;
+    }
+  else
+    {
+      success = TR_ERROR_IS_ENOENT (error->code);
+      tr_error_free (error);
     }
 
   /* cleanup */
   tr_variantFree (&sessionDefaults);
   tr_free (filename);
-  success = (err==0) || (err==ENOENT);
   return success;
 }
 
@@ -509,8 +515,7 @@ tr_sessionSaveSettings (tr_session       * session,
   /* the existing file settings are the fallback values */
   {
     tr_variant fileSettings;
-    const int err = tr_variantFromFile (&fileSettings, TR_VARIANT_FMT_JSON, filename);
-    if (!err)
+    if (tr_variantFromFile (&fileSettings, TR_VARIANT_FMT_JSON, filename, NULL))
       {
         tr_variantMergeDicts (&settings, &fileSettings);
         tr_variantFree (&fileSettings);

diff --git a/libtransmission/stats.c b/libtransmission/stats.c
index b57a2d2f1ce73243c6aa8b2b00a39d3caca2fd49..bf4f763d79a4363ec9e7da6b814dfd243ab80a65 100644 (file)
--- a/libtransmission/stats.c
+++ b/libtransmission/stats.c
@@ -50,13 +50,13 @@ loadCumulativeStats (const tr_session * session, tr_session_stats * setme)
   bool loaded = false;
 
   filename = getFilename (session);
-  loaded = !tr_variantFromFile (&top, TR_VARIANT_FMT_JSON, filename);
+  loaded = tr_variantFromFile (&top, TR_VARIANT_FMT_JSON, filename, NULL);
   tr_free (filename);
 
   if (!loaded)
     {
       filename = getOldFilename (session);
-      loaded = !tr_variantFromFile (&top, TR_VARIANT_FMT_BENC, filename);
+      loaded = tr_variantFromFile (&top, TR_VARIANT_FMT_BENC, filename, NULL);
       tr_free (filename);
     }
 

diff --git a/libtransmission/torrent-ctor.c b/libtransmission/torrent-ctor.c
index c8d52ca92c85b8019db4ec7175525de6a19473ae..21b18fa3510896bb2144b866cc3518e49e18f549 100644 (file)
--- a/libtransmission/torrent-ctor.c
+++ b/libtransmission/torrent-ctor.c
@@ -135,7 +135,7 @@ tr_ctorSetMetainfoFromFile (tr_ctor *    ctor,
     size_t    len;
     int       err;
 
-    metainfo = tr_loadFile (filename, &len);
+    metainfo = tr_loadFile (filename, &len, NULL);
     if (metainfo && len)
         err = tr_ctorSetMetainfo (ctor, metainfo, len);
     else

diff --git a/libtransmission/torrent-magnet.c b/libtransmission/torrent-magnet.c
index 8500865991de560dd2628044c3dea2f14d90195d..fd71cd75c304dc797873c576261b7351d5c7c5a6 100644 (file)
--- a/libtransmission/torrent-magnet.c
+++ b/libtransmission/torrent-magnet.c
@@ -106,7 +106,7 @@ findInfoDictOffset (const tr_torrent * tor)
   int offset = 0;
 
   /* load the file, and find the info dict's offset inside the file */
-  if ((fileContents = tr_loadFile (tor->info.torrent, &fileLen)))
+  if ((fileContents = tr_loadFile (tor->info.torrent, &fileLen, NULL)))
     {
       tr_variant top;
 
@@ -253,7 +253,7 @@ tr_torrentSetMetadataPiece (tr_torrent  * tor, int piece, const void  * data, in
               tr_variant newMetainfo;
               char * path = tr_strdup (tor->info.torrent);
 
-              if (!tr_variantFromFile (&newMetainfo, TR_VARIANT_FMT_BENC, path))
+              if (tr_variantFromFile (&newMetainfo, TR_VARIANT_FMT_BENC, path, NULL))
                 {
                   bool hasInfo;
                   tr_info info;

diff --git a/libtransmission/torrent.c b/libtransmission/torrent.c
index 8b16738c705c71f691c211a5eaef1f98618699f3..03ff37b2c6b0eb602c69b2acb294b3258fe89ae0 100644 (file)
--- a/libtransmission/torrent.c
+++ b/libtransmission/torrent.c
@@ -2660,7 +2660,7 @@ tr_torrentSetAnnounceList (tr_torrent             * tor,
       ok = false;
 
   /* save to the .torrent file */
-  if (ok && !tr_variantFromFile (&metainfo, TR_VARIANT_FMT_BENC, tor->info.torrent))
+  if (ok && tr_variantFromFile (&metainfo, TR_VARIANT_FMT_BENC, tor->info.torrent, NULL))
     {
       bool hasInfo;
       tr_info tmpInfo;

diff --git a/libtransmission/tr-dht.c b/libtransmission/tr-dht.c
index 8d34fa95405495fb4e006be8207607a89ed079ee..3bb665cc6290de1b2434c3e1d041c8aa90d28f14 100644 (file)
--- a/libtransmission/tr-dht.c
+++ b/libtransmission/tr-dht.c
@@ -281,7 +281,7 @@ tr_dhtInit (tr_session *ss)
         dht_debug = stderr;
 
     dat_file = tr_buildPath (ss->configDir, "dht.dat", NULL);
-    rc = tr_variantFromFile (&benc, TR_VARIANT_FMT_BENC, dat_file);
+    rc = tr_variantFromFile (&benc, TR_VARIANT_FMT_BENC, dat_file, NULL) ? 0 : -1;
     tr_free (dat_file);
     if (rc == 0) {
         have_id = tr_variantDictFindRaw (&benc, TR_KEY_id, &raw, &len);

diff --git a/libtransmission/utils.c b/libtransmission/utils.c
index d4c9f6699ab4f573aa79897d06bea5f814d71489..e40c9a9c077802440e4226bb6ca0d483e8af1c9f 100644 (file)
--- a/libtransmission/utils.c
+++ b/libtransmission/utils.c
@@ -213,29 +213,28 @@ tr_timerAddMsec (struct event * timer, int msec)
 **/
 
 uint8_t *
-tr_loadFile (const char * path,
-             size_t     * size)
+tr_loadFile (const char  * path,
+             size_t      * size,
+             tr_error   ** error)
 {
   uint8_t * buf;
   tr_sys_path_info info;
   tr_sys_file_t fd;
-  tr_error * error = NULL;
+  tr_error * my_error = NULL;
   const char * const err_fmt = _("Couldn't read \"%1$s\": %2$s");
 
   /* try to stat the file */
-  if (!tr_sys_path_get_info (path, 0, &info, &error))
+  if (!tr_sys_path_get_info (path, 0, &info, &my_error))
     {
-      const int err = error->code;
-      tr_logAddDebug (err_fmt, path, error->message);
-      tr_error_free (error);
-      errno = err;
+      tr_logAddDebug (err_fmt, path, my_error->message);
+      tr_error_propagate (error, &my_error);
       return NULL;
     }
 
   if (info.type != TR_SYS_PATH_IS_FILE)
     {
       tr_logAddError (err_fmt, path, _("Not a regular file"));
-      errno = EISDIR;
+      tr_error_set_literal (error, TR_ERROR_EISDIR, _("Not a regular file"));
       return NULL;
     }
 
@@ -244,32 +243,22 @@ tr_loadFile (const char * path,
     assert (info.size <= SIZE_MAX);
 
   /* Load the torrent file into our buffer */
-  fd = tr_sys_file_open (path, TR_SYS_FILE_READ | TR_SYS_FILE_SEQUENTIAL, 0, &error);
+  fd = tr_sys_file_open (path, TR_SYS_FILE_READ | TR_SYS_FILE_SEQUENTIAL, 0, &my_error);
   if (fd == TR_BAD_SYS_FILE)
     {
-      const int err = error->code;
-      tr_logAddError (err_fmt, path, error->message);
-      tr_error_free (error);
-      errno = err;
+      tr_logAddError (err_fmt, path, my_error->message);
+      tr_error_propagate (error, &my_error);
       return NULL;
     }
+
   buf = tr_malloc (info.size + 1);
-  if (!buf)
-    {
-      const int err = errno;
-      tr_logAddError (err_fmt, path, _("Memory allocation failed"));
-      tr_sys_file_close (fd, NULL);
-      errno = err;
-      return NULL;
-    }
-  if (!tr_sys_file_read (fd, buf, info.size, NULL, &error))
+
+  if (!tr_sys_file_read (fd, buf, info.size, NULL, &my_error))
     {
-      const int err = error->code;
-      tr_logAddError (err_fmt, path, error->message);
+      tr_logAddError (err_fmt, path, my_error->message);
       tr_sys_file_close (fd, NULL);
       free (buf);
-      tr_error_free (error);
-      errno = err;
+      tr_error_propagate (error, &my_error);
       return NULL;
     }
 

diff --git a/libtransmission/utils.h b/libtransmission/utils.h
index 18aacb9ce24145696ebcddcb01ce5f0127c92482..8289f24ea79f5e350d54c5583f5a5640139d98a9 100644 (file)
--- a/libtransmission/utils.h
+++ b/libtransmission/utils.h
@@ -132,8 +132,9 @@ bool tr_wildmat (const char * text, const char * pattern) TR_GNUC_NONNULL (1,2);
  * @brief Loads a file and returns its contents.
  * On failure, NULL is returned and errno is set.
  */
-uint8_t* tr_loadFile (const char * filename, size_t * size) TR_GNUC_MALLOC
-                                                             TR_GNUC_NONNULL (1);
+uint8_t * tr_loadFile (const char  * filename,
+                       size_t      * size,
+                       tr_error   ** error) TR_GNUC_MALLOC TR_GNUC_NONNULL (1);
 
 
 /** @brief build a filename from a series of elements using the

diff --git a/libtransmission/variant.c b/libtransmission/variant.c
index e0f6ca5532b382bbe86a199fa4ea1671c6748b65..fc1f826b24b4ddbeda0bdd91a7308ac764c82344 100644 (file)
--- a/libtransmission/variant.c
+++ b/libtransmission/variant.c
@@ -1218,27 +1218,28 @@ tr_variantToFile (const tr_variant  * v,
 ****
 ***/
 
-int
+bool
 tr_variantFromFile (tr_variant      * setme,
                     tr_variant_fmt    fmt,
-                    const char      * filename)
+                    const char      * filename,
+                    tr_error       ** error)
 {
-  int err;
-  size_t buflen;
+  bool ret = false;
   uint8_t * buf;
-  const int old_errno = errno;
+  size_t buflen;
 
-  errno = 0;
-  buf = tr_loadFile (filename, &buflen);
+  buf = tr_loadFile (filename, &buflen, error);
+  if (buf != NULL)
+    {
+      if (tr_variantFromBuf (setme, fmt, buf, buflen, filename, NULL) == 0)
+        ret = true;
+      else
+        tr_error_set_literal (error, 0, _("Unable to parse file content"));
 
-  if (errno)
-    err = errno;
-  else
-    err = tr_variantFromBuf (setme, fmt, buf, buflen, filename, NULL);
+      tr_free (buf);
+    }
 
-  tr_free (buf);
-  errno = old_errno;
-  return err;
+  return ret;
 }
 
 int

diff --git a/libtransmission/variant.h b/libtransmission/variant.h
index 4a4dbb13ab0c338a7626f2c5a6e0e3a8084a4593..60ec5ed72ca22bede4c4094f4f542ccb5f0f334a 100644 (file)
--- a/libtransmission/variant.h
+++ b/libtransmission/variant.h
@@ -19,6 +19,8 @@ extern "C" {
 
 struct evbuffer;
 
+struct tr_error;
+
 /**
  * @addtogroup tr_variant Variant
  *
@@ -120,9 +122,10 @@ struct evbuffer * tr_variantToBuf (const tr_variant * variant,
                                    tr_variant_fmt     fmt);
 
 /* TR_VARIANT_FMT_JSON_LEAN and TR_VARIANT_FMT_JSON are equivalent here. */
-int tr_variantFromFile (tr_variant      * setme,
-                        tr_variant_fmt    fmt,
-                        const char      * filename);
+bool tr_variantFromFile (tr_variant       * setme,
+                         tr_variant_fmt     fmt,
+                         const char       * filename,
+                         struct tr_error ** error);
 
 /* TR_VARIANT_FMT_JSON_LEAN and TR_VARIANT_FMT_JSON are equivalent here. */
 int tr_variantFromBuf (tr_variant     * setme,

diff --git a/qt/prefs.cc b/qt/prefs.cc
index 0a5c0dc7d675370160eb950ae3551e7162d9b27b..818327c0b897cf8f95c0db6635c3ae8663b02fe5 100644 (file)
--- a/qt/prefs.cc
+++ b/qt/prefs.cc
@@ -258,7 +258,7 @@ Prefs::~Prefs ()
   // update settings.json with our settings
   tr_variant file_settings;
   const QFile file (QDir(myConfigDir).absoluteFilePath(QLatin1String ("settings.json")));
-  if (tr_variantFromFile (&file_settings, TR_VARIANT_FMT_JSON, file.fileName().toUtf8().constData()))
+  if (!tr_variantFromFile (&file_settings, TR_VARIANT_FMT_JSON, file.fileName().toUtf8().constData(), NULL))
     tr_variantInitDict (&file_settings, PREFS_COUNT);
   tr_variantMergeDicts (&file_settings, &current_settings);
   tr_variantToFile (&file_settings, TR_VARIANT_FMT_JSON, file.fileName().toUtf8().constData());

diff --git a/utils/edit.c b/utils/edit.c
index ced150b8921e30267b0880ceeff35ff55ed3dd37..4559e17db1939621f1554322d9a3af245cd40d7c 100644 (file)
--- a/utils/edit.c
+++ b/utils/edit.c
@@ -14,6 +14,7 @@
 #include <event2/buffer.h>
 
 #include <libtransmission/transmission.h>
+#include <libtransmission/error.h>
 #include <libtransmission/tr-getopt.h>
 #include <libtransmission/utils.h>
 #include <libtransmission/variant.h>
@@ -328,12 +329,14 @@ main (int argc, char * argv[])
       tr_variant top;
       bool changed = false;
       const char * filename = files[i];
+      tr_error * error = NULL;
 
       printf ("%s\n", filename);
 
-      if (tr_variantFromFile (&top, TR_VARIANT_FMT_BENC, filename))
+      if (!tr_variantFromFile (&top, TR_VARIANT_FMT_BENC, filename, &error))
         {
-          printf ("\tError reading file\n");
+          printf ("\tError reading file: %s\n", error->message);
+          tr_error_free (error);
           continue;
         }