From 6b0408b320f280c0d5d7fe335469495a3b7a8634 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sun, 8 Nov 2020 13:54:40 -0600 Subject: [PATCH] refactor: fix more sonarcloud warnings (#1509) * chore: simplify loop logic * refactor: simplify isValidUtf8() * refactor: use std::make_unique in Application::Application * refactor: avoid raw pointers in DetailsDialog * refactor: simplify DetailsDialog::refreshPref() * refactor: make Application methods const * refactor: reduce cognitive complexity of buildTrackerSummary() --- gtk/details.c | 267 ++++++++++++++++++------------------- gtk/file-list.c | 99 +++++++------- libtransmission/cache.c | 5 +- libtransmission/file.c | 5 +- libtransmission/makemeta.c | 7 +- libtransmission/torrent.c | 8 +- qt/Application.cc | 65 +++++---- qt/Application.h | 34 ++--- qt/DetailsDialog.cc | 55 +++----- qt/DetailsDialog.h | 9 +- qt/FilterBar.cc | 4 +- qt/InteropObject.cc | 4 +- qt/MainWindow.cc | 2 +- qt/Prefs.cc | 12 +- qt/Torrent.cc | 4 +- qt/Utils.cc | 51 ------- qt/Utils.h | 3 - qt/VariantHelpers.cc | 4 +- utils/remote.c | 20 +-- 19 files changed, 295 insertions(+), 363 deletions(-) diff --git a/gtk/details.c b/gtk/details.c index 883d5b6ff..bc7c8b01b 100644 --- a/gtk/details.c +++ b/gtk/details.c @@ -1650,23 +1650,19 @@ static void refreshWebseedList(struct DetailsImpl* di, tr_torrent** torrents, in for (unsigned int j = 0; j < inf->webseedCount; ++j) { - char buf[128]; - char key[256]; - char const* url = inf->webseeds[j]; + char const* const url = inf->webseeds[j]; + char key[256]; g_snprintf(key, sizeof(key), "%d.%s", tr_torrentId(tor), url); GtkTreeRowReference* const ref = g_hash_table_lookup(hash, key); GtkTreePath* const p = gtk_tree_row_reference_get_path(ref); gtk_tree_model_get_iter(model, &iter, p); + char buf[128] = { 0 }; if (speeds_KBps[j] > 0) { tr_formatter_speed_KBps(buf, speeds_KBps[j], sizeof(buf)); } - else - { - *buf = '\0'; - } gtk_list_store_set(store, &iter, WEBSEED_COL_DOWNLOAD_RATE_DOUBLE, speeds_KBps[j], @@ -1893,11 +1889,6 @@ static void setPeerViewColumns(GtkTreeView* peer_view) sort_col = PEER_COL_ADDRESS_COLLATED; break; - case PEER_COL_CLIENT: - r = gtk_cell_renderer_text_new(); - c = gtk_tree_view_column_new_with_attributes(t, r, "text", col, NULL); - break; - case PEER_COL_PROGRESS: r = gtk_cell_renderer_progress_new(); c = gtk_tree_view_column_new_with_attributes(t, r, "value", PEER_COL_PROGRESS, NULL); @@ -1961,6 +1952,7 @@ static void setPeerViewColumns(GtkTreeView* peer_view) sort_col = PEER_COL_UPLOAD_RATE_DOUBLE; break; + case PEER_COL_CLIENT: case PEER_COL_FLAGS: r = gtk_cell_renderer_text_new(); c = gtk_tree_view_column_new_with_attributes(t, r, "text", col, NULL); @@ -1979,12 +1971,10 @@ static void setPeerViewColumns(GtkTreeView* peer_view) that doesn't look quite correct in any of these columns... so create a non-visible column and assign it as the 'expander column. */ - { - c = gtk_tree_view_column_new(); - gtk_tree_view_column_set_visible(c, FALSE); - gtk_tree_view_append_column(GTK_TREE_VIEW(peer_view), c); - gtk_tree_view_set_expander_column(GTK_TREE_VIEW(peer_view), c); - } + c = gtk_tree_view_column_new(); + gtk_tree_view_column_set_visible(c, FALSE); + gtk_tree_view_append_column(GTK_TREE_VIEW(peer_view), c); + gtk_tree_view_set_expander_column(GTK_TREE_VIEW(peer_view), c); } static void onMorePeerInfoToggled(GtkToggleButton* button, struct DetailsImpl* di) @@ -2090,7 +2080,14 @@ static GtkWidget* peer_page_new(struct DetailsImpl* di) ***** ****/ -/* if it's been longer than a minute, don't bother showing the seconds */ +static char const err_markup_begin[] = ""; +static char const err_markup_end[] = ""; +static char const timeout_markup_begin[] = ""; +static char const timeout_markup_end[] = ""; +static char const success_markup_begin[] = ""; +static char const success_markup_end[] = ""; + +// if it's been longer than a minute, don't bother showing the seconds static void tr_strltime_rounded(char* buf, time_t t, size_t buflen) { if (t > 60) @@ -2101,126 +2098,123 @@ static void tr_strltime_rounded(char* buf, time_t t, size_t buflen) tr_strltime(buf, t, buflen); } -static void buildTrackerSummary(GString* gstr, char const* key, tr_tracker_stat const* st, gboolean showScrape) +static void appendAnnounceInfo(tr_tracker_stat const* const st, time_t const now, GString* const gstr) { - char* str; char timebuf[256]; - time_t const now = time(NULL); - char const* err_markup_begin = ""; - char const* err_markup_end = ""; - char const* timeout_markup_begin = ""; - char const* timeout_markup_end = ""; - char const* success_markup_begin = ""; - char const* success_markup_end = ""; - /* hostname */ + if (st->hasAnnounced && st->announceState != TR_TRACKER_INACTIVE) { - g_string_append(gstr, st->isBackup ? "" : ""); + g_string_append_c(gstr, '\n'); + tr_strltime_rounded(timebuf, now - st->lastAnnounceTime, sizeof(timebuf)); - if (key != NULL) + if (st->lastAnnounceSucceeded) { - str = g_markup_printf_escaped("%s - %s", st->host, key); + g_string_append_printf(gstr, _("Got a list of %1$s%2$'d peers%3$s %4$s ago"), + success_markup_begin, st->lastAnnouncePeerCount, success_markup_end, timebuf); + } + else if (st->lastAnnounceTimedOut) + { + g_string_append_printf(gstr, _("Peer list request %1$stimed out%2$s %3$s ago; will retry"), + timeout_markup_begin, timeout_markup_end, timebuf); } else { - str = g_markup_printf_escaped("%s", st->host); + g_string_append_printf(gstr, _("Got an error %1$s\"%2$s\"%3$s %4$s ago"), err_markup_begin, + st->lastAnnounceResult, err_markup_end, timebuf); } - - g_string_append(gstr, str); - g_free(str); - g_string_append(gstr, st->isBackup ? "" : ""); } + switch (st->announceState) + { + case TR_TRACKER_INACTIVE: + g_string_append_c(gstr, '\n'); + g_string_append(gstr, _("No updates scheduled")); + break; + + case TR_TRACKER_WAITING: + tr_strltime_rounded(timebuf, st->nextAnnounceTime - now, sizeof(timebuf)); + g_string_append_c(gstr, '\n'); + g_string_append_printf(gstr, _("Asking for more peers in %s"), timebuf); + break; + + case TR_TRACKER_QUEUED: + g_string_append_c(gstr, '\n'); + g_string_append(gstr, _("Queued to ask for more peers")); + break; + + case TR_TRACKER_ACTIVE: + tr_strltime_rounded(timebuf, now - st->lastAnnounceStartTime, sizeof(timebuf)); + g_string_append_c(gstr, '\n'); + g_string_append_printf(gstr, _("Asking for more peers now… %s"), timebuf); + break; + } +} + +static void appendScrapeInfo(tr_tracker_stat const* const st, time_t const now, GString* const gstr) +{ + char timebuf[256]; + + if (st->hasScraped) + { + g_string_append_c(gstr, '\n'); + tr_strltime_rounded(timebuf, now - st->lastScrapeTime, sizeof(timebuf)); + + if (st->lastScrapeSucceeded) + { + g_string_append_printf(gstr, _("Tracker had %s%'d seeders and %'d leechers%s %s ago"), success_markup_begin, + st->seederCount, st->leecherCount, success_markup_end, timebuf); + } + else + { + g_string_append_printf(gstr, _("Got a scrape error \"%s%s%s\" %s ago"), err_markup_begin, + st->lastScrapeResult, err_markup_end, timebuf); + } + } + + switch (st->scrapeState) + { + case TR_TRACKER_INACTIVE: + break; + + case TR_TRACKER_WAITING: + g_string_append_c(gstr, '\n'); + tr_strltime_rounded(timebuf, st->nextScrapeTime - now, sizeof(timebuf)); + g_string_append_printf(gstr, _("Asking for peer counts in %s"), timebuf); + break; + + case TR_TRACKER_QUEUED: + g_string_append_c(gstr, '\n'); + g_string_append(gstr, _("Queued to ask for peer counts")); + break; + + case TR_TRACKER_ACTIVE: + g_string_append_c(gstr, '\n'); + tr_strltime_rounded(timebuf, now - st->lastScrapeStartTime, sizeof(timebuf)); + g_string_append_printf(gstr, _("Asking for peer counts now… %s"), timebuf); + break; + } +} + +static void buildTrackerSummary(GString* gstr, char const* key, tr_tracker_stat const* st, gboolean showScrape) +{ + // hostname + g_string_append(gstr, st->isBackup ? "" : ""); + char* const str = key != NULL ? + g_markup_printf_escaped("%s - %s", st->host, key) : + g_markup_printf_escaped("%s", st->host); + g_string_append(gstr, str); + g_free(str); + g_string_append(gstr, st->isBackup ? "" : ""); + if (!st->isBackup) { - if (st->hasAnnounced && st->announceState != TR_TRACKER_INACTIVE) - { - g_string_append_c(gstr, '\n'); - tr_strltime_rounded(timebuf, now - st->lastAnnounceTime, sizeof(timebuf)); + time_t const now = time(NULL); - if (st->lastAnnounceSucceeded) - { - g_string_append_printf(gstr, _("Got a list of %1$s%2$'d peers%3$s %4$s ago"), - success_markup_begin, st->lastAnnouncePeerCount, success_markup_end, timebuf); - } - else if (st->lastAnnounceTimedOut) - { - g_string_append_printf(gstr, _("Peer list request %1$stimed out%2$s %3$s ago; will retry"), - timeout_markup_begin, timeout_markup_end, timebuf); - } - else - { - g_string_append_printf(gstr, _("Got an error %1$s\"%2$s\"%3$s %4$s ago"), err_markup_begin, - st->lastAnnounceResult, err_markup_end, timebuf); - } - } - - switch (st->announceState) - { - case TR_TRACKER_INACTIVE: - g_string_append_c(gstr, '\n'); - g_string_append(gstr, _("No updates scheduled")); - break; - - case TR_TRACKER_WAITING: - tr_strltime_rounded(timebuf, st->nextAnnounceTime - now, sizeof(timebuf)); - g_string_append_c(gstr, '\n'); - g_string_append_printf(gstr, _("Asking for more peers in %s"), timebuf); - break; - - case TR_TRACKER_QUEUED: - g_string_append_c(gstr, '\n'); - g_string_append(gstr, _("Queued to ask for more peers")); - break; - - case TR_TRACKER_ACTIVE: - tr_strltime_rounded(timebuf, now - st->lastAnnounceStartTime, sizeof(timebuf)); - g_string_append_c(gstr, '\n'); - g_string_append_printf(gstr, _("Asking for more peers now… %s"), timebuf); - break; - } + appendAnnounceInfo(st, now, gstr); if (showScrape) { - if (st->hasScraped) - { - g_string_append_c(gstr, '\n'); - tr_strltime_rounded(timebuf, now - st->lastScrapeTime, sizeof(timebuf)); - - if (st->lastScrapeSucceeded) - { - g_string_append_printf(gstr, _("Tracker had %s%'d seeders and %'d leechers%s %s ago"), success_markup_begin, - st->seederCount, st->leecherCount, success_markup_end, timebuf); - } - else - { - g_string_append_printf(gstr, _("Got a scrape error \"%s%s%s\" %s ago"), err_markup_begin, - st->lastScrapeResult, err_markup_end, timebuf); - } - } - - switch (st->scrapeState) - { - case TR_TRACKER_INACTIVE: - break; - - case TR_TRACKER_WAITING: - g_string_append_c(gstr, '\n'); - tr_strltime_rounded(timebuf, st->nextScrapeTime - now, sizeof(timebuf)); - g_string_append_printf(gstr, _("Asking for peer counts in %s"), timebuf); - break; - - case TR_TRACKER_QUEUED: - g_string_append_c(gstr, '\n'); - g_string_append(gstr, _("Queued to ask for peer counts")); - break; - - case TR_TRACKER_ACTIVE: - g_string_append_c(gstr, '\n'); - tr_strltime_rounded(timebuf, now - st->lastScrapeStartTime, sizeof(timebuf)); - g_string_append_printf(gstr, _("Asking for peer counts now… %s"), timebuf); - break; - } + appendScrapeInfo(st, now, gstr); } } } @@ -2354,17 +2348,14 @@ static void refreshTracker(struct DetailsImpl* di, tr_torrent** torrents, int n) if (g_hash_table_lookup(hash, gstr->str) == NULL) { - GtkTreePath* p; - GtkTreeRowReference* ref; - gtk_list_store_insert_with_values(store, &iter, -1, TRACKER_COL_TORRENT_ID, torrent_id, TRACKER_COL_TRACKER_ID, st->id, TRACKER_COL_KEY, gstr->str, -1); - p = gtk_tree_model_get_path(model, &iter); - ref = gtk_tree_row_reference_new(model, p); + GtkTreePath* const p = gtk_tree_model_get_path(model, &iter); + GtkTreeRowReference* ref = gtk_tree_row_reference_new(model, p); g_hash_table_insert(hash, g_strdup(gstr->str), ref); ref = gtk_tree_row_reference_new(model, p); gtr_get_favicon_from_url(session, st->announce, favicon_ready_cb, ref); @@ -2381,15 +2372,13 @@ static void refreshTracker(struct DetailsImpl* di, tr_torrent** torrents, int n) for (int j = 0; j < statCount[i]; ++j) { - GtkTreePath* p; - GtkTreeRowReference* ref; tr_tracker_stat const* st = &stats[i][j]; /* build the key to find the row */ g_string_truncate(gstr, 0); g_string_append_printf(gstr, "%d\t%d\t%s", tr_torrentId(tor), st->tier, st->announce); - ref = g_hash_table_lookup(hash, gstr->str); - p = gtk_tree_row_reference_get_path(ref); + GtkTreeRowReference* const ref = g_hash_table_lookup(hash, gstr->str); + GtkTreePath* const p = gtk_tree_row_reference_get_path(ref); gtk_tree_model_get_iter(model, &iter, p); /* update the row */ @@ -2467,24 +2456,22 @@ static void on_edit_trackers_response(GtkDialog* dialog, int response, gpointer if (response == GTK_RESPONSE_ACCEPT) { - int n; - int tier; - GtkTextIter start; - GtkTextIter end; int const torrent_id = GPOINTER_TO_INT(g_object_get_qdata(G_OBJECT(dialog), TORRENT_ID_KEY)); - GtkTextBuffer* text_buffer = g_object_get_qdata(G_OBJECT(dialog), TEXT_BUFFER_KEY); - tr_torrent* tor = gtr_core_find_torrent(di->core, torrent_id); + GtkTextBuffer* const text_buffer = g_object_get_qdata(G_OBJECT(dialog), TEXT_BUFFER_KEY); + tr_torrent* const tor = gtr_core_find_torrent(di->core, torrent_id); if (tor != NULL) { /* build the array of trackers */ + GtkTextIter start; + GtkTextIter end; gtk_text_buffer_get_bounds(text_buffer, &start, &end); char* const tracker_text = gtk_text_buffer_get_text(text_buffer, &start, &end, FALSE); char** const tracker_strings = g_strsplit(tracker_text, "\n", 0); tr_tracker_info* const trackers = g_new0(tr_tracker_info, g_strv_length(tracker_strings)); - n = 0; - tier = 0; + int n = 0; + int tier = 0; for (int i = 0; tracker_strings[i] != NULL; ++i) { @@ -2934,8 +2921,8 @@ GtkWidget* gtr_torrent_details_dialog_new(GtkWindow* parent, TrCore* core) /* return saved window size */ gtk_window_resize(GTK_WINDOW(d), - gtr_pref_int_get(TR_KEY_details_window_width), - gtr_pref_int_get(TR_KEY_details_window_height)); + (gint)gtr_pref_int_get(TR_KEY_details_window_width), + (gint)gtr_pref_int_get(TR_KEY_details_window_height)); g_signal_connect(d, "size-allocate", G_CALLBACK(on_details_window_size_allocated), NULL); g_signal_connect_swapped(d, "response", G_CALLBACK(gtk_widget_destroy), d); diff --git a/gtk/file-list.c b/gtk/file-list.c index 673c0ff77..9d3dbb59d 100644 --- a/gtk/file-list.c +++ b/gtk/file-list.c @@ -504,14 +504,12 @@ static GNode* find_child(GNode* parent, char const* name) void gtr_file_list_set_torrent(GtkWidget* w, int torrentId) { - GtkTreeStore* store; - FileData* data = g_object_get_data(G_OBJECT(w), "file-data"); - /* unset the old fields */ + FileData* const data = g_object_get_data(G_OBJECT(w), "file-data"); clearData(data); /* instantiate the model */ - store = gtk_tree_store_new(N_FILE_COLS, + GtkTreeStore* const store = gtk_tree_store_new(N_FILE_COLS, GDK_TYPE_PIXBUF, /* icon */ G_TYPE_STRING, /* label */ G_TYPE_STRING, /* label esc */ @@ -528,65 +526,64 @@ void gtr_file_list_set_torrent(GtkWidget* w, int torrentId) data->torrentId = torrentId; /* populate the model */ - if (torrentId > 0) + tr_torrent* const tor = torrentId > 0 ? + gtr_core_find_torrent(data->core, torrentId) : + NULL; + if (tor != NULL) { - tr_torrent* tor = gtr_core_find_torrent(data->core, torrentId); + // build a GNode tree of the files + struct row_struct* const root_data = g_new0(struct row_struct, 1); + root_data->name = g_strdup(tr_torrentName(tor)); + root_data->index = -1; + root_data->length = 0; - if (tor != NULL) + GNode* const root = g_node_new(root_data); + + tr_info const* inf = tr_torrentInfo(tor); + for (tr_file_index_t i = 0; i < inf->fileCount; ++i) { - tr_info const* inf = tr_torrentInfo(tor); - struct row_struct* root_data; - GNode* root; - struct build_data build; + GNode* parent = root; + tr_file const* const file = &inf->files[i]; + char** const tokens = g_strsplit(file->name, G_DIR_SEPARATOR_S, 0); - /* build a GNode tree of the files */ - root_data = g_new0(struct row_struct, 1); - root_data->name = g_strdup(tr_torrentName(tor)); - root_data->index = -1; - root_data->length = 0; - root = g_node_new(root_data); - - for (tr_file_index_t i = 0; i < inf->fileCount; ++i) + for (int j = 0; tokens[j] != NULL; ++j) { - GNode* parent = root; - tr_file const* file = &inf->files[i]; - char** tokens = g_strsplit(file->name, G_DIR_SEPARATOR_S, 0); + gboolean const isLeaf = tokens[j + 1] == NULL; + char const* const name = tokens[j]; + GNode* node = find_child(parent, name); - for (int j = 0; tokens[j] != NULL; ++j) + if (node == NULL) { - gboolean const isLeaf = tokens[j + 1] == NULL; - char const* name = tokens[j]; - GNode* node = find_child(parent, name); - - if (node == NULL) - { - struct row_struct* row = g_new(struct row_struct, 1); - row->name = g_strdup(name); - row->index = isLeaf ? (int)i : -1; - row->length = isLeaf ? file->length : 0; - node = g_node_new(row); - g_node_append(parent, node); - } - - parent = node; + struct row_struct* row = g_new(struct row_struct, 1); + row->name = g_strdup(name); + row->index = isLeaf ? (int)i : -1; + row->length = isLeaf ? file->length : 0; + node = g_node_new(row); + g_node_append(parent, node); } - g_strfreev(tokens); + parent = node; } - /* now, add them to the model */ - build.w = w; - build.tor = tor; - build.store = data->store; - build.iter = NULL; - g_node_children_foreach(root, G_TRAVERSE_ALL, buildTree, &build); - - /* cleanup */ - g_node_destroy(root); - g_free(root_data->name); - g_free(root_data); + g_strfreev(tokens); } + // now, add them to the model + struct build_data build; + build.w = w; + build.tor = tor; + build.store = data->store; + build.iter = NULL; + g_node_children_foreach(root, G_TRAVERSE_ALL, buildTree, &build); + + // cleanup + g_node_destroy(root); + g_free(root_data->name); + g_free(root_data); + } + + if (torrentId > 0) + { refresh(data); data->timeout_tag = gdk_threads_add_timeout_seconds(SECONDARY_WINDOW_REFRESH_INTERVAL_SECONDS, refreshModel, data); } @@ -668,7 +665,7 @@ static char* buildFilename(tr_torrent const* tor, GtkTreeModel* model, GtkTreePa return ret; } -static gboolean onRowActivated(GtkTreeView* view, GtkTreePath* path, GtkTreeViewColumn* col, gpointer gdata) +static gboolean onRowActivated(GtkTreeView* view, GtkTreePath* path, GtkTreeViewColumn const* col, gpointer gdata) { TR_UNUSED(col); diff --git a/libtransmission/cache.c b/libtransmission/cache.c index f15e4e432..384ec4ccf 100644 --- a/libtransmission/cache.c +++ b/libtransmission/cache.c @@ -78,7 +78,7 @@ static int getBlockRun(tr_cache const* cache, int pos, struct run_info* info) tr_block_index_t block = ref->block; int len = 0; - for (int i = pos; i < n; ++i, ++block, ++len) + for (int i = pos; i < n; ++i) { struct cache_block const* b = blocks[i]; @@ -91,6 +91,9 @@ static int getBlockRun(tr_cache const* cache, int pos, struct run_info* info) { break; } + + ++block; + ++len; } if (info != NULL) diff --git a/libtransmission/file.c b/libtransmission/file.c index 14335f70b..a495a743b 100644 --- a/libtransmission/file.c +++ b/libtransmission/file.c @@ -41,13 +41,16 @@ bool tr_sys_file_read_line(tr_sys_file_t handle, char* buffer, size_t buffer_siz int64_t delta = 0; - for (size_t i = 0; i < bytes_read; ++i, ++offset, --buffer_size) + for (size_t i = 0; i < bytes_read; ++i) { if (buffer[offset] == '\n') { delta = i - (int64_t)bytes_read + 1; break; } + + ++offset; + --buffer_size; } if (delta != 0 || buffer_size == 0 || bytes_read == 0) diff --git a/libtransmission/makemeta.c b/libtransmission/makemeta.c index 48af46d03..da92e188f 100644 --- a/libtransmission/makemeta.c +++ b/libtransmission/makemeta.c @@ -169,12 +169,13 @@ tr_metainfo_builder* tr_metaInfoBuilderCreate(char const* topFileArg) ret->files = tr_new0(tr_metainfo_builder_file, ret->fileCount); - for (int i = 0; files != NULL; ++i) + int i = 0; + while (files != NULL) { - struct FileList* tmp = files; + struct FileList* const tmp = files; files = files->next; - tr_metainfo_builder_file* file = &ret->files[i]; + tr_metainfo_builder_file* const file = &ret->files[i++]; file->filename = tmp->filename; file->size = tmp->size; diff --git a/libtransmission/torrent.c b/libtransmission/torrent.c index 202545405..1dd2bc16d 100644 --- a/libtransmission/torrent.c +++ b/libtransmission/torrent.c @@ -1542,11 +1542,13 @@ tr_file_stat* tr_torrentFiles(tr_torrent const* tor, tr_file_index_t* fileCount) tr_file_stat* walk = files; bool const isSeed = tor->completeness == TR_SEED; - for (tr_file_index_t i = 0; i < n; ++i, ++walk) + for (tr_file_index_t i = 0; i < n; ++i) { - uint64_t const b = isSeed ? tor->info.files[i].length : countFileBytesCompleted(tor, i); + uint64_t const length = tor->info.files[i].length; + uint64_t const b = isSeed ? length : countFileBytesCompleted(tor, i); walk->bytesCompleted = b; - walk->progress = tor->info.files[i].length > 0 ? (float)b / tor->info.files[i].length : 1.0F; + walk->progress = length > 0 ? (float)b / (float)length : 1.0F; + ++walk; } if (fileCount != NULL) diff --git a/qt/Application.cc b/qt/Application.cc index a9ec858b6..bc65ae518 100644 --- a/qt/Application.cc +++ b/qt/Application.cc @@ -238,7 +238,7 @@ Application::Application(int& argc, char** argv) : bool const first_time = !dir.exists(QStringLiteral("settings.json")); // initialize the prefs - prefs_ = new Prefs(config_dir); + prefs_ = std::make_unique(config_dir); if (!host.isNull()) { @@ -276,20 +276,21 @@ Application::Application(int& argc, char** argv) : minimized = false; } - session_ = new Session(config_dir, *prefs_); - model_ = new TorrentModel(*prefs_); - window_ = new MainWindow(*session_, *prefs_, *model_, minimized); - watch_dir_ = new WatchDir(*model_); + session_ = std::make_unique(config_dir, *prefs_); + model_ = std::make_unique(*prefs_); + window_ = std::make_unique(*session_, *prefs_, *model_, minimized); + watch_dir_ = std::make_unique(*model_); - connect(model_, &TorrentModel::torrentsAdded, this, &Application::onTorrentsAdded); - connect(model_, &TorrentModel::torrentsCompleted, this, &Application::onTorrentsCompleted); - connect(model_, &TorrentModel::torrentsEdited, this, &Application::onTorrentsEdited); - connect(model_, &TorrentModel::torrentsNeedInfo, this, &Application::onTorrentsNeedInfo); - connect(prefs_, &Prefs::changed, this, &Application::refreshPref); - connect(session_, &Session::sourceChanged, this, &Application::onSessionSourceChanged); - connect(session_, &Session::torrentsRemoved, model_, &TorrentModel::removeTorrents); - connect(session_, &Session::torrentsUpdated, model_, &TorrentModel::updateTorrents); - connect(watch_dir_, &WatchDir::torrentFileAdded, this, qOverload(&Application::addTorrent)); + connect(this, &QCoreApplication::aboutToQuit, this, &Application::saveGeometry); + connect(model_.get(), &TorrentModel::torrentsAdded, this, &Application::onTorrentsAdded); + connect(model_.get(), &TorrentModel::torrentsCompleted, this, &Application::onTorrentsCompleted); + connect(model_.get(), &TorrentModel::torrentsEdited, this, &Application::onTorrentsEdited); + connect(model_.get(), &TorrentModel::torrentsNeedInfo, this, &Application::onTorrentsNeedInfo); + connect(prefs_.get(), &Prefs::changed, this, &Application::refreshPref); + connect(session_.get(), &Session::sourceChanged, this, &Application::onSessionSourceChanged); + connect(session_.get(), &Session::torrentsRemoved, model_.get(), &TorrentModel::removeTorrents); + connect(session_.get(), &Session::torrentsUpdated, model_.get(), &TorrentModel::updateTorrents); + connect(watch_dir_.get(), &WatchDir::torrentFileAdded, this, qOverload(&Application::addTorrent)); // init from preferences for (auto const key : { Prefs::DIR_WATCH }) @@ -304,13 +305,13 @@ Application::Application(int& argc, char** argv) : timer->start(); timer = &stats_timer_; - connect(timer, &QTimer::timeout, session_, &Session::refreshSessionStats); + connect(timer, &QTimer::timeout, session_.get(), &Session::refreshSessionStats); timer->setSingleShot(false); timer->setInterval(STATS_REFRESH_INTERVAL_MSEC); timer->start(); timer = &session_timer_; - connect(timer, &QTimer::timeout, session_, &Session::refreshSessionInfo); + connect(timer, &QTimer::timeout, session_.get(), &Session::refreshSessionInfo); timer->setSingleShot(false); timer->setInterval(SESSION_REFRESH_INTERVAL_MSEC); timer->start(); @@ -329,7 +330,7 @@ Application::Application(int& argc, char** argv) : if (!prefs_->getBool(Prefs::USER_HAS_GIVEN_INFORMED_CONSENT)) { auto* dialog = new QMessageBox(QMessageBox::Information, QString(), - tr("Transmission is a file sharing program."), QMessageBox::Ok | QMessageBox::Cancel, window_); + tr("Transmission is a file sharing program."), QMessageBox::Ok | QMessageBox::Cancel, window_.get()); dialog->setInformativeText(tr("When you run a torrent, its data will be made available to others by means of upload. " "Any content you share is your sole responsibility.")); dialog->button(QMessageBox::Ok)->setText(tr("I &Agree")); @@ -387,7 +388,7 @@ void Application::quitLater() const QTimer::singleShot(0, this, SLOT(quit())); } -void Application::onTorrentsEdited(torrent_ids_t const& ids) +void Application::onTorrentsEdited(torrent_ids_t const& ids) const { // the backend's tr_info has changed, so reload those fields session_->initTorrents(ids); @@ -434,7 +435,7 @@ void Application::onTorrentsCompleted(torrent_ids_t const& ids) const } } -void Application::onTorrentsNeedInfo(torrent_ids_t const& ids) +void Application::onTorrentsNeedInfo(torrent_ids_t const& ids) const { if (!ids.empty()) { @@ -446,7 +447,7 @@ void Application::onTorrentsNeedInfo(torrent_ids_t const& ids) **** ***/ -void Application::consentGiven(int result) +void Application::consentGiven(int result) const { if (result == QMessageBox::Ok) { @@ -458,7 +459,7 @@ void Application::consentGiven(int result) } } -Application::~Application() +void Application::saveGeometry() const { if (prefs_ != nullptr && window_ != nullptr) { @@ -468,19 +469,13 @@ Application::~Application() prefs_->set(Prefs::MAIN_WINDOW_X, geometry.x()); prefs_->set(Prefs::MAIN_WINDOW_Y, geometry.y()); } - - delete watch_dir_; - delete window_; - delete model_; - delete session_; - delete prefs_; } /*** **** ***/ -void Application::refreshPref(int key) +void Application::refreshPref(int key) const { switch (key) { @@ -499,7 +494,7 @@ void Application::refreshPref(int key) } } -void Application::maybeUpdateBlocklist() +void Application::maybeUpdateBlocklist() const { if (!prefs_->getBool(Prefs::BLOCKLIST_UPDATES_ENABLED)) { @@ -517,7 +512,7 @@ void Application::maybeUpdateBlocklist() } } -void Application::onSessionSourceChanged() +void Application::onSessionSourceChanged() const { session_->initTorrents(); session_->refreshSessionStats(); @@ -546,12 +541,12 @@ void Application::refreshTorrents() **** ***/ -void Application::addTorrent(QString const& addme) +void Application::addTorrent(QString const& addme) const { addTorrent(AddData(addme)); } -void Application::addTorrent(AddData const& addme) +void Application::addTorrent(AddData const& addme) const { if (addme.type == addme.NONE) { @@ -564,7 +559,7 @@ void Application::addTorrent(AddData const& addme) } else { - auto* o = new OptionsDialog(*session_, *prefs_, addme, window_); + auto* o = new OptionsDialog(*session_, *prefs_, addme, window_.get()); o->show(); } @@ -575,9 +570,9 @@ void Application::addTorrent(AddData const& addme) **** ***/ -void Application::raise() +void Application::raise() const { - alert(window_); + alert(window_.get()); } bool Application::notifyApp(QString const& title, QString const& body) const diff --git a/qt/Application.h b/qt/Application.h index 5eb3ba1c7..36494f6e5 100644 --- a/qt/Application.h +++ b/qt/Application.h @@ -8,6 +8,7 @@ #pragma once +#include #include #include @@ -34,39 +35,39 @@ class Application : public QApplication public: Application(int& argc, char** argv); - ~Application() override; - void raise(); + void raise() const; bool notifyApp(QString const& title, QString const& body) const; QString const& intern(QString const& in) { return *interned_strings_.insert(in).first; } FaviconCache& faviconCache(); public slots: - void addTorrent(AddData const&); - void addTorrent(QString const&); + void addTorrent(AddData const&) const; + void addTorrent(QString const&) const; private slots: - void consentGiven(int result); - void onSessionSourceChanged(); + void consentGiven(int result) const; + void onSessionSourceChanged() const; void onTorrentsAdded(torrent_ids_t const& torrents) const; void onTorrentsCompleted(torrent_ids_t const& torrents) const; - void onTorrentsEdited(torrent_ids_t const& torrents); - void onTorrentsNeedInfo(torrent_ids_t const& torrents); - void refreshPref(int key); + void onTorrentsEdited(torrent_ids_t const& torrents) const; + void onTorrentsNeedInfo(torrent_ids_t const& torrents) const; + void refreshPref(int key) const; void refreshTorrents(); + void saveGeometry() const; private: - void maybeUpdateBlocklist(); + void maybeUpdateBlocklist() const; void loadTranslations(); QStringList getNames(torrent_ids_t const& ids) const; void quitLater() const; - Prefs* prefs_ = {}; - Session* session_ = {}; - TorrentModel* model_ = {}; - MainWindow* window_ = {}; - WatchDir* watch_dir_ = {}; + std::unique_ptr prefs_; + std::unique_ptr session_; + std::unique_ptr model_; + std::unique_ptr window_; + std::unique_ptr watch_dir_; QTimer model_timer_; QTimer stats_timer_; QTimer session_timer_; @@ -81,5 +82,4 @@ private: std::unordered_set interned_strings_; }; -#undef qApp -#define qApp static_cast(Application::instance()) +#define trApp static_cast(Application::instance()) diff --git a/qt/DetailsDialog.cc b/qt/DetailsDialog.cc index 89769fea3..b88d811d2 100644 --- a/qt/DetailsDialog.cc +++ b/qt/DetailsDialog.cc @@ -253,13 +253,6 @@ DetailsDialog::DetailsDialog(Session& session, Prefs& prefs, TorrentModel const& ui_debounce_timer_.setSingleShot(true); } -DetailsDialog::~DetailsDialog() -{ - tracker_delegate_->deleteLater(); - tracker_filter_->deleteLater(); - tracker_model_->deleteLater(); -} - void DetailsDialog::setIds(torrent_ids_t const& ids) { if (ids != ids_) @@ -278,29 +271,18 @@ void DetailsDialog::setIds(torrent_ids_t const& ids) void DetailsDialog::refreshPref(int key) { - QString str; - - switch (key) + if (key == Prefs::SHOW_TRACKER_SCRAPES) + { + auto* selection_model = ui_.trackersView->selectionModel(); + tracker_delegate_->setShowMore(prefs_.getBool(key)); + selection_model->clear(); + ui_.trackersView->reset(); + selection_model->select(selection_model->selection(), QItemSelectionModel::Select); + selection_model->setCurrentIndex(selection_model->currentIndex(), QItemSelectionModel::NoUpdate); + } + else if (key == Prefs::SHOW_BACKUP_TRACKERS) { - case Prefs::SHOW_TRACKER_SCRAPES: - { - QItemSelectionModel* selection_model(ui_.trackersView->selectionModel()); - QItemSelection const selection(selection_model->selection()); - QModelIndex const current_index(selection_model->currentIndex()); - tracker_delegate_->setShowMore(prefs_.getBool(key)); - selection_model->clear(); - ui_.trackersView->reset(); - selection_model->select(selection, QItemSelectionModel::Select); - selection_model->setCurrentIndex(current_index, QItemSelectionModel::NoUpdate); - break; - } - - case Prefs::SHOW_BACKUP_TRACKERS: tracker_filter_->setShowBackupTrackers(prefs_.getBool(key)); - break; - - default: - break; } } @@ -1449,13 +1431,18 @@ void DetailsDialog::initOptionsTab() void DetailsDialog::initTrackerTab() { - tracker_model_ = new TrackerModel(); - tracker_filter_ = new TrackerModelFilter(); - tracker_filter_->setSourceModel(tracker_model_); - tracker_delegate_ = new TrackerDelegate(); + auto deleter = [](QObject* o) { o->deleteLater(); }; - ui_.trackersView->setModel(tracker_filter_); - ui_.trackersView->setItemDelegate(tracker_delegate_); + // NOLINTNEXTLINE(modernize-make-shared) no custom deleters in make_shared + tracker_model_.reset(new TrackerModel, deleter); + // NOLINTNEXTLINE(modernize-make-shared) no custom deleters in make_shared + tracker_filter_.reset(new TrackerModelFilter, deleter); + tracker_filter_->setSourceModel(tracker_model_.get()); + // NOLINTNEXTLINE(modernize-make-shared) no custom deleters in make_shared + tracker_delegate_.reset(new TrackerDelegate, deleter); + + ui_.trackersView->setModel(tracker_filter_.get()); + ui_.trackersView->setItemDelegate(tracker_delegate_.get()); ui_.addTrackerButton->setIcon(getStockIcon(QStringLiteral("list-add"), QStyle::SP_DialogOpenButton)); ui_.editTrackerButton->setIcon(getStockIcon(QStringLiteral("document-properties"), QStyle::SP_DesktopIcon)); diff --git a/qt/DetailsDialog.h b/qt/DetailsDialog.h index 1cd529ec7..e63a37777 100644 --- a/qt/DetailsDialog.h +++ b/qt/DetailsDialog.h @@ -8,6 +8,8 @@ #pragma once +#include + #include #include #include @@ -37,7 +39,6 @@ class DetailsDialog : public BaseDialog public: DetailsDialog(Session&, Prefs&, TorrentModel const&, QWidget* parent = nullptr); - ~DetailsDialog() override; void setIds(torrent_ids_t const& ids); @@ -127,9 +128,9 @@ private: QTimer model_timer_; QTimer ui_debounce_timer_; - TrackerModel* tracker_model_ = {}; - TrackerModelFilter* tracker_filter_ = {}; - TrackerDelegate* tracker_delegate_ = {}; + std::shared_ptr tracker_model_; + std::shared_ptr tracker_filter_; + std::shared_ptr tracker_delegate_; QMap peers_; diff --git a/qt/FilterBar.cc b/qt/FilterBar.cc index 3024976a7..56991dbc2 100644 --- a/qt/FilterBar.cc +++ b/qt/FilterBar.cc @@ -132,7 +132,7 @@ void FilterBar::refreshTrackers() auto const& key = it->first; auto const& display_name = FaviconCache::getDisplayName(key); auto const& count = it->second; - auto const icon = qApp->faviconCache().find(key); + auto const icon = trApp->faviconCache().find(key); i->setData(display_name, Qt::DisplayRole); i->setData(display_name, TRACKER_ROLE); @@ -244,7 +244,7 @@ FilterBar::FilterBar(Prefs& prefs, TorrentModel const& torrents, TorrentFilter c connect(&torrents_, &TorrentModel::rowsRemoved, this, &FilterBar::recountAllSoon); connect(&torrents_, &TorrentModel::torrentsChanged, this, &FilterBar::onTorrentsChanged); connect(&recount_timer_, &QTimer::timeout, this, &FilterBar::recount); - connect(&qApp->faviconCache(), &FaviconCache::pixmapReady, this, &FilterBar::recountTrackersSoon); + connect(&trApp->faviconCache(), &FaviconCache::pixmapReady, this, &FilterBar::recountTrackersSoon); recountAllSoon(); is_bootstrapping_ = false; diff --git a/qt/InteropObject.cc b/qt/InteropObject.cc index 5f0e5bd1f..7f00443c0 100644 --- a/qt/InteropObject.cc +++ b/qt/InteropObject.cc @@ -18,7 +18,7 @@ InteropObject::InteropObject(QObject* parent) : // NOLINTNEXTLINE(readability-identifier-naming) bool InteropObject::PresentWindow() { - qApp->raise(); + trApp->raise(); return true; } @@ -29,7 +29,7 @@ bool InteropObject::AddMetainfo(QString const& metainfo) if (addme.type != addme.NONE) { - qApp->addTorrent(addme); + trApp->addTorrent(addme); } return true; diff --git a/qt/MainWindow.cc b/qt/MainWindow.cc index 9af4fc3e8..06cca722e 100644 --- a/qt/MainWindow.cc +++ b/qt/MainWindow.cc @@ -1592,7 +1592,7 @@ void MainWindow::dropEvent(QDropEvent* event) key = url.toLocalFile(); } - qApp->addTorrent(AddData(key)); + trApp->addTorrent(AddData(key)); } } } diff --git a/qt/Prefs.cc b/qt/Prefs.cc index 04130e24c..0fffc859e 100644 --- a/qt/Prefs.cc +++ b/qt/Prefs.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -20,7 +21,6 @@ #include "CustomVariantType.h" #include "Prefs.h" -#include "Utils.h" #include "VariantHelpers.h" using ::trqt::variant_helpers::dictAdd; @@ -155,6 +155,14 @@ auto const SortModes = std::array, SortMode::NU { SortMode::SORT_BY_ID, "sort-by-id" } }}; +bool isValidUtf8(QByteArray const& byteArray) +{ + auto const* const codec = QTextCodec::codecForName("UTF-8"); + auto state = QTextCodec::ConverterState {}; + auto const text = codec->toUnicode(byteArray.constData(), byteArray.size(), &state); + return state.invalidChars == 0; +} + } // namespace /*** @@ -433,7 +441,7 @@ QString Prefs::getString(int key) const assert(Items[key].type == QVariant::String); QByteArray const b = values_[key].toByteArray(); - if (Utils::isValidUtf8(b.constData())) + if (isValidUtf8(b.constData())) { values_[key].setValue(QString::fromUtf8(b.constData())); } diff --git a/qt/Torrent.cc b/qt/Torrent.cc index 3f7652ac7..0fac3438c 100644 --- a/qt/Torrent.cc +++ b/qt/Torrent.cc @@ -250,7 +250,7 @@ Torrent::fields_t Torrent::update(tr_quark const* keys, tr_variant const* const* field_changed = change(field ## _, child); \ if (field_changed) \ { \ - field ## _ = qApp->intern(field ## _); \ + field ## _ = trApp->intern(field ## _); \ } \ changed.set(bit, field_changed); \ break; @@ -370,5 +370,5 @@ QString Torrent::getError() const QPixmap TrackerStat::getFavicon() const { - return qApp->faviconCache().find(favicon_key); + return trApp->faviconCache().find(favicon_key); } diff --git a/qt/Utils.cc b/qt/Utils.cc index 11e24063a..457fc13dd 100644 --- a/qt/Utils.cc +++ b/qt/Utils.cc @@ -71,57 +71,6 @@ QIcon Utils::getIconFromIndex(QModelIndex const& index) } } -bool Utils::isValidUtf8(char const* s) -{ - int n; // number of bytes in a UTF-8 sequence - - for (char const* c = s; *c != '\0'; c += n) - { - if ((*c & 0x80) == 0x00) - { - n = 1; // ASCII - } - else if ((*c & 0xc0) == 0x80) - { // NOLINT(bugprone-branch-clone) - return false; // not valid - } - else if ((*c & 0xe0) == 0xc0) - { - n = 2; - } - else if ((*c & 0xf0) == 0xe0) - { - n = 3; - } - else if ((*c & 0xf8) == 0xf0) - { - n = 4; - } - else if ((*c & 0xfc) == 0xf8) - { - n = 5; - } - else if ((*c & 0xfe) == 0xfc) - { - n = 6; - } - else - { - return false; - } - - for (int m = 1; m < n; m++) - { - if ((c[m] & 0xc0) != 0x80) - { - return false; - } - } - } - - return true; -} - QString Utils::removeTrailingDirSeparator(QString const& path) { int i = path.size(); diff --git a/qt/Utils.h b/qt/Utils.h index 48868d326..4e500e76e 100644 --- a/qt/Utils.h +++ b/qt/Utils.h @@ -45,9 +45,6 @@ class Utils public: static QIcon getIconFromIndex(QModelIndex const& index); - // Test if string is UTF-8 or not - static bool isValidUtf8(char const* s); - static QString removeTrailingDirSeparator(QString const& path); static void narrowRect(QRect& rect, int dx1, int dx2, Qt::LayoutDirection direction) diff --git a/qt/VariantHelpers.cc b/qt/VariantHelpers.cc index f40c1422a..4038df976 100644 --- a/qt/VariantHelpers.cc +++ b/qt/VariantHelpers.cc @@ -165,8 +165,8 @@ bool change(TrackerStat& setme, tr_variant const* value) { if (key == TR_KEY_announce) { - setme.announce = qApp->intern(setme.announce); - setme.favicon_key = qApp->faviconCache().add(setme.announce); + setme.announce = trApp->intern(setme.announce); + setme.favicon_key = trApp->faviconCache().add(setme.announce); } changed = true; diff --git a/utils/remote.c b/utils/remote.c index bacf5e6dc..bb58899e4 100644 --- a/utils/remote.c +++ b/utils/remote.c @@ -1368,22 +1368,24 @@ static void printPeers(tr_variant* top) } } -static void printPiecesImpl(uint8_t const* raw, size_t rawlen, size_t j) +static void printPiecesImpl(uint8_t const* raw, size_t raw_len, size_t piece_count) { - size_t len; - char* str = tr_base64_decode(raw, rawlen, &len); + size_t len = 0; + char* const str = tr_base64_decode(raw, raw_len, &len); printf(" "); - for (size_t i = 0, k = 0; k < len; ++k) + size_t piece = 0; + size_t const col_width = 64; + for (char const* it = str, * end = it + len; it != end; ++it) { - for (int e = 0; i < j && e < 8; ++e, ++i) + for (int bit = 0; piece < piece_count && bit < 8; ++bit, ++piece) { - printf("%c", (str[k] & (1 << (7 - e))) != 0 ? '1' : '0'); + printf("%c", (*it & (1 << (7 - bit))) != 0 ? '1' : '0'); } printf(" "); - if (i % 64 == 0) + if (piece % col_width == 0) { printf("\n "); } @@ -1529,10 +1531,10 @@ static void printTorrentList(tr_variant* top) static void printTrackersImpl(tr_variant* trackerStats) { char buf[512]; - tr_variant* t; - for (int i = 0; (t = tr_variantListChild(trackerStats, i)) != NULL; ++i) + for (size_t i = 0, n = tr_variantListSize(trackerStats); i < n; ++i) { + tr_variant* const t = tr_variantListChild(trackerStats, i); int64_t downloadCount; bool hasAnnounced; bool hasScraped;