From 97a0fed7340279ed211d14596d1162a8c07c509d Mon Sep 17 00:00:00 2001 From: Mike Gelfand Date: Sun, 17 Mar 2019 09:09:08 +0300 Subject: [PATCH] Remove side effects from right hand operands of && or || * MISRA C:2004, 12.4 - The right-hand operand of a logical && or || operator shall not contain side effects. * MISRA C++:2008, 5-14-1 - The right hand operand of a logical && or || operator shall not contain side effects. * MISRA C:2012, 13.5 - The right hand operand of a logical && or || operator shall not contain persistent side effects * CERT, EXP02-C. - Be aware of the short-circuit behavior of the logical AND and OR operators --- libtransmission/libtransmission-test.c | 54 +++++----- libtransmission/makemeta.c | 5 +- libtransmission/metainfo.c | 6 +- libtransmission/peer-msgs.c | 131 +++++++++++++------------ libtransmission/rpc-server.c | 31 +++--- libtransmission/rpcimpl.c | 13 +-- libtransmission/session.c | 38 +++---- libtransmission/variant.c | 26 +++-- libtransmission/webseed.c | 20 ++-- 9 files changed, 173 insertions(+), 151 deletions(-) diff --git a/libtransmission/libtransmission-test.c b/libtransmission/libtransmission-test.c index 98c60b4da..ea8d81496 100644 --- a/libtransmission/libtransmission-test.c +++ b/libtransmission/libtransmission-test.c @@ -215,34 +215,36 @@ static void rm_rf(char const* killme) { tr_sys_path_info info; - if (tr_sys_path_get_info(killme, 0, &info, NULL)) + if (!tr_sys_path_get_info(killme, 0, &info, NULL)) { - tr_sys_dir_t odir; - - if (info.type == TR_SYS_PATH_IS_DIRECTORY && (odir = tr_sys_dir_open(killme, NULL)) != TR_BAD_SYS_DIR) - { - char const* name; - - while ((name = tr_sys_dir_read_name(odir, NULL)) != NULL) - { - if (strcmp(name, ".") != 0 && strcmp(name, "..") != 0) - { - char* tmp = tr_buildPath(killme, name, NULL); - rm_rf(tmp); - tr_free(tmp); - } - } - - tr_sys_dir_close(odir, NULL); - } - - if (verbose) - { - fprintf(stderr, "cleanup: removing %s\n", killme); - } - - tr_sys_path_remove(killme, NULL); + return; } + + tr_sys_dir_t odir = info.type == TR_SYS_PATH_IS_DIRECTORY ? tr_sys_dir_open(killme, NULL) : TR_BAD_SYS_DIR; + + if (odir != TR_BAD_SYS_DIR) + { + char const* name; + + while ((name = tr_sys_dir_read_name(odir, NULL)) != NULL) + { + if (strcmp(name, ".") != 0 && strcmp(name, "..") != 0) + { + char* tmp = tr_buildPath(killme, name, NULL); + rm_rf(tmp); + tr_free(tmp); + } + } + + tr_sys_dir_close(odir, NULL); + } + + if (verbose) + { + fprintf(stderr, "cleanup: removing %s\n", killme); + } + + tr_sys_path_remove(killme, NULL); } void libtest_sandbox_destroy(char const* sandbox) diff --git a/libtransmission/makemeta.c b/libtransmission/makemeta.c index 462c4bcbc..481dc2a3d 100644 --- a/libtransmission/makemeta.c +++ b/libtransmission/makemeta.c @@ -43,7 +43,6 @@ static struct FileList* getFiles(char const* dir, char const* base, struct FileL return NULL; } - tr_sys_dir_t odir; char* buf; tr_sys_path_info info; tr_error* error = NULL; @@ -58,7 +57,9 @@ static struct FileList* getFiles(char const* dir, char const* base, struct FileL return list; } - if (info.type == TR_SYS_PATH_IS_DIRECTORY && (odir = tr_sys_dir_open(buf, NULL)) != TR_BAD_SYS_DIR) + tr_sys_dir_t odir = info.type == TR_SYS_PATH_IS_DIRECTORY ? tr_sys_dir_open(buf, NULL) : TR_BAD_SYS_DIR; + + if (odir != TR_BAD_SYS_DIR) { char const* name; diff --git a/libtransmission/metainfo.c b/libtransmission/metainfo.c index f96e605d5..9edccd994 100644 --- a/libtransmission/metainfo.c +++ b/libtransmission/metainfo.c @@ -233,7 +233,6 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const* static char* tr_convertAnnounceToScrape(char const* announce) { char* scrape = NULL; - char const* s; /* To derive the scrape URL use the following steps: * Begin with the announce URL. Find the last '/' in it. @@ -241,7 +240,10 @@ static char* tr_convertAnnounceToScrape(char const* announce) * it will be taken as a sign that that tracker doesn't support * the scrape convention. If it does, substitute 'scrape' for * 'announce' to find the scrape page. */ - if ((s = strrchr(announce, '/')) != NULL && strncmp(s + 1, "announce", 8) == 0) + + char const* s = strrchr(announce, '/'); + + if (s != NULL && strncmp(s + 1, "announce", 8) == 0) { char const* prefix = announce; size_t const prefix_len = s + 1 - announce; diff --git a/libtransmission/peer-msgs.c b/libtransmission/peer-msgs.c index b527ba575..94c16e9b1 100644 --- a/libtransmission/peer-msgs.c +++ b/libtransmission/peer-msgs.c @@ -1146,88 +1146,93 @@ static void parseUtMetadata(tr_peerMsgs* msgs, uint32_t msglen, struct evbuffer* static void parseUtPex(tr_peerMsgs* msgs, uint32_t msglen, struct evbuffer* inbuf) { - bool loaded = false; - uint8_t* tmp = tr_new(uint8_t, msglen); - tr_variant val; tr_torrent* tor = msgs->torrent; + if (!tr_torrentAllowsPex(tor)) + { + return; + } + + uint8_t* tmp = tr_new(uint8_t, msglen); + tr_peerIoReadBytes(msgs->io, inbuf, tmp, msglen); + + tr_variant val; + bool const loaded = tr_variantFromBenc(&val, tmp, msglen) == 0; + + tr_free(tmp); + + if (!loaded) + { + return; + } + uint8_t const* added; size_t added_len; - tr_peerIoReadBytes(msgs->io, inbuf, tmp, msglen); - - if (tr_torrentAllowsPex(tor) && (loaded = tr_variantFromBenc(&val, tmp, msglen) == 0)) + if (tr_variantDictFindRaw(&val, TR_KEY_added, &added, &added_len)) { - if (tr_variantDictFindRaw(&val, TR_KEY_added, &added, &added_len)) + tr_pex* pex; + size_t n; + size_t added_f_len; + uint8_t const* added_f; + + if (!tr_variantDictFindRaw(&val, TR_KEY_added_f, &added_f, &added_f_len)) { - tr_pex* pex; - size_t n; - size_t added_f_len; - uint8_t const* added_f; - - if (!tr_variantDictFindRaw(&val, TR_KEY_added_f, &added_f, &added_f_len)) - { - added_f_len = 0; - added_f = NULL; - } - - pex = tr_peerMgrCompactToPex(added, added_len, added_f, added_f_len, &n); - - n = MIN(n, MAX_PEX_PEER_COUNT); - - for (size_t i = 0; i < n; ++i) - { - int seedProbability = -1; - - if (i < added_f_len) - { - seedProbability = (added_f[i] & ADDED_F_SEED_FLAG) != 0 ? 100 : 0; - } - - tr_peerMgrAddPex(tor, TR_PEER_FROM_PEX, pex + i, seedProbability); - } - - tr_free(pex); + added_f_len = 0; + added_f = NULL; } - if (tr_variantDictFindRaw(&val, TR_KEY_added6, &added, &added_len)) + pex = tr_peerMgrCompactToPex(added, added_len, added_f, added_f_len, &n); + + n = MIN(n, MAX_PEX_PEER_COUNT); + + for (size_t i = 0; i < n; ++i) { - tr_pex* pex; - size_t n; - size_t added_f_len; - uint8_t const* added_f; + int seedProbability = -1; - if (!tr_variantDictFindRaw(&val, TR_KEY_added6_f, &added_f, &added_f_len)) + if (i < added_f_len) { - added_f_len = 0; - added_f = NULL; + seedProbability = (added_f[i] & ADDED_F_SEED_FLAG) != 0 ? 100 : 0; } - pex = tr_peerMgrCompact6ToPex(added, added_len, added_f, added_f_len, &n); - - n = MIN(n, MAX_PEX_PEER_COUNT); - - for (size_t i = 0; i < n; ++i) - { - int seedProbability = -1; - - if (i < added_f_len) - { - seedProbability = (added_f[i] & ADDED_F_SEED_FLAG) != 0 ? 100 : 0; - } - - tr_peerMgrAddPex(tor, TR_PEER_FROM_PEX, pex + i, seedProbability); - } - - tr_free(pex); + tr_peerMgrAddPex(tor, TR_PEER_FROM_PEX, pex + i, seedProbability); } + + tr_free(pex); } - if (loaded) + if (tr_variantDictFindRaw(&val, TR_KEY_added6, &added, &added_len)) { - tr_variantFree(&val); + tr_pex* pex; + size_t n; + size_t added_f_len; + uint8_t const* added_f; + + if (!tr_variantDictFindRaw(&val, TR_KEY_added6_f, &added_f, &added_f_len)) + { + added_f_len = 0; + added_f = NULL; + } + + pex = tr_peerMgrCompact6ToPex(added, added_len, added_f, added_f_len, &n); + + n = MIN(n, MAX_PEX_PEER_COUNT); + + for (size_t i = 0; i < n; ++i) + { + int seedProbability = -1; + + if (i < added_f_len) + { + seedProbability = (added_f[i] & ADDED_F_SEED_FLAG) != 0 ? 100 : 0; + } + + tr_peerMgrAddPex(tor, TR_PEER_FROM_PEX, pex + i, seedProbability); + } + + tr_free(pex); } - tr_free(tmp); + tr_variantFree(&val); } static void sendPex(tr_peerMsgs* msgs); diff --git a/libtransmission/rpc-server.c b/libtransmission/rpc-server.c index c34247910..6a112a6c7 100644 --- a/libtransmission/rpc-server.c +++ b/libtransmission/rpc-server.c @@ -511,24 +511,28 @@ static void handle_rpc_from_json(struct evhttp_request* req, struct tr_rpc_serve static void handle_rpc(struct evhttp_request* req, struct tr_rpc_server* server) { - char const* q; - if (req->type == EVHTTP_REQ_POST) { handle_rpc_from_json(req, server, (char const*)evbuffer_pullup(req->input_buffer, -1), evbuffer_get_length(req->input_buffer)); + return; } - else if (req->type == EVHTTP_REQ_GET && (q = strchr(req->uri, '?')) != NULL) + + if (req->type == EVHTTP_REQ_GET) { - struct rpc_response_data* data = tr_new0(struct rpc_response_data, 1); - data->req = req; - data->server = server; - tr_rpc_request_exec_uri(server->session, q + 1, TR_BAD_SIZE, rpc_response_func, data); - } - else - { - send_simple_response(req, 405, NULL); + char const* q = strchr(req->uri, '?'); + + if (q != NULL) + { + struct rpc_response_data* data = tr_new0(struct rpc_response_data, 1); + data->req = req; + data->server = server; + tr_rpc_request_exec_uri(server->session, q + 1, TR_BAD_SIZE, rpc_response_func, data); + return; + } } + + send_simple_response(req, 405, NULL); } static bool isAddressAllowed(tr_rpc_server const* server, char const* address) @@ -650,12 +654,11 @@ static void handle_request(struct evhttp_request* req, void* arg) if (auth != NULL && evutil_ascii_strncasecmp(auth, "basic ", 6) == 0) { - size_t plen; - char* p = tr_base64_decode_str(auth + 6, &plen); + char* p = tr_base64_decode_str(auth + 6, NULL); if (p != NULL) { - if (plen > 0 && (pass = strchr(p, ':')) != NULL) + if ((pass = strchr(p, ':')) != NULL) { user = p; *pass++ = '\0'; diff --git a/libtransmission/rpcimpl.c b/libtransmission/rpcimpl.c index 94f0624f4..b510bb747 100644 --- a/libtransmission/rpcimpl.c +++ b/libtransmission/rpcimpl.c @@ -1187,8 +1187,6 @@ static char const* addTrackerUrls(tr_torrent* tor, tr_variant* urls) static char const* replaceTrackers(tr_torrent* tor, tr_variant* urls) { - int i; - tr_variant* pair[2]; tr_tracker_info* trackers; bool changed = false; tr_info const* inf = tr_torrentInfo(tor); @@ -1200,23 +1198,20 @@ static char const* replaceTrackers(tr_torrent* tor, tr_variant* urls) copyTrackers(trackers, inf->trackers, n); /* make the substitutions... */ - i = 0; - - while ((pair[0] = tr_variantListChild(urls, i)) != NULL && (pair[1] = tr_variantListChild(urls, i + 1)) != NULL) + for (size_t i = 0, url_count = tr_variantListSize(urls); i + 1 < url_count; i += 2) { size_t len; int64_t pos; char const* newval; - if (tr_variantGetInt(pair[0], &pos) && tr_variantGetStr(pair[1], &newval, &len) && tr_urlIsValidTracker(newval) && - pos < n && pos >= 0) + if (tr_variantGetInt(tr_variantListChild(urls, i), &pos) && + tr_variantGetStr(tr_variantListChild(urls, i + 1), &newval, &len) && + tr_urlIsValidTracker(newval) && pos < n && pos >= 0) { tr_free(trackers[pos].announce); trackers[pos].announce = tr_strndup(newval, len); changed = true; } - - i += 2; } if (!changed) diff --git a/libtransmission/session.c b/libtransmission/session.c index 9a0b7c104..7623fcc1b 100644 --- a/libtransmission/session.c +++ b/libtransmission/session.c @@ -2124,15 +2124,16 @@ static void sessionLoadTorrents(void* vdata) int i; int n = 0; - tr_sys_path_info info; - tr_sys_dir_t odir = NULL; tr_list* list = NULL; - char const* dirname = tr_getTorrentDir(data->session); tr_ctorSetSave(data->ctor, false); /* since we already have them */ - if (tr_sys_path_get_info(dirname, 0, &info, NULL) && info.type == TR_SYS_PATH_IS_DIRECTORY && - (odir = tr_sys_dir_open(dirname, NULL)) != TR_BAD_SYS_DIR) + tr_sys_path_info info; + char const* dirname = tr_getTorrentDir(data->session); + tr_sys_dir_t odir = (tr_sys_path_get_info(dirname, 0, &info, NULL) && info.type == TR_SYS_PATH_IS_DIRECTORY) ? + tr_sys_dir_open(dirname, NULL) : TR_BAD_SYS_DIR; + + if (odir != TR_BAD_SYS_DIR) { char const* name; @@ -2634,24 +2635,24 @@ static void metainfoLookupInit(tr_session* session) { TR_ASSERT(tr_isSession(session)); - tr_sys_path_info info; - char const* dirname = tr_getTorrentDir(session); - tr_sys_dir_t odir; - tr_ctor* ctor = NULL; - tr_variant* lookup; + tr_variant* lookup = tr_new0(tr_variant, 1); + tr_variantInitDict(lookup, 0); + int n = 0; - /* walk through the directory and find the mappings */ - lookup = tr_new0(tr_variant, 1); - tr_variantInitDict(lookup, 0); - ctor = tr_ctorNew(session); - tr_ctorSetSave(ctor, false); /* since we already have them */ + tr_sys_path_info info; + char const* dirname = tr_getTorrentDir(session); + tr_sys_dir_t odir = (tr_sys_path_get_info(dirname, 0, &info, NULL) && info.type == TR_SYS_PATH_IS_DIRECTORY) ? + tr_sys_dir_open(dirname, NULL) : TR_BAD_SYS_DIR; - if (tr_sys_path_get_info(dirname, 0, &info, NULL) && info.type == TR_SYS_PATH_IS_DIRECTORY && - (odir = tr_sys_dir_open(dirname, NULL)) != TR_BAD_SYS_DIR) + if (odir != TR_BAD_SYS_DIR) { + tr_ctor* ctor = tr_ctorNew(session); + tr_ctorSetSave(ctor, false); /* since we already have them */ + char const* name; + /* walk through the directory and find the mappings */ while ((name = tr_sys_dir_read_name(odir, NULL)) != NULL) { if (tr_str_has_suffix(name, ".torrent")) @@ -2671,10 +2672,9 @@ static void metainfoLookupInit(tr_session* session) } tr_sys_dir_close(odir, NULL); + tr_ctorFree(ctor); } - tr_ctorFree(ctor); - session->metainfoLookup = lookup; tr_logAddDebug("Found %d torrents in \"%s\"", n, dirname); } diff --git a/libtransmission/variant.c b/libtransmission/variant.c index b4616f7c9..59e922914 100644 --- a/libtransmission/variant.c +++ b/libtransmission/variant.c @@ -290,20 +290,24 @@ bool tr_variantGetInt(tr_variant const* v, int64_t* setme) { bool success = false; - if (!success && (success = tr_variantIsInt(v))) + if (tr_variantIsInt(v)) { if (setme != NULL) { *setme = v->val.i; } + + success = true; } - if (!success && (success = tr_variantIsBool(v))) + if (!success && tr_variantIsBool(v)) { if (setme != NULL) { *setme = v->val.b ? 1 : 0; } + + success = true; } return success; @@ -344,24 +348,27 @@ bool tr_variantGetBool(tr_variant const* v, bool* setme) char const* str; bool success = false; - if ((success = tr_variantIsBool(v))) + if (tr_variantIsBool(v)) { *setme = v->val.b; + success = true; } if (!success && tr_variantIsInt(v)) { - if ((success = v->val.i == 0 || v->val.i == 1)) + if (v->val.i == 0 || v->val.i == 1) { *setme = v->val.i != 0; + success = true; } } if (!success && tr_variantGetStr(v, &str, NULL)) { - if ((success = strcmp(str, "true") == 0 || strcmp(str, "false") == 0)) + if (strcmp(str, "true") == 0 || strcmp(str, "false") == 0) { *setme = strcmp(str, "true") == 0; + success = true; } } @@ -372,14 +379,16 @@ bool tr_variantGetReal(tr_variant const* v, double* setme) { bool success = false; - if (!success && (success = tr_variantIsReal(v))) + if (tr_variantIsReal(v)) { *setme = v->val.d; + success = true; } - if (!success && (success = tr_variantIsInt(v))) + if (!success && tr_variantIsInt(v)) { *setme = v->val.i; + success = true; } if (!success && tr_variantIsString(v)) @@ -393,9 +402,10 @@ bool tr_variantGetReal(tr_variant const* v, double* setme) d = strtod(getStr(v), &endptr); restore_locale(&locale_ctx); - if ((success = getStr(v) != endptr && *endptr == '\0')) + if (getStr(v) != endptr && *endptr == '\0') { *setme = d; + success = true; } } diff --git a/libtransmission/webseed.c b/libtransmission/webseed.c index 3edde02f3..01409cf54 100644 --- a/libtransmission/webseed.c +++ b/libtransmission/webseed.c @@ -190,7 +190,6 @@ struct connection_succeeded_data static void connection_succeeded(void* vdata) { - tr_torrent* tor; struct connection_succeeded_data* data = vdata; struct tr_webseed* w = data->webseed; @@ -200,15 +199,20 @@ static void connection_succeeded(void* vdata) w->consecutive_failures = w->retry_tickcount = w->retry_challenge = 0; } - if (data->real_url != NULL && (tor = tr_torrentFindFromId(w->session, w->torrent_id)) != NULL) + if (data->real_url != NULL) { - uint64_t file_offset; - tr_file_index_t file_index; + tr_torrent* tor = tr_torrentFindFromId(w->session, w->torrent_id); - tr_ioFindFileLocation(tor, data->piece_index, data->piece_offset, &file_index, &file_offset); - tr_free(w->file_urls[file_index]); - w->file_urls[file_index] = data->real_url; - data->real_url = NULL; + if (tor != NULL) + { + uint64_t file_offset; + tr_file_index_t file_index; + + tr_ioFindFileLocation(tor, data->piece_index, data->piece_offset, &file_index, &file_offset); + tr_free(w->file_urls[file_index]); + w->file_urls[file_index] = data->real_url; + data->real_url = NULL; + } } tr_free(data->real_url);