From 3b129a72d88ea6ae8d94d88f02c5bdac02544ec0 Mon Sep 17 00:00:00 2001 From: Mike Gelfand Date: Sat, 11 Apr 2015 10:51:59 +0000 Subject: [PATCH] #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. --- cli/cli.c | 2 +- daemon/remote.c | 2 +- libtransmission/error-types.h | 5 ++++ libtransmission/platform.c | 2 +- libtransmission/rename-test.c | 5 ++-- libtransmission/resume.c | 7 ++++-- libtransmission/rpc-server.c | 21 ++++++++-------- libtransmission/session.c | 19 ++++++++------ libtransmission/stats.c | 4 +-- libtransmission/torrent-ctor.c | 2 +- libtransmission/torrent-magnet.c | 4 +-- libtransmission/torrent.c | 2 +- libtransmission/tr-dht.c | 2 +- libtransmission/utils.c | 43 ++++++++++++-------------------- libtransmission/utils.h | 5 ++-- libtransmission/variant.c | 29 ++++++++++----------- libtransmission/variant.h | 9 ++++--- qt/prefs.cc | 2 +- utils/edit.c | 7 ++++-- 19 files changed, 91 insertions(+), 81 deletions(-) diff --git a/cli/cli.c b/cli/cli.c index 6b903d9ac..2c9f06a09 100644 --- 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 4898d1341..ee5b88068 100644 --- 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 f3689c59e..476a8d49c 100644 --- a/libtransmission/error-types.h +++ b/libtransmission/error-types.h @@ -14,17 +14,22 @@ #include +#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 +#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 92d8b60d6..f4f9ca0ce 100644 --- 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 94d4bc0b9..489ee4a84 100644 --- 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 03131a770..29902454f 100644 --- 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 aa0cb5c37..e67cbdcd5 100644 --- 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 b259594b7..8e83441d2 100644 --- 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 b57a2d2f1..bf4f763d7 100644 --- 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 c8d52ca92..21b18fa35 100644 --- 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 850086599..fd71cd75c 100644 --- 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 8b16738c7..03ff37b2c 100644 --- 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 8d34fa954..3bb665cc6 100644 --- 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 d4c9f6699..e40c9a9c0 100644 --- 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 18aacb9ce..8289f24ea 100644 --- 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 e0f6ca553..fc1f826b2 100644 --- 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 4a4dbb13a..60ec5ed72 100644 --- 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 0a5c0dc7d..818327c0b 100644 --- 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, ¤t_settings); tr_variantToFile (&file_settings, TR_VARIANT_FMT_JSON, file.fileName().toUtf8().constData()); diff --git a/utils/edit.c b/utils/edit.c index ced150b89..4559e17db 100644 --- a/utils/edit.c +++ b/utils/edit.c @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -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; } -- 2.40.0