From 2321bc3fadeefd9318310eda5d7c17a207fedeab Mon Sep 17 00:00:00 2001 From: Mike Gelfand Date: Sat, 9 May 2015 08:37:55 +0000 Subject: [PATCH] Fix some issues revealed by coverity --- gtk/conf.c | 3 ++- gtk/tr-prefs.c | 6 +----- gtk/util.c | 19 ++++++++++++------- libtransmission/announcer.c | 2 +- libtransmission/peer-mgr.c | 5 ++++- libtransmission/peer-msgs.c | 2 +- libtransmission/rpc-server.c | 15 +++++++++++---- libtransmission/torrent-magnet.c | 2 ++ libtransmission/torrent.c | 8 ++++++-- libtransmission/trevent.c | 3 +++ libtransmission/upnp.c | 2 +- qt/details.cc | 3 +++ qt/freespace-label.cc | 3 +-- qt/torrent-filter.cc | 1 + 14 files changed, 49 insertions(+), 25 deletions(-) diff --git a/gtk/conf.c b/gtk/conf.c index 6302d4738..6803fb15a 100644 --- a/gtk/conf.c +++ b/gtk/conf.c @@ -172,7 +172,8 @@ gtr_pref_flag_get (const tr_quark key) { bool boolVal; - tr_variantDictFindBool (getPrefs (), key, &boolVal); + if (!tr_variantDictFindBool (getPrefs (), key, &boolVal)) + boolVal = false; return boolVal != 0; } diff --git a/gtk/tr-prefs.c b/gtk/tr-prefs.c index 1298c5745..0e14a87ab 100644 --- a/gtk/tr-prefs.c +++ b/gtk/tr-prefs.c @@ -970,11 +970,7 @@ new_time_combo (GObject * core, const tr_quark key) { char buf[128]; GtkTreeIter iter; - struct tm tm; - tm.tm_hour = i / 60; - tm.tm_min = i % 60; - tm.tm_sec = 0; - strftime (buf, sizeof (buf), "%H:%M", &tm); + g_snprintf (buf, sizeof (buf), "%02d:%02d", i / 60, i % 60); gtk_list_store_append (store, &iter); gtk_list_store_set (store, &iter, 0, i, 1, buf, -1); } diff --git a/gtk/util.c b/gtk/util.c index 838393e2c..6f96ec583 100644 --- a/gtk/util.c +++ b/gtk/util.c @@ -431,20 +431,25 @@ gtr_combo_box_new_enum (const char * text_1, ...) GtkWidget * w; GtkCellRenderer * r; GtkListStore * store; - va_list vl; const char * text; - va_start (vl, text_1); store = gtk_list_store_new (2, G_TYPE_INT, G_TYPE_STRING); text = text_1; - if (text != NULL) do + if (text != NULL) { - const int val = va_arg (vl, int); - gtk_list_store_insert_with_values (store, NULL, INT_MAX, 0, val, 1, text, -1); - text = va_arg (vl, const char *); + va_list vl; + + va_start (vl, text_1); + do + { + const int val = va_arg (vl, int); + gtk_list_store_insert_with_values (store, NULL, INT_MAX, 0, val, 1, text, -1); + text = va_arg (vl, const char *); + } + while (text != NULL); + va_end (vl); } - while (text != NULL); w = gtk_combo_box_new_with_model (GTK_TREE_MODEL (store)); r = gtk_cell_renderer_text_new (); diff --git a/libtransmission/announcer.c b/libtransmission/announcer.c index 00cbc8140..f5b79a753 100644 --- a/libtransmission/announcer.c +++ b/libtransmission/announcer.c @@ -1145,7 +1145,7 @@ on_announce_done (const tr_announce_response * response, /* if the tracker included scrape fields in its announce response, then a separate scrape isn't needed */ - if ((scrape_fields >= 3) || (!tracker->scrape && (scrape_fields >= 1))) + if (scrape_fields >= 3 || (scrape_fields >= 1 && tracker->scrape != NULL)) { tr_logAddTorDbg (tier->tor, "Announce response contained scrape info; " "rescheduling next scrape to %d seconds from now.", diff --git a/libtransmission/peer-mgr.c b/libtransmission/peer-mgr.c index c490facc2..642de1274 100644 --- a/libtransmission/peer-mgr.c +++ b/libtransmission/peer-mgr.c @@ -877,7 +877,7 @@ testForEndgame (const tr_swarm * s) { /* we consider ourselves to be in endgame if the number of bytes we've got requested is >= the number of bytes left to download */ - return (s->requestCount * s->tor->blockSize) + return ((uint64_t) s->requestCount * s->tor->blockSize) >= tr_torrentGetLeftUntilDone (s->tor); } @@ -1994,6 +1994,9 @@ myHandshakeDoneCB (tr_handshake * handshake, ensureAtomExists (s, addr, port, 0, -1, TR_PEER_FROM_INCOMING); atom = getExistingAtom (s, addr); + + assert (atom != NULL); + atom->time = tr_time (); atom->piece_data_time = 0; atom->lastConnectionAt = tr_time (); diff --git a/libtransmission/peer-msgs.c b/libtransmission/peer-msgs.c index 17dcd3896..36fe7313b 100644 --- a/libtransmission/peer-msgs.c +++ b/libtransmission/peer-msgs.c @@ -1951,7 +1951,7 @@ fillOutputBuffer (tr_peerMsgs * msgs, time_t now) bool ok = false; data = tr_torrentGetMetadataPiece (msgs->torrent, piece, &dataLen); - if ((dataLen > 0) && (data != NULL)) + if (data != NULL) { tr_variant tmp; struct evbuffer * payload; diff --git a/libtransmission/rpc-server.c b/libtransmission/rpc-server.c index 634e633b8..6064d820b 100644 --- a/libtransmission/rpc-server.c +++ b/libtransmission/rpc-server.c @@ -500,7 +500,7 @@ handle_web_client (struct evhttp_request * req, char * filename = tr_strdup_printf ("%s%s%s", webClientDir, TR_PATH_DELIMITER_STR, - subpath && *subpath ? subpath : "index.html"); + *subpath != '\0' ? subpath : "index.html"); serve_file (req, server, filename); tr_free (filename); } @@ -613,10 +613,17 @@ handle_request (struct evhttp_request * req, void * arg) { size_t plen; char * p = tr_base64_decode_str (auth + 6, &plen); - if (p && plen && ((pass = strchr (p, ':')))) + if (p != NULL) { - user = p; - *pass++ = '\0'; + if (plen > 0 && (pass = strchr (p, ':')) != NULL) + { + user = p; + *pass++ = '\0'; + } + else + { + tr_free (p); + } } } diff --git a/libtransmission/torrent-magnet.c b/libtransmission/torrent-magnet.c index fd71cd75c..8b40bc1d2 100644 --- a/libtransmission/torrent-magnet.c +++ b/libtransmission/torrent-magnet.c @@ -192,6 +192,8 @@ tr_torrentGetMetadataPiece (tr_torrent * tor, int piece, int * len) } } + assert (ret == NULL || *len > 0); + return ret; } diff --git a/libtransmission/torrent.c b/libtransmission/torrent.c index 03ff37b2c..8c33b99ea 100644 --- a/libtransmission/torrent.c +++ b/libtransmission/torrent.c @@ -2093,11 +2093,15 @@ onSigCHLD (int i UNUSED) static void torrentCallScript (const tr_torrent * tor, const char * script) { - char timeStr[128]; + char timeStr[128], * newlinePos; const time_t now = tr_time (); tr_strlcpy (timeStr, ctime (&now), sizeof (timeStr)); - *strchr (timeStr,'\n') = '\0'; + + /* ctime () includes '\n', but it's better to be safe */ + newlinePos = strchr (timeStr, '\n'); + if (newlinePos != NULL) + *newlinePos = '\0'; if (script && *script) { diff --git a/libtransmission/trevent.c b/libtransmission/trevent.c index e70218834..7c2ae089c 100644 --- a/libtransmission/trevent.c +++ b/libtransmission/trevent.c @@ -284,6 +284,9 @@ tr_eventClose (tr_session * session) { assert (tr_isSession (session)); + if (session->events == NULL) + return; + session->events->die = true; tr_logAddDeep (__FILE__, __LINE__, NULL, "closing trevent pipe"); tr_netCloseSocket (session->events->fds[1]); diff --git a/libtransmission/upnp.c b/libtransmission/upnp.c index 35b0d153f..59d4ed773 100644 --- a/libtransmission/upnp.c +++ b/libtransmission/upnp.c @@ -282,7 +282,7 @@ tr_upnpPulse (tr_upnp * handle, int err_udp = -1; errno = 0; - if (!handle->urls.controlURL || !handle->data.first.servicetype) + if (!handle->urls.controlURL) handle->isMapped = 0; else { diff --git a/qt/details.cc b/qt/details.cc index ba5217d3b..25daa3471 100644 --- a/qt/details.cc +++ b/qt/details.cc @@ -112,6 +112,9 @@ class PeerItem: public QTreeWidgetItem const PeerItem * i = dynamic_cast (&other); QTreeWidget * tw (treeWidget ()); const int column = tw ? tw->sortColumn () : 0; + + assert (i != nullptr); + switch (column) { case COL_UP: return peer.rateToPeer < i->peer.rateToPeer; diff --git a/qt/freespace-label.cc b/qt/freespace-label.cc index 16ce02f15..aa76b0212 100644 --- a/qt/freespace-label.cc +++ b/qt/freespace-label.cc @@ -89,8 +89,7 @@ FreespaceLabel::onSessionExecuted (int64_t tag, const QString& result, tr_varian // update the label int64_t bytes = -1; - tr_variantDictFindInt (arguments, TR_KEY_size_bytes, &bytes); - if (bytes >= 0) + if (tr_variantDictFindInt (arguments, TR_KEY_size_bytes, &bytes) && bytes >= 0) setText (tr("%1 free").arg(Formatter::sizeToString (bytes))); else setText (QString ()); diff --git a/qt/torrent-filter.cc b/qt/torrent-filter.cc index 6d50fc74d..53c568554 100644 --- a/qt/torrent-filter.cc +++ b/qt/torrent-filter.cc @@ -132,6 +132,7 @@ TorrentFilter::lessThan (const QModelIndex& left, const QModelIndex& right) cons val = a->compareSeedRatio (*b); if (!val) val = -compare (a->queuePosition(), b->queuePosition()); + // fall through case SortMode::SORT_BY_RATIO: if (!val)