Remove undefined behaviors and clean code (#1769)

* fix: avoid memory leaks in case of exceptions.

* fix(fs): remove undefined behavior when removing mounts in m_mounts

* cleanup: remove double checks

* fix: remove memory leaks

* cleanup(xresources): capture exception by reference

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

include/modules/fs.hpp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <utility>
+
 #include "components/config.hpp"
 #include "modules/meta/timer_module.hpp"
 #include "settings.hpp"
@@ -25,7 +27,7 @@ namespace modules {
     int percentage_free{0};
     int percentage_used{0};
 
-    explicit fs_mount(const string& mountpoint, bool mounted = false) : mountpoint(mountpoint), mounted(mounted) {}
+    explicit fs_mount(string mountpoint, bool mounted = false) : mountpoint(move(mountpoint)), mounted(mounted) {}
   };
 
   using fs_mount_t = unique_ptr<fs_mount>;

include/x11/xresources.hpp

@@ -48,7 +48,7 @@ class xresource_manager {
   T get(const char* name, const T& fallback) const {
     try {
       return convert<T>(require<T>(name));
-    } catch (const xresource_error) {
+    } catch (const xresource_error&) {
       return fallback;
     }
   }

src/adapters/mpd.cpp

@@ -168,7 +168,7 @@ namespace mpd {
   }
 
   bool mpdconnection::connected() {
-    return m_connection && m_connection != nullptr;
+    return static_cast<bool>(m_connection);
   }
 
   bool mpdconnection::retry_connection(int interval) {

src/modules/alsa.cpp

@@ -265,15 +265,15 @@ namespace modules {
     bool headphones{m_headphones};
 
     if (m_mixer[mixer::MASTER] && !m_mixer[mixer::MASTER]->get_name().empty()) {
-      mixers.emplace_back(new mixer_t::element_type(
+      mixers.emplace_back(std::make_shared<mixer_t::element_type>(
           string{m_mixer[mixer::MASTER]->get_name()}, string{m_mixer[mixer::MASTER]->get_sound_card()}));
     }
     if (m_mixer[mixer::HEADPHONE] && !m_mixer[mixer::HEADPHONE]->get_name().empty() && headphones) {
-      mixers.emplace_back(new mixer_t::element_type(
+      mixers.emplace_back(std::make_shared<mixer_t::element_type>(
           string{m_mixer[mixer::HEADPHONE]->get_name()}, string{m_mixer[mixer::HEADPHONE]->get_sound_card()}));
     }
     if (m_mixer[mixer::SPEAKER] && !m_mixer[mixer::SPEAKER]->get_name().empty() && !headphones) {
-      mixers.emplace_back(new mixer_t::element_type(
+      mixers.emplace_back(std::make_shared<mixer_t::element_type>(
           string{m_mixer[mixer::SPEAKER]->get_name()}, string{m_mixer[mixer::SPEAKER]->get_sound_card()}));
     }
 

src/modules/fs.cpp

@@ -87,7 +87,7 @@ namespace modules {
     for (auto&& mountpoint : m_mountpoints) {
       auto details = std::find_if(mountinfo.begin(), mountinfo.end(), [&](const vector<string>& m) { return m.size() > 4 && m[MOUNTINFO_DIR] == mountpoint; });
 
-      m_mounts.emplace_back(new fs_mount{mountpoint, details != mountinfo.end()});
+      m_mounts.emplace_back(std::make_unique<fs_mount>(mountpoint, details != mountinfo.end()));
       struct statvfs buffer {};
 
       if (!m_mounts.back()->mounted) {
@@ -106,20 +106,24 @@ namespace modules {
         mount->bytes_used = mount->bytes_total - mount->bytes_free;
         mount->bytes_avail = static_cast<uint64_t>(buffer.f_frsize) * static_cast<uint64_t>(buffer.f_bavail);
 
-        mount->percentage_free = math_util::percentage<double>(mount->bytes_avail, mount->bytes_used + mount->bytes_avail);
-        mount->percentage_used = math_util::percentage<double>(mount->bytes_used, mount->bytes_used + mount->bytes_avail);
+        mount->percentage_free =
+            math_util::percentage<double>(mount->bytes_avail, mount->bytes_used + mount->bytes_avail);
+        mount->percentage_used =
+            math_util::percentage<double>(mount->bytes_used, mount->bytes_used + mount->bytes_avail);
       }
     }
 
     if (m_remove_unmounted) {
-      for (auto&& mount : m_mounts) {
-        if (!mount->mounted) {
-          m_log.info("%s: Removing mountpoint \"%s\" (reason: `remove-unmounted = true`)", name(), mount->mountpoint);
-          m_mountpoints.erase(
-              std::remove(m_mountpoints.begin(), m_mountpoints.end(), mount->mountpoint), m_mountpoints.end());
-          m_mounts.erase(std::remove(m_mounts.begin(), m_mounts.end(), mount), m_mounts.end());
-        }
+      auto new_end =
+          std::stable_partition(m_mounts.begin(), m_mounts.end(), [](const auto& mount) { return mount->mounted; });
+
+      for (auto it = new_end; it < m_mounts.end(); ++it) {
+        m_log.info("%s: Removing mountpoint \"%s\" (reason: `remove-unmounted = true`)", name(), (*it)->mountpoint);
+        m_mountpoints.erase(
+            std::remove(m_mountpoints.begin(), m_mountpoints.end(), (*it)->mountpoint), m_mountpoints.end());
       }
+
+      m_mounts.erase(new_end, m_mounts.end());
     }
 
     return true;
@@ -197,6 +201,6 @@ namespace modules {
 
     return true;
   }
-}
+}  // namespace modules
 
 POLYBAR_NS_END

src/modules/ipc.cpp

@@ -16,7 +16,7 @@ namespace modules {
     size_t index = 0;
 
     for (auto&& command : m_conf.get_list<string>(name(), "hook")) {
-      m_hooks.emplace_back(new hook{name() + to_string(++index), command});
+      m_hooks.emplace_back(std::make_unique<hook>(hook{name() + to_string(++index), command}));
     }
 
     if (m_hooks.empty()) {

src/modules/mpd.cpp

@@ -254,7 +254,7 @@ namespace modules {
       if (m_mpd) {
         auto song = m_mpd->get_song();
 
-        if (song && song.get()) {
+        if (song) {
           artist = song->get_artist();
           album_artist = song->get_album_artist();
           album = song->get_album();

src/modules/xwindow.cpp

@@ -132,7 +132,7 @@ namespace modules {
    * Output content as defined in the config
    */
   bool xwindow_module::build(builder* builder, const string& tag) const {
-    if (tag == TAG_LABEL && m_label && m_label.get()) {
+    if (tag == TAG_LABEL && m_label) {
       builder->node(m_label);
       return true;
     }

src/x11/background_manager.cpp

@@ -189,7 +189,7 @@ void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) {
 }
 
 void bg_slice::free_resources() {
-  m_surface.release();
+  m_surface.reset();
 
   if(m_pixmap != XCB_NONE) {
     m_connection.free_pixmap(m_pixmap);

src/x11/tray_manager.cpp

@@ -253,8 +253,8 @@ void tray_manager::deactivate(bool clear_selection) {
     m_log.trace("tray: Destroy window");
     m_connection.destroy_window(m_tray);
   }
-  m_context.release();
-  m_surface.release();
+  m_context.reset();
+  m_surface.reset();
   if (m_pixmap) {
     m_connection.free_pixmap(m_pixmap);
   }
@@ -519,11 +519,11 @@ void tray_manager::create_bg(bool realloc) {
     m_gc = 0;
   }
 
-  if (realloc && m_surface) {
-    m_surface.release();
+  if(realloc && m_surface) {
+    m_surface.reset();
   }
-  if (realloc && m_context) {
-    m_context.release();
+  if(realloc && m_context) {
+    m_context.reset();
   }
 
   auto w = m_opts.width_max;