From 7d7f2d58a8d00619ef49be1a42b372baac8c0363 Mon Sep 17 00:00:00 2001 From: Pablo Curiel Date: Thu, 18 Apr 2024 10:46:29 +0200 Subject: [PATCH] [ci skip] UI code quality improvements * Always use value references for periodic Borealis tasks. * Always use nullptr instead of NULL in C++ headers and modules (except when dealing with functions with C name mangling). * Update StatusInfoData to use a char array for the IP address instead of directly using the pointer returned by inet_ntoa(). --- include/download_task.hpp | 2 +- include/dump_options_frame.hpp | 10 +++---- include/options_tab.hpp | 2 +- include/root_view.hpp | 6 ++-- include/tasks.hpp | 39 ++++++++++++------------- include/titles_tab.hpp | 10 +++---- source/root_view.cpp | 19 +++++++------ source/tasks.cpp | 52 ++++++++++++++++------------------ source/titles_tab.cpp | 16 +++++------ 9 files changed, 78 insertions(+), 78 deletions(-) diff --git a/include/download_task.hpp b/include/download_task.hpp index 78a1e03..7ac49df 100644 --- a/include/download_task.hpp +++ b/include/download_task.hpp @@ -103,7 +103,7 @@ namespace nxdt::tasks char *buf = nullptr; size_t buf_size = 0; - /* If the process fails or if it's cancelled, httpDownloadData() will take care of freeing up the allocated memory and return NULL. */ + /* If the process fails or if it's cancelled, httpDownloadData() will take care of freeing up the allocated memory and returning NULL. */ buf = httpDownloadData(&buf_size, url.c_str(), force_https, DownloadDataTask::HttpProgressCallback, this); return std::make_pair(buf, buf_size); diff --git a/include/dump_options_frame.hpp b/include/dump_options_frame.hpp index 44d3ab4..6ec833e 100644 --- a/include/dump_options_frame.hpp +++ b/include/dump_options_frame.hpp @@ -70,13 +70,13 @@ namespace nxdt::views return output; } - void UpdateStorages(const nxdt::tasks::UmsDeviceVector* ums_devices) + void UpdateStorages(const nxdt::tasks::UmsDeviceVector& ums_devices) { if (!this->output_storage_item) return; std::vector storages{}; - size_t elem_count = (ConfigOutputStorage_Count + ums_devices->size()); + size_t elem_count = (ConfigOutputStorage_Count + ums_devices.size()); u32 selected = this->output_storage_item->getSelectedValue(); /* Fill storages vector. */ @@ -91,7 +91,7 @@ namespace nxdt::views u64 total_sz = 0, free_sz = 0; char total_sz_str[64] = {0}, free_sz_str[64] = {0}; - const UsbHsFsDevice *cur_ums_device = (i >= ConfigOutputStorage_Count ? (ums_devices->data() + (i - ConfigOutputStorage_Count)) : nullptr); + const UsbHsFsDevice *cur_ums_device = (i >= ConfigOutputStorage_Count ? &(ums_devices.at(i - ConfigOutputStorage_Count)) : nullptr); sprintf(total_sz_str, "%s/", cur_ums_device ? cur_ums_device->name : DEVOPTAB_SDMC_DEVICE); utilsGetFileSystemStatsByPath(total_sz_str, &total_sz, &free_sz); @@ -175,7 +175,7 @@ namespace nxdt::views /* Subscribe to SelectListItem's value selected event. */ this->output_storage_item->getValueSelectedEvent()->subscribe([this](int selected) { /* Make sure the current value isn't out of bounds. */ - if (selected < ConfigOutputStorage_SdCard || selected >= static_cast(this->root_view->GetUmsDevices()->size() + ConfigOutputStorage_Count)) return; + if (selected < ConfigOutputStorage_SdCard || selected >= static_cast(this->root_view->GetUmsDevices().size() + ConfigOutputStorage_Count)) return; /* Update configuration. */ if (selected == ConfigOutputStorage_SdCard || selected == ConfigOutputStorage_UsbHost) configSetInteger("output_storage", selected); @@ -195,7 +195,7 @@ namespace nxdt::views /* Subscribe to the UMS device event. */ - this->ums_task_sub = this->root_view->RegisterUmsTaskListener([this](const nxdt::tasks::UmsDeviceVector* ums_devices) { + this->ums_task_sub = this->root_view->RegisterUmsTaskListener([this](const nxdt::tasks::UmsDeviceVector& ums_devices) { /* Update output storages vector. */ this->UpdateStorages(ums_devices); }); diff --git a/include/options_tab.hpp b/include/options_tab.hpp index 3c3f6f2..312a7f2 100644 --- a/include/options_tab.hpp +++ b/include/options_tab.hpp @@ -46,7 +46,7 @@ namespace nxdt::views { private: nxdt::tasks::DownloadDataTask json_task; - char *json_buf = NULL; + char *json_buf = nullptr; size_t json_buf_size = 0; UtilsGitHubReleaseJsonData json_data = {0}; diff --git a/include/root_view.hpp b/include/root_view.hpp index bf762ae..4797c69 100644 --- a/include/root_view.hpp +++ b/include/root_view.hpp @@ -72,19 +72,19 @@ namespace nxdt::views return this->status_info_task->IsInternetConnectionAvailable(); } - ALWAYS_INLINE const nxdt::tasks::TitleApplicationMetadataVector* GetApplicationMetadata(bool is_system) + ALWAYS_INLINE const nxdt::tasks::TitleApplicationMetadataVector& GetApplicationMetadata(bool is_system) { return this->title_task->GetApplicationMetadata(is_system); } - ALWAYS_INLINE const nxdt::tasks::UmsDeviceVector* GetUmsDevices(void) + ALWAYS_INLINE const nxdt::tasks::UmsDeviceVector& GetUmsDevices(void) { return this->ums_task->GetUmsDevices(); } EVENT_SUBSCRIPTION(StatusInfoTask, StatusInfoEvent, status_info_task); EVENT_SUBSCRIPTION(GameCardTask, GameCardStatusEvent, gc_status_task); - EVENT_SUBSCRIPTION(TitleTask, TitleEvent, title_task); + EVENT_SUBSCRIPTION(TitleTask, UserTitleEvent, title_task); EVENT_SUBSCRIPTION(UmsTask, UmsEvent, ums_task); EVENT_SUBSCRIPTION(UsbHostTask, UsbHostEvent, usb_host_task); }; diff --git a/include/tasks.hpp b/include/tasks.hpp index ddb0ccb..c5b5ceb 100644 --- a/include/tasks.hpp +++ b/include/tasks.hpp @@ -44,8 +44,9 @@ namespace nxdt::tasks struct tm timeinfo; u32 charge_percentage; PsmChargerType charger_type; + bool connected; NifmInternetConnectionType connection_type; - char *ip_addr; + char ip_addr[16]; } StatusInfoData; /* Used to hold pointers to application metadata entries. */ @@ -55,19 +56,19 @@ namespace nxdt::tasks typedef std::vector UmsDeviceVector; /* Custom event types. */ - typedef brls::Event StatusInfoEvent; - typedef brls::Event GameCardStatusEvent; - typedef brls::Event TitleEvent; - typedef brls::Event UmsEvent; - typedef brls::Event UsbHostEvent; + typedef brls::Event StatusInfoEvent; + typedef brls::Event GameCardStatusEvent; + typedef brls::Event UserTitleEvent; + typedef brls::Event UmsEvent; + typedef brls::Event UsbHostEvent; /* Status info task. */ - /* Its event returns a pointer to a StatusInfoData struct. */ + /* Its event returns a reference to a StatusInfoData struct. */ class StatusInfoTask: public brls::RepeatingTask { private: StatusInfoEvent status_info_event; - StatusInfoData status_info_data = {0}; + StatusInfoData status_info_data{}; protected: void run(retro_time_t current_time) override; @@ -102,14 +103,14 @@ namespace nxdt::tasks }; /* Title task. */ - /* Its event returns a pointer to a TitleApplicationMetadataVector with metadata for user titles (system titles don't change at runtime). */ + /* Its event returns a reference to a TitleApplicationMetadataVector with metadata for user titles (system titles don't change at runtime). */ class TitleTask: public brls::RepeatingTask { private: - TitleEvent title_event; + UserTitleEvent user_title_event; - TitleApplicationMetadataVector system_metadata; - TitleApplicationMetadataVector user_metadata; + TitleApplicationMetadataVector system_metadata{}; + TitleApplicationMetadataVector user_metadata{}; void PopulateApplicationMetadataVector(bool is_system); @@ -120,19 +121,19 @@ namespace nxdt::tasks TitleTask(void); ~TitleTask(void); - /* Intentionally left here to let views retrieve title metadata. */ - const TitleApplicationMetadataVector* GetApplicationMetadata(bool is_system); + /* Intentionally left here to let views retrieve title metadata on-demand. */ + const TitleApplicationMetadataVector& GetApplicationMetadata(bool is_system); - EVENT_SUBSCRIPTION(TitleEvent, title_event); + EVENT_SUBSCRIPTION(UserTitleEvent, user_title_event); }; /* USB Mass Storage task. */ - /* Its event returns a pointer to a UmsDeviceVector. */ + /* Its event returns a reference to a UmsDeviceVector. */ class UmsTask: public brls::RepeatingTask { private: UmsEvent ums_event; - UmsDeviceVector ums_devices; + UmsDeviceVector ums_devices{}; void PopulateUmsDeviceVector(void); @@ -143,8 +144,8 @@ namespace nxdt::tasks UmsTask(void); ~UmsTask(void); - /* Intentionally left here to let views retrieve UMS device info. */ - const UmsDeviceVector* GetUmsDevices(void); + /* Intentionally left here to let views retrieve UMS device info on-demand. */ + const UmsDeviceVector& GetUmsDevices(void); EVENT_SUBSCRIPTION(UmsEvent, ums_event); }; diff --git a/include/titles_tab.hpp b/include/titles_tab.hpp index 8b3ce6a..b89de53 100644 --- a/include/titles_tab.hpp +++ b/include/titles_tab.hpp @@ -36,8 +36,8 @@ namespace nxdt::views const TitleApplicationMetadata *app_metadata = nullptr; bool is_system = false; - TitleUserApplicationData user_app_data = {0}; - TitleInfo *system_title_info = NULL; + TitleUserApplicationData user_app_data{}; + TitleInfo *system_title_info = nullptr; public: TitlesTabPopup(const TitleApplicationMetadata *app_metadata, bool is_system); @@ -50,7 +50,7 @@ namespace nxdt::views private: const TitleApplicationMetadata *app_metadata = nullptr; bool is_system = false; - bool click_anim; + bool click_anim = true; public: TitlesTabItem(const TitleApplicationMetadata *app_metadata, bool is_system, bool click_anim = true); @@ -73,10 +73,10 @@ namespace nxdt::views private: RootView *root_view = nullptr; - nxdt::tasks::TitleEvent::Subscription title_task_sub; + nxdt::tasks::UserTitleEvent::Subscription title_task_sub; bool is_system = false; - void PopulateList(const nxdt::tasks::TitleApplicationMetadataVector* app_metadata); + void PopulateList(const nxdt::tasks::TitleApplicationMetadataVector& app_metadata); public: TitlesTab(RootView *root_view, bool is_system); diff --git a/source/root_view.cpp b/source/root_view.cpp index e19ad05..14309fa 100644 --- a/source/root_view.cpp +++ b/source/root_view.cpp @@ -128,15 +128,16 @@ namespace nxdt::views this->addTab("root_view/tabs/about"_i18n, new AboutTab()); /* Subscribe to status info event. */ - this->status_info_task_sub = this->status_info_task->RegisterListener([this](const nxdt::tasks::StatusInfoData *status_info_data) { - u32 charge_percentage = status_info_data->charge_percentage; - PsmChargerType charger_type = status_info_data->charger_type; + this->status_info_task_sub = this->status_info_task->RegisterListener([this](const nxdt::tasks::StatusInfoData& status_info_data) { + u32 charge_percentage = status_info_data.charge_percentage; + PsmChargerType charger_type = status_info_data.charger_type; - NifmInternetConnectionType connection_type = status_info_data->connection_type; - char *ip_addr = status_info_data->ip_addr; + bool connected = status_info_data.connected; + NifmInternetConnectionType connection_type = status_info_data.connection_type; + const char *ip_addr = status_info_data.ip_addr; /* Update time label. */ - this->time_lbl->setText(this->GetFormattedDateString(status_info_data->timeinfo)); + this->time_lbl->setText(this->GetFormattedDateString(status_info_data.timeinfo)); /* Update battery labels. */ this->battery_icon->setText(charger_type != PsmChargerType_Unconnected ? "\uE1A3" : (charge_percentage >= 100 ? "\uE1A4" : (charge_percentage >= 83 ? "\uEBD2" : \ @@ -150,12 +151,12 @@ namespace nxdt::views this->battery_percentage->setText(fmt::format("{}%", charge_percentage)); /* Update network labels. */ - this->connection_icon->setText(!connection_type ? "\uE195" : (connection_type == NifmInternetConnectionType_WiFi ? "\uE63E" : "\uE8BE")); - this->connection_status_lbl->setText(ip_addr ? std::string(ip_addr) : "root_view/not_connected"_i18n); + this->connection_icon->setText(connected ? (connection_type == NifmInternetConnectionType_WiFi ? "\uE63E" : "\uE8BE") : "\uE195"); + this->connection_status_lbl->setText(connected ? std::string(ip_addr) : "root_view/not_connected"_i18n); }); /* Subscribe to UMS event. */ - this->ums_task_sub = this->ums_task->RegisterListener([this](const nxdt::tasks::UmsDeviceVector* ums_devices) { + this->ums_task_sub = this->ums_task->RegisterListener([this](const nxdt::tasks::UmsDeviceVector& ums_devices) { /* Update UMS counter label. */ this->ums_counter_lbl->setText(i18n::getStr("root_view/ums_counter"_i18n, usbHsFsGetPhysicalDeviceCount())); }); diff --git a/source/tasks.cpp b/source/tasks.cpp index 8d74513..1233a8e 100644 --- a/source/tasks.cpp +++ b/source/tasks.cpp @@ -43,7 +43,7 @@ namespace nxdt::tasks bool StatusInfoTask::IsInternetConnectionAvailable(void) { - return (this->status_info_data.ip_addr != NULL); + return this->status_info_data.connected; } void StatusInfoTask::run(retro_time_t current_time) @@ -53,7 +53,7 @@ namespace nxdt::tasks StatusInfoData *status_info_data = &(this->status_info_data); /* Get current time. */ - time_t unix_time = time(NULL); + time_t unix_time = time(nullptr); localtime_r(&unix_time, &(status_info_data->timeinfo)); /* Get battery stats. */ @@ -62,26 +62,24 @@ namespace nxdt::tasks /* Get network connection status. */ u32 signal_strength = 0; - NifmInternetConnectionStatus connection_status = static_cast(0); + NifmInternetConnectionStatus connection_status{}; + char *ip_addr = nullptr; + + status_info_data->connected = false; Result rc = nifmGetInternetConnectionStatus(&(status_info_data->connection_type), &signal_strength, &connection_status); - if (R_SUCCEEDED(rc)) + if (R_SUCCEEDED(rc) && status_info_data->connection_type && connection_status == NifmInternetConnectionStatus_Connected) { - if (status_info_data->connection_type && connection_status == NifmInternetConnectionStatus_Connected) - { - struct in_addr addr = { .s_addr = INADDR_NONE }; - nifmGetCurrentIpAddress(&(addr.s_addr)); - status_info_data->ip_addr = (addr.s_addr != INADDR_NONE ? inet_ntoa(addr) : NULL); - } else { - status_info_data->ip_addr = NULL; - } - } else { - status_info_data->connection_type = static_cast(0); - status_info_data->ip_addr = NULL; + status_info_data->connected = true; + + struct in_addr addr = { .s_addr = INADDR_NONE }; + nifmGetCurrentIpAddress(&(addr.s_addr)); + + if (addr.s_addr != INADDR_NONE && (ip_addr = inet_ntoa(addr))) snprintf(status_info_data->ip_addr, MAX_ELEMENTS(status_info_data->ip_addr), "%s", ip_addr); } /* Fire task event. */ - this->status_info_event.fire(status_info_data); + this->status_info_event.fire(this->status_info_data); } /* Gamecard task. */ @@ -171,30 +169,30 @@ namespace nxdt::tasks this->PopulateApplicationMetadataVector(false); /* Fire task event. */ - this->title_event.fire(&(this->user_metadata)); + this->user_title_event.fire(this->user_metadata); } } - const TitleApplicationMetadataVector* TitleTask::GetApplicationMetadata(bool is_system) + const TitleApplicationMetadataVector& TitleTask::GetApplicationMetadata(bool is_system) { - return (is_system ? &(this->system_metadata) : &(this->user_metadata)); + return (is_system ? this->system_metadata : this->user_metadata); } void TitleTask::PopulateApplicationMetadataVector(bool is_system) { - TitleApplicationMetadata **app_metadata = NULL; + TitleApplicationMetadata **app_metadata = nullptr; u32 app_metadata_count = 0; /* Get pointer to output vector. */ - TitleApplicationMetadataVector *vector = (is_system ? &(this->system_metadata) : &(this->user_metadata)); - vector->clear(); + TitleApplicationMetadataVector& vector = (is_system ? this->system_metadata : this->user_metadata); + vector.clear(); /* Get application metadata entries. */ app_metadata = titleGetApplicationMetadataEntries(is_system, &app_metadata_count); if (app_metadata) { /* Fill output vector. */ - for(u32 i = 0; i < app_metadata_count; i++) vector->push_back(app_metadata[i]); + for(u32 i = 0; i < app_metadata_count; i++) vector.push_back(app_metadata[i]); /* Free application metadata array. */ free(app_metadata); @@ -232,18 +230,18 @@ namespace nxdt::tasks this->PopulateUmsDeviceVector(); /* Fire task event. */ - this->ums_event.fire(&(this->ums_devices)); + this->ums_event.fire(this->ums_devices); } } - const UmsDeviceVector* UmsTask::GetUmsDevices(void) + const UmsDeviceVector& UmsTask::GetUmsDevices(void) { - return &(this->ums_devices); + return this->ums_devices; } void UmsTask::PopulateUmsDeviceVector(void) { - UsbHsFsDevice *ums_devices = NULL; + UsbHsFsDevice *ums_devices = nullptr; u32 ums_device_count = 0; /* Clear UMS device vector. */ diff --git a/source/titles_tab.cpp b/source/titles_tab.cpp index f86bef7..62b3b89 100644 --- a/source/titles_tab.cpp +++ b/source/titles_tab.cpp @@ -41,7 +41,7 @@ namespace nxdt::views this->system_title_info = titleGetInfoFromStorageByTitleId(NcmStorageId_BuiltInSystem, title_id); } - /* Make sure we got title information. */ + /* Make sure we got title information. This should never get triggered. */ if ((!this->is_system && !user_ret) || (this->is_system && !this->system_title_info)) throw fmt::format("Failed to retrieve title information for {:016X}.", title_id); /* Add tabs. */ @@ -89,7 +89,7 @@ namespace nxdt::views /* Subscribe to the title event if this is the user titles tab. */ if (!this->is_system) { - this->title_task_sub = this->root_view->RegisterTitleTaskListener([this](const nxdt::tasks::TitleApplicationMetadataVector* app_metadata) { + this->title_task_sub = this->root_view->RegisterTitleTaskListener([this](const nxdt::tasks::TitleApplicationMetadataVector& app_metadata) { /* Update list. */ this->PopulateList(app_metadata); }); @@ -102,10 +102,10 @@ namespace nxdt::views if (!this->is_system) this->root_view->UnregisterTitleTaskListener(this->title_task_sub); } - void TitlesTab::PopulateList(const nxdt::tasks::TitleApplicationMetadataVector* app_metadata) + void TitlesTab::PopulateList(const nxdt::tasks::TitleApplicationMetadataVector& app_metadata) { /* Populate variables. */ - size_t app_metadata_count = (app_metadata ? app_metadata->size() : 0); + size_t app_metadata_count = app_metadata.size(); bool update_focused_view = this->IsListItemFocused(); int focus_stack_index = this->GetFocusStackViewIndex(); @@ -120,13 +120,13 @@ namespace nxdt::views if (!app_metadata_count) return; /* Populate list. */ - for(const TitleApplicationMetadata *cur_app_metadata : *app_metadata) + for(const TitleApplicationMetadata *cur_app_metadata : app_metadata) { /* Create list item. */ - TitlesTabItem *title = new TitlesTabItem(cur_app_metadata, this->is_system); + TitlesTabItem *item = new TitlesTabItem(cur_app_metadata, this->is_system); /* Register click event. */ - title->getClickEvent()->subscribe([](brls::View *view) { + item->getClickEvent()->subscribe([](brls::View *view) { TitlesTabItem *item = static_cast(view); const TitleApplicationMetadata *app_metadata = item->GetApplicationMetadata(); bool is_system = item->IsSystemTitle(); @@ -157,7 +157,7 @@ namespace nxdt::views }); /* Add list item to our view. */ - this->list->addView(title); + this->list->addView(item); } /* Update focus stack, if needed. */