Blob Blame History Raw
From f80270519b55d7b12d06538df5d989c615eea1df Mon Sep 17 00:00:00 2001
From: Aleksei Bavshin <alebastr89@gmail.com>
Date: Mon, 13 Jan 2020 23:27:57 -0800
Subject: [PATCH] refactor(client): use std::list<waybar_output> to store
 outputs

std::unique_ptr is not required here as the only benefit it gives is
stability of address on vector resize and it's easy to invalidate it
accidentaly. std::list provides the same guarantee of stable addresses
of the elements and correct destruction while avoiding smart pointer
overhead.

Also fixes #554, caused by incorrect usage of std::remove_if.
---
 include/client.hpp | 16 ++++++++--------
 src/client.cpp     | 42 +++++++++++++++++++-----------------------
 2 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/include/client.hpp b/include/client.hpp
index 65a814fc..39b6ae3b 100644
--- a/include/client.hpp
+++ b/include/client.hpp
@@ -30,12 +30,12 @@ class Client {
                                                               const std::string &style) const;
   void                                             bindInterfaces();
   const std::string getValidPath(const std::vector<std::string> &paths) const;
-  void              handleOutput(std::unique_ptr<struct waybar_output> &output);
-  bool isValidOutput(const Json::Value &config, std::unique_ptr<struct waybar_output> &output);
+  void              handleOutput(struct waybar_output &output);
+  bool isValidOutput(const Json::Value &config, struct waybar_output &output);
   auto setupConfig(const std::string &config_file) -> void;
   auto setupCss(const std::string &css_file) -> void;
-  std::unique_ptr<struct waybar_output> &getOutput(void *);
-  std::vector<Json::Value> getOutputConfigs(std::unique_ptr<struct waybar_output> &output);
+  struct waybar_output &getOutput(void *);
+  std::vector<Json::Value> getOutputConfigs(struct waybar_output &output);
 
   static void handleGlobal(void *data, struct wl_registry *registry, uint32_t name,
                            const char *interface, uint32_t version);
@@ -44,10 +44,10 @@ class Client {
   void        handleMonitorAdded(Glib::RefPtr<Gdk::Monitor> monitor);
   void        handleMonitorRemoved(Glib::RefPtr<Gdk::Monitor> monitor);
 
-  Json::Value                                        config_;
-  Glib::RefPtr<Gtk::StyleContext>                    style_context_;
-  Glib::RefPtr<Gtk::CssProvider>                     css_provider_;
-  std::vector<std::unique_ptr<struct waybar_output>> outputs_;
+  Json::Value                     config_;
+  Glib::RefPtr<Gtk::StyleContext> style_context_;
+  Glib::RefPtr<Gtk::CssProvider>  css_provider_;
+  std::list<struct waybar_output> outputs_;
 };
 
 }  // namespace waybar
diff --git a/src/client.cpp b/src/client.cpp
index dd4cd50e..f7c70e06 100644
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -49,7 +49,7 @@ void waybar::Client::handleGlobalRemove(void *   data, struct wl_registry * /*re
   // Nothing here
 }
 
-void waybar::Client::handleOutput(std::unique_ptr<struct waybar_output> &output) {
+void waybar::Client::handleOutput(struct waybar_output &output) {
   static const struct zxdg_output_v1_listener xdgOutputListener = {
       .logical_position = [](void *, struct zxdg_output_v1 *, int32_t, int32_t) {},
       .logical_size = [](void *, struct zxdg_output_v1 *, int32_t, int32_t) {},
@@ -58,42 +58,39 @@ void waybar::Client::handleOutput(std::unique_ptr<struct waybar_output> &output)
       .description = [](void *, struct zxdg_output_v1 *, const char *) {},
   };
   // owned by output->monitor; no need to destroy
-  auto wl_output = gdk_wayland_monitor_get_wl_output(output->monitor->gobj());
-  output->xdg_output.reset(zxdg_output_manager_v1_get_xdg_output(xdg_output_manager, wl_output));
-  zxdg_output_v1_add_listener(output->xdg_output.get(), &xdgOutputListener, output.get());
+  auto wl_output = gdk_wayland_monitor_get_wl_output(output.monitor->gobj());
+  output.xdg_output.reset(zxdg_output_manager_v1_get_xdg_output(xdg_output_manager, wl_output));
+  zxdg_output_v1_add_listener(output.xdg_output.get(), &xdgOutputListener, &output);
 }
 
-bool waybar::Client::isValidOutput(const Json::Value &                    config,
-                                   std::unique_ptr<struct waybar_output> &output) {
+bool waybar::Client::isValidOutput(const Json::Value &config, struct waybar_output &output) {
   bool found = true;
   if (config["output"].isArray()) {
     bool in_array = false;
     for (auto const &output_conf : config["output"]) {
-      if (output_conf.isString() && output_conf.asString() == output->name) {
+      if (output_conf.isString() && output_conf.asString() == output.name) {
         in_array = true;
         break;
       }
     }
     found = in_array;
   }
-  if (config["output"].isString() && config["output"].asString() != output->name) {
+  if (config["output"].isString() && config["output"].asString() != output.name) {
     found = false;
   }
   return found;
 }
 
-std::unique_ptr<struct waybar::waybar_output> &waybar::Client::getOutput(void *addr) {
-  auto it = std::find_if(outputs_.begin(), outputs_.end(), [&addr](const auto &output) {
-    return output.get() == addr;
-  });
+struct waybar::waybar_output &waybar::Client::getOutput(void *addr) {
+  auto it = std::find_if(
+      outputs_.begin(), outputs_.end(), [&addr](const auto &output) { return &output == addr; });
   if (it == outputs_.end()) {
     throw std::runtime_error("Unable to find valid output");
   }
   return *it;
 }
 
-std::vector<Json::Value> waybar::Client::getOutputConfigs(
-    std::unique_ptr<struct waybar_output> &output) {
+std::vector<Json::Value> waybar::Client::getOutputConfigs(struct waybar_output &output) {
   std::vector<Json::Value> configs;
   if (config_.isArray()) {
     for (auto const &config : config_) {
@@ -112,18 +109,18 @@ void waybar::Client::handleOutputName(void *      data, struct zxdg_output_v1 *
   auto client = waybar::Client::inst();
   try {
     auto &output = client->getOutput(data);
-    output->name = name;
+    output.name = name;
     spdlog::debug("Output detected: {} ({} {})",
                   name,
-                  output->monitor->get_manufacturer(),
-                  output->monitor->get_model());
+                  output.monitor->get_manufacturer(),
+                  output.monitor->get_model());
     auto configs = client->getOutputConfigs(output);
     if (configs.empty()) {
-      output->xdg_output.reset();
+      output.xdg_output.reset();
     } else {
       wl_display_roundtrip(client->wl_display);
       for (const auto &config : configs) {
-        client->bars.emplace_back(std::make_unique<Bar>(output.get(), config));
+        client->bars.emplace_back(std::make_unique<Bar>(&output, config));
         Glib::RefPtr<Gdk::Screen> screen = client->bars.back()->window.get_screen();
         client->style_context_->add_provider_for_screen(
             screen, client->css_provider_, GTK_STYLE_PROVIDER_PRIORITY_USER);
@@ -135,7 +132,8 @@ void waybar::Client::handleOutputName(void *      data, struct zxdg_output_v1 *
 }
 
 void waybar::Client::handleMonitorAdded(Glib::RefPtr<Gdk::Monitor> monitor) {
-  auto &output = outputs_.emplace_back(new struct waybar_output({monitor}));
+  auto &output = outputs_.emplace_back();
+  output.monitor = monitor;
   handleOutput(output);
 }
 
@@ -151,9 +149,7 @@ void waybar::Client::handleMonitorRemoved(Glib::RefPtr<Gdk::Monitor> monitor) {
       ++it;
     }
   }
-  std::remove_if(outputs_.begin(), outputs_.end(), [&monitor](const auto &output) {
-    return output->monitor == monitor;
-  });
+  outputs_.remove_if([&monitor](const auto &output) { return output.monitor == monitor; });
 }
 
 std::tuple<const std::string, const std::string> waybar::Client::getConfigs(