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(