From 2d33c20231a5baf3a427837ceead9046d5152f96 Mon Sep 17 00:00:00 2001 From: Alexis Rouillard Date: Sun, 22 Oct 2023 09:44:46 +0200 Subject: [PATCH] Revert "Fix potential memory leaks" --- include/modules/upower/upower.hpp | 2 +- include/modules/upower/upower_tooltip.hpp | 3 +-- include/util/scope_guard.hpp | 21 ---------------- src/ALabel.cpp | 2 +- src/modules/bluetooth.cpp | 8 +----- src/modules/cpu_usage/bsd.cpp | 7 +----- src/modules/custom.cpp | 8 +----- src/modules/mpris/mpris.cpp | 30 ++++++++--------------- src/modules/sni/host.cpp | 16 +++--------- src/modules/sni/watcher.cpp | 8 +----- src/modules/upower/upower.cpp | 8 +++--- src/modules/upower/upower_tooltip.cpp | 3 ++- src/util/prepare_for_sleep.cpp | 3 ++- 13 files changed, 29 insertions(+), 90 deletions(-) delete mode 100644 include/util/scope_guard.hpp diff --git a/include/modules/upower/upower.hpp b/include/modules/upower/upower.hpp index 9c1a1830..446d1f53 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; - std::unique_ptr upower_tooltip; + UPowerTooltip *upower_tooltip; std::string lastStatus; bool showAltText; bool upowerRunning; diff --git a/include/modules/upower/upower_tooltip.hpp b/include/modules/upower/upower_tooltip.hpp index bc99abed..05e9dcb3 100644 --- a/include/modules/upower/upower_tooltip.hpp +++ b/include/modules/upower/upower_tooltip.hpp @@ -2,7 +2,6 @@ #include -#include #include #include "gtkmm/box.h" @@ -17,7 +16,7 @@ class UPowerTooltip : public Gtk::Window { const std::string getDeviceIcon(UpDeviceKind& kind); - std::unique_ptr contentBox; + Gtk::Box* contentBox; uint iconSize; uint tooltipSpacing; diff --git a/include/util/scope_guard.hpp b/include/util/scope_guard.hpp deleted file mode 100644 index 5c717704..00000000 --- a/include/util/scope_guard.hpp +++ /dev/null @@ -1,21 +0,0 @@ -#pragma once - -#include - -namespace waybar::util { - -template -class scope_guard { - public: - explicit scope_guard(Func&& exit_function) : f{std::forward(exit_function)} {} - scope_guard(const scope_guard&) = delete; - scope_guard(scope_guard&&) = default; - scope_guard& operator=(const scope_guard&) = delete; - scope_guard& operator=(scope_guard&&) = default; - ~scope_guard() { f(); } - - private: - Func f; -}; - -} // namespace waybar::util diff --git a/src/ALabel.cpp b/src/ALabel.cpp index c87e3228..4d8b2218 100644 --- a/src/ALabel.cpp +++ b/src/ALabel.cpp @@ -12,7 +12,7 @@ ALabel::ALabel(const Json::Value& config, const std::string& name, const std::st : AModule(config, name, id, config["format-alt"].isString() || enable_click, enable_scroll), format_(config_["format"].isString() ? config_["format"].asString() : format), interval_(config_["interval"] == "once" - ? std::chrono::seconds::max() + ? std::chrono::seconds(100000000) : std::chrono::seconds( config_["interval"].isUInt() ? config_["interval"].asUInt() : interval)), default_format_(format_) { diff --git a/src/modules/bluetooth.cpp b/src/modules/bluetooth.cpp index 6b6cb7ef..9e207507 100644 --- a/src/modules/bluetooth.cpp +++ b/src/modules/bluetooth.cpp @@ -6,19 +6,12 @@ #include #include -#include "util/scope_guard.hpp" - namespace { using GDBusManager = std::unique_ptr; auto generateManager() -> GDBusManager { GError* error = nullptr; - waybar::util::scope_guard error_deleter([error]() { - if (error) { - g_error_free(error); - } - }); GDBusObjectManager* manager = g_dbus_object_manager_client_new_for_bus_sync( G_BUS_TYPE_SYSTEM, GDBusObjectManagerClientFlags::G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, @@ -26,6 +19,7 @@ auto generateManager() -> GDBusManager { if (error) { spdlog::error("g_dbus_object_manager_client_new_for_bus_sync() failed: {}", error->message); + g_error_free(error); } auto destructor = [](GDBusObjectManager* manager) { diff --git a/src/modules/cpu_usage/bsd.cpp b/src/modules/cpu_usage/bsd.cpp index 663bc0a7..c987a770 100644 --- a/src/modules/cpu_usage/bsd.cpp +++ b/src/modules/cpu_usage/bsd.cpp @@ -9,7 +9,6 @@ #include // malloc #include "modules/cpu_usage.hpp" -#include "util/scope_guard.hpp" #if defined(__NetBSD__) || defined(__OpenBSD__) #include @@ -34,11 +33,6 @@ 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, @@ -103,5 +97,6 @@ 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 d0a39802..4889b7a3 100644 --- a/src/modules/custom.cpp +++ b/src/modules/custom.cpp @@ -2,8 +2,6 @@ #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, "{}"), @@ -59,11 +57,6 @@ 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; @@ -97,6 +90,7 @@ 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 685b1881..140bc785 100644 --- a/src/modules/mpris/mpris.cpp +++ b/src/modules/mpris/mpris.cpp @@ -6,8 +6,6 @@ #include #include -#include "util/scope_guard.hpp" - extern "C" { #include } @@ -119,11 +117,6 @@ 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)); @@ -143,7 +136,9 @@ Mpris::Mpris(const std::string& id, const Json::Value& config) } else { GList* players = playerctl_list_players(&error); if (error) { - throw std::runtime_error(fmt::format("unable to list players: {}", error->message)); + auto e = fmt::format("unable to list players: {}", error->message); + g_error_free(error); + throw std::runtime_error(e); } for (auto p = players; p != NULL; p = p->next) { @@ -415,7 +410,8 @@ auto Mpris::onPlayerNameAppeared(PlayerctlPlayerManager* manager, PlayerctlPlaye return; } - mpris->player = playerctl_player_new_from_name(player_name, NULL); + GError* error = nullptr; + mpris->player = playerctl_player_new_from_name(player_name, &error); 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", @@ -482,11 +478,6 @@ 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; @@ -496,7 +487,9 @@ auto Mpris::getPlayerInfo() -> std::optional { if (player_name == "playerctld") { GList* players = playerctl_list_players(&error); if (error) { - throw std::runtime_error(fmt::format("unable to list players: {}", error->message)); + auto e = fmt::format("unable to list players: {}", error->message); + g_error_free(error); + throw std::runtime_error(e); } // > get the list of players [..] in order of activity // https://github.com/altdesktop/playerctl/blob/b19a71cb9dba635df68d271bd2b3f6a99336a223/playerctl/playerctl-common.c#L248-L249 @@ -575,16 +568,12 @@ 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; @@ -614,6 +603,7 @@ 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/sni/host.cpp b/src/modules/sni/host.cpp index 136c2941..0bbd4d2f 100644 --- a/src/modules/sni/host.cpp +++ b/src/modules/sni/host.cpp @@ -2,8 +2,6 @@ #include -#include "util/scope_guard.hpp" - namespace waybar::modules::SNI { Host::Host(const std::size_t id, const Json::Value& config, const Bar& bar, @@ -59,20 +57,17 @@ void Host::nameVanished(const Glib::RefPtr& conn, const G void Host::proxyReady(GObject* src, GAsyncResult* res, gpointer data) { GError* error = nullptr; - waybar::util::scope_guard error_deleter([error]() { - if (error != nullptr) { - g_error_free(error); - } - }); SnWatcher* watcher = sn_watcher_proxy_new_finish(res, &error); if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { spdlog::error("Host: {}", error->message); + g_error_free(error); return; } auto host = static_cast(data); host->watcher_ = watcher; if (error != nullptr) { spdlog::error("Host: {}", error->message); + g_error_free(error); return; } sn_watcher_call_register_host(host->watcher_, host->object_path_.c_str(), host->cancellable_, @@ -81,19 +76,16 @@ void Host::proxyReady(GObject* src, GAsyncResult* res, gpointer data) { void Host::registerHost(GObject* src, GAsyncResult* res, gpointer data) { GError* error = nullptr; - waybar::util::scope_guard error_deleter([error]() { - if (error != nullptr) { - g_error_free(error); - } - }); sn_watcher_call_register_host_finish(SN_WATCHER(src), res, &error); if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { spdlog::error("Host: {}", error->message); + g_error_free(error); return; } auto host = static_cast(data); if (error != nullptr) { spdlog::error("Host: {}", error->message); + g_error_free(error); return; } g_signal_connect(host->watcher_, "item-registered", G_CALLBACK(&Host::itemRegistered), data); diff --git a/src/modules/sni/watcher.cpp b/src/modules/sni/watcher.cpp index 22434709..dfd076ef 100644 --- a/src/modules/sni/watcher.cpp +++ b/src/modules/sni/watcher.cpp @@ -2,8 +2,6 @@ #include -#include "util/scope_guard.hpp" - using namespace waybar::modules::SNI; Watcher::Watcher() @@ -31,11 +29,6 @@ Watcher::~Watcher() { void Watcher::busAcquired(const Glib::RefPtr& conn, Glib::ustring name) { GError* error = nullptr; - waybar::util::scope_guard error_deleter([error]() { - if (error) { - g_error_free(error); - } - }); g_dbus_interface_skeleton_export(G_DBUS_INTERFACE_SKELETON(watcher_), conn->gobj(), "/StatusNotifierWatcher", &error); if (error != nullptr) { @@ -43,6 +36,7 @@ void Watcher::busAcquired(const Glib::RefPtr& conn, Glib: if (error->code != 2) { spdlog::error("Watcher: {}", error->message); } + g_error_free(error); return; } g_signal_connect_swapped(watcher_, "handle-register-item", diff --git a/src/modules/upower/upower.cpp b/src/modules/upower/upower.cpp index e3b3981a..f2bc621d 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 = std::make_unique(iconSize, tooltip_spacing, tooltip_padding); + upower_tooltip = new UPowerTooltip(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,13 +72,14 @@ UPower::UPower(const std::string& id, const Json::Value& config) G_BUS_NAME_WATCHER_FLAGS_AUTO_START, upowerAppear, upowerDisappear, this, NULL); - client = up_client_new_full(NULL, NULL); + GError* error = NULL; + client = up_client_new_full(NULL, &error); 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, NULL); + login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); if (!login1_connection) { throw std::runtime_error("Unable to connect to the SYSTEM Bus!..."); } else { @@ -98,7 +99,6 @@ 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/modules/upower/upower_tooltip.cpp b/src/modules/upower/upower_tooltip.cpp index 1a653f85..45544bbc 100644 --- a/src/modules/upower/upower_tooltip.cpp +++ b/src/modules/upower/upower_tooltip.cpp @@ -9,10 +9,11 @@ namespace waybar::modules::upower { UPowerTooltip::UPowerTooltip(uint iconSize_, uint tooltipSpacing_, uint tooltipPadding_) : Gtk::Window(), - contentBox(std::make_unique(Gtk::ORIENTATION_VERTICAL)), iconSize(iconSize_), tooltipSpacing(tooltipSpacing_), tooltipPadding(tooltipPadding_) { + contentBox = new Gtk::Box(Gtk::ORIENTATION_VERTICAL); + // Sets the Tooltip Padding contentBox->set_margin_top(tooltipPadding); contentBox->set_margin_bottom(tooltipPadding); diff --git a/src/util/prepare_for_sleep.cpp b/src/util/prepare_for_sleep.cpp index 661285a2..218c1e29 100644 --- a/src/util/prepare_for_sleep.cpp +++ b/src/util/prepare_for_sleep.cpp @@ -7,7 +7,8 @@ namespace { class PrepareForSleep { private: PrepareForSleep() { - login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL); + GError *error = NULL; + login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); if (!login1_connection) { spdlog::warn("Unable to connect to the SYSTEM Bus!..."); } else {