From ae748b26441712b93578889db0a5f1be5471893c Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Fri, 20 Oct 2023 22:41:53 +0200 Subject: [PATCH] modules+util: fix actual (potential) memory leaks --- include/modules/upower/upower.hpp | 2 +- src/modules/cpu_usage/bsd.cpp | 7 ++++++- src/modules/custom.cpp | 8 +++++++- src/modules/mpris/mpris.cpp | 30 ++++++++++++++++++++---------- src/modules/upower/upower.cpp | 8 ++++---- src/util/prepare_for_sleep.cpp | 3 +-- 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/include/modules/upower/upower.hpp b/include/modules/upower/upower.hpp index 446d1f53..9c1a1830 100644 --- a/include/modules/upower/upower.hpp +++ b/include/modules/upower/upower.hpp @@ -69,7 +69,7 @@ class UPower : public AModule { UpDevice *displayDevice; guint login1_id; GDBusConnection *login1_connection; - UPowerTooltip *upower_tooltip; + std::unique_ptr upower_tooltip; std::string lastStatus; bool showAltText; bool upowerRunning; diff --git a/src/modules/cpu_usage/bsd.cpp b/src/modules/cpu_usage/bsd.cpp index c987a770..663bc0a7 100644 --- a/src/modules/cpu_usage/bsd.cpp +++ b/src/modules/cpu_usage/bsd.cpp @@ -9,6 +9,7 @@ #include // malloc #include "modules/cpu_usage.hpp" +#include "util/scope_guard.hpp" #if defined(__NetBSD__) || defined(__OpenBSD__) #include @@ -33,6 +34,11 @@ std::vector> waybar::modules::CpuUsage::parseCpuinfo( int ncpu = sysconf(_SC_NPROCESSORS_CONF); size_t sz = CPUSTATES * (ncpu + 1) * sizeof(pcp_time_t); pcp_time_t *cp_time = static_cast(malloc(sz)), *pcp_time = cp_time; + waybar::util::scope_guard cp_time_deleter([cp_time]() { + if (cp_time) { + free(cp_time); + } + }); #if defined(__NetBSD__) int mib[] = { CTL_KERN, @@ -97,6 +103,5 @@ std::vector> waybar::modules::CpuUsage::parseCpuinfo( } cpuinfo.emplace_back(single_cp_time[CP_IDLE], total); } - free(cp_time); return cpuinfo; } diff --git a/src/modules/custom.cpp b/src/modules/custom.cpp index 4889b7a3..d0a39802 100644 --- a/src/modules/custom.cpp +++ b/src/modules/custom.cpp @@ -2,6 +2,8 @@ #include +#include "util/scope_guard.hpp" + waybar::modules::Custom::Custom(const std::string& name, const std::string& id, const Json::Value& config) : ALabel(config, "custom-" + name, id, "{}"), @@ -57,6 +59,11 @@ void waybar::modules::Custom::continuousWorker() { } thread_ = [this, cmd] { char* buff = nullptr; + waybar::util::scope_guard buff_deleter([buff]() { + if (buff) { + free(buff); + } + }); size_t len = 0; if (getline(&buff, &len, fp_) == -1) { int exit_code = 1; @@ -90,7 +97,6 @@ void waybar::modules::Custom::continuousWorker() { output_ = {0, output}; dp.emit(); } - free(buff); }; } diff --git a/src/modules/mpris/mpris.cpp b/src/modules/mpris/mpris.cpp index 140bc785..685b1881 100644 --- a/src/modules/mpris/mpris.cpp +++ b/src/modules/mpris/mpris.cpp @@ -6,6 +6,8 @@ #include #include +#include "util/scope_guard.hpp" + extern "C" { #include } @@ -117,6 +119,11 @@ Mpris::Mpris(const std::string& id, const Json::Value& config) } GError* error = nullptr; + waybar::util::scope_guard error_deleter([error]() { + if (error) { + g_error_free(error); + } + }); manager = playerctl_player_manager_new(&error); if (error) { throw std::runtime_error(fmt::format("unable to create MPRIS client: {}", error->message)); @@ -136,9 +143,7 @@ Mpris::Mpris(const std::string& id, const Json::Value& config) } else { GList* players = playerctl_list_players(&error); if (error) { - auto e = fmt::format("unable to list players: {}", error->message); - g_error_free(error); - throw std::runtime_error(e); + throw std::runtime_error(fmt::format("unable to list players: {}", error->message)); } for (auto p = players; p != NULL; p = p->next) { @@ -410,8 +415,7 @@ auto Mpris::onPlayerNameAppeared(PlayerctlPlayerManager* manager, PlayerctlPlaye return; } - GError* error = nullptr; - mpris->player = playerctl_player_new_from_name(player_name, &error); + mpris->player = playerctl_player_new_from_name(player_name, NULL); g_object_connect(mpris->player, "signal::play", G_CALLBACK(onPlayerPlay), mpris, "signal::pause", G_CALLBACK(onPlayerPause), mpris, "signal::stop", G_CALLBACK(onPlayerStop), mpris, "signal::stop", G_CALLBACK(onPlayerStop), mpris, "signal::metadata", @@ -478,6 +482,11 @@ auto Mpris::getPlayerInfo() -> std::optional { } GError* error = nullptr; + waybar::util::scope_guard error_deleter([error]() { + if (error) { + g_error_free(error); + } + }); char* player_status = nullptr; auto player_playback_status = PLAYERCTL_PLAYBACK_STATUS_STOPPED; @@ -487,9 +496,7 @@ auto Mpris::getPlayerInfo() -> std::optional { if (player_name == "playerctld") { GList* players = playerctl_list_players(&error); if (error) { - auto e = fmt::format("unable to list players: {}", error->message); - g_error_free(error); - throw std::runtime_error(e); + throw std::runtime_error(fmt::format("unable to list players: {}", error->message)); } // > get the list of players [..] in order of activity // https://github.com/altdesktop/playerctl/blob/b19a71cb9dba635df68d271bd2b3f6a99336a223/playerctl/playerctl-common.c#L248-L249 @@ -568,12 +575,16 @@ auto Mpris::getPlayerInfo() -> std::optional { errorexit: spdlog::error("mpris[{}]: {}", info.name, error->message); - g_error_free(error); return std::nullopt; } bool Mpris::handleToggle(GdkEventButton* const& e) { GError* error = nullptr; + waybar::util::scope_guard error_deleter([error]() { + if (error) { + g_error_free(error); + } + }); auto info = getPlayerInfo(); if (!info) return false; @@ -603,7 +614,6 @@ bool Mpris::handleToggle(GdkEventButton* const& e) { if (error) { spdlog::error("mpris[{}]: error running builtin on-click action: {}", (*info).name, error->message); - g_error_free(error); return false; } return true; diff --git a/src/modules/upower/upower.cpp b/src/modules/upower/upower.cpp index f2bc621d..e3b3981a 100644 --- a/src/modules/upower/upower.cpp +++ b/src/modules/upower/upower.cpp @@ -63,7 +63,7 @@ UPower::UPower(const std::string& id, const Json::Value& config) box_.set_has_tooltip(tooltip_enabled); if (tooltip_enabled) { // Sets the window to use when showing the tooltip - upower_tooltip = new UPowerTooltip(iconSize, tooltip_spacing, tooltip_padding); + upower_tooltip = std::make_unique(iconSize, tooltip_spacing, tooltip_padding); box_.set_tooltip_window(*upower_tooltip); box_.signal_query_tooltip().connect(sigc::mem_fun(*this, &UPower::show_tooltip_callback)); } @@ -72,14 +72,13 @@ UPower::UPower(const std::string& id, const Json::Value& config) G_BUS_NAME_WATCHER_FLAGS_AUTO_START, upowerAppear, upowerDisappear, this, NULL); - GError* error = NULL; - client = up_client_new_full(NULL, &error); + client = up_client_new_full(NULL, NULL); if (client == NULL) { throw std::runtime_error("Unable to create UPower client!"); } // Connect to Login1 PrepareForSleep signal - login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); + login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL); if (!login1_connection) { throw std::runtime_error("Unable to connect to the SYSTEM Bus!..."); } else { @@ -99,6 +98,7 @@ UPower::UPower(const std::string& id, const Json::Value& config) } UPower::~UPower() { + if (displayDevice != NULL) g_object_unref(displayDevice); if (client != NULL) g_object_unref(client); if (login1_id > 0) { g_dbus_connection_signal_unsubscribe(login1_connection, login1_id); diff --git a/src/util/prepare_for_sleep.cpp b/src/util/prepare_for_sleep.cpp index 218c1e29..661285a2 100644 --- a/src/util/prepare_for_sleep.cpp +++ b/src/util/prepare_for_sleep.cpp @@ -7,8 +7,7 @@ namespace { class PrepareForSleep { private: PrepareForSleep() { - GError *error = NULL; - login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); + login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL); if (!login1_connection) { spdlog::warn("Unable to connect to the SYSTEM Bus!..."); } else {