Merge pull request #2599 from Alexays/revert-2586-bugfix/fix-potentia…

…l-memory-leaks Revert "Fix potential memory leaks"
Alexays · Oct 22, 2023 · dafe49d · dafe49d
2 parents dbb887b + 2d33c20
commit dafe49d
Show file tree

Hide file tree

Showing 13 changed files with 29 additions and 90 deletions.
diff --git 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<UPowerTooltip> 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
@@ -2,7 +2,6 @@
 
 #include <libupower-glib/upower.h>
 
-#include <memory>
 #include <unordered_map>
 
 #include "gtkmm/box.h"
@@ -17,7 +16,7 @@ class UPowerTooltip : public Gtk::Window {
 
   const std::string getDeviceIcon(UpDeviceKind& kind);
 
-  std::unique_ptr<Gtk::Box> contentBox;
+  Gtk::Box* contentBox;
 
   uint iconSize;
   uint tooltipSpacing;

diff --git a/include/util/scope_guard.hpp b/include/util/scope_guard.hpp
diff --git 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
@@ -6,26 +6,20 @@
 #include <algorithm>
 #include <sstream>
 
-#include "util/scope_guard.hpp"
-
 namespace {
 
 using GDBusManager = std::unique_ptr<GDBusObjectManager, void (*)(GDBusObjectManager*)>;
 
 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,
       "org.bluez", "/", NULL, NULL, NULL, NULL, &error);
 
   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
@@ -9,7 +9,6 @@
 #include <cstdlib>  // malloc
 
 #include "modules/cpu_usage.hpp"
-#include "util/scope_guard.hpp"
 
 #if defined(__NetBSD__) || defined(__OpenBSD__)
 #include <sys/sched.h>
@@ -34,11 +33,6 @@ std::vector<std::tuple<size_t, size_t>> 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<pcp_time_t *>(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<std::tuple<size_t, size_t>> 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
@@ -2,8 +2,6 @@
 
 #include <spdlog/spdlog.h>
 
-#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
@@ -6,8 +6,6 @@
 #include <sstream>
 #include <string>
 
-#include "util/scope_guard.hpp"
-
 extern "C" {
 #include <playerctl/playerctl.h>
 }
@@ -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<PlayerInfo> {
   }
 
   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<PlayerInfo> {
   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<PlayerInfo> {
 
 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
@@ -2,8 +2,6 @@
 
 #include <spdlog/spdlog.h>
 
-#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<Gio::DBus::Connection>& 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<SNI::Host*>(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<SNI::Host*>(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
@@ -2,8 +2,6 @@
 
 #include <spdlog/spdlog.h>
 
-#include "util/scope_guard.hpp"
-
 using namespace waybar::modules::SNI;
 
 Watcher::Watcher()
@@ -31,18 +29,14 @@ Watcher::~Watcher() {
 
 void Watcher::busAcquired(const Glib::RefPtr<Gio::DBus::Connection>& 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) {
     // Don't print an error when a watcher is already present
     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
@@ -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<UPowerTooltip>(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
@@ -9,10 +9,11 @@
 namespace waybar::modules::upower {
 UPowerTooltip::UPowerTooltip(uint iconSize_, uint tooltipSpacing_, uint tooltipPadding_)
     : Gtk::Window(),
-      contentBox(std::make_unique<Gtk::Box>(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
@@ -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 {