refactor(x11): Use shared_ptr for X pointers

include/x11/fonts.hpp

@@ -38,7 +38,7 @@ class font_manager {
   using make_type = unique_ptr<font_manager>;
   static make_type make();
 
-  explicit font_manager(connection& conn, const logger& logger);
+  explicit font_manager(connection& conn, const logger& logger, shared_ptr<Display>&& dsp, shared_ptr<Visual>&& vis);
   ~font_manager();
 
   bool load(const string& name, int8_t fontindex = DEFAULT_FONT_INDEX, int8_t offset_y = 0);
@@ -67,8 +67,8 @@ class font_manager {
   connection& m_connection;
   const logger& m_logger;
 
-  Display* m_display{nullptr};
+  shared_ptr<Display> m_display{nullptr};
-  Visual* m_visual{nullptr};
+  shared_ptr<Visual> m_visual{nullptr};
   Colormap m_colormap{};
 
   std::map<uint8_t, fonttype_pointer> m_fonts;

include/x11/xlib.hpp

@@ -7,34 +7,24 @@
 POLYBAR_NS
 
 namespace xlib {
-  extern Display* g_display;
+  shared_ptr<Display> get_display();
-  extern XVisualInfo g_visual_info;
+  shared_ptr<Visual> get_visual(int screen = 0);
 
-  extern Display* get_display();
+  Colormap create_colormap(int screen = 0);
-  extern Visual* get_visual(int screen = 0);
-  extern Colormap create_colormap(int screen = 0);
 
   /**
    * RAII wrapper for Xlib display locking
    */
-  class xlib_lock {
+  class display_lock {
    public:
-    explicit xlib_lock(Display* display) {
+    explicit display_lock(shared_ptr<Display>&& display);
-      XLockDisplay(display);
+    ~display_lock();
-      m_display = display;
-    }
-
-    ~xlib_lock() {
-      XUnlockDisplay(m_display);
-    }
 
    protected:
-    Display* m_display;
+    shared_ptr<Display> m_display;
   };
 
-  inline auto make_display_lock() {
+  inline auto make_display_lock();
-    return make_unique<xlib_lock>(get_display());
-  }
 }
 
 POLYBAR_NS_END

include/x11/xresources.hpp

@@ -12,7 +12,7 @@ class xresource_manager {
   using make_type = const xresource_manager&;
   static make_type make();
 
-  explicit xresource_manager();
+  explicit xresource_manager(shared_ptr<Display>&&);
   ~xresource_manager();
 
   string get_string(string name, string fallback = "") const;
@@ -23,7 +23,7 @@ class xresource_manager {
   string load_value(const string& key, const string& res_type, size_t n) const;
 
  private:
-  Display* m_display{nullptr};
+  shared_ptr<Display> m_display;
   XrmDatabase m_db;
   char* m_manager{nullptr};
 };

include/x11/xutils.hpp

@@ -13,7 +13,7 @@ class connection;
 class config;
 
 namespace xutils {
-  xcb_connection_t* get_connection();
+  shared_ptr<xcb_connection_t> get_connection();
   int get_connection_fd();
 
   void pack_values(uint32_t mask, const uint32_t* src, uint32_t* dest);

src/main.cpp

@@ -41,14 +41,14 @@ int main(int argc, char** argv) {
     // Connect to X server
     //==================================================
     XInitThreads();
-    xcb_connection_t* xcbconn{nullptr};
+    shared_ptr<xcb_connection_t> xcbconn{xutils::get_connection()};
 
-    if ((xcbconn = xutils::get_connection()) == nullptr) {
+    if (!xcbconn) {
       logger.err("A connection to X could not be established... ");
       throw exit_failure{};
     }
 
-    connection conn{xcbconn};
+    connection conn{xcbconn.get()};
     conn.preload_atoms();
     conn.query_extensions();
 

src/x11/connection.cpp

@@ -13,7 +13,7 @@ POLYBAR_NS
  */
 connection::make_type connection::make() {
   return static_cast<connection::make_type>(*factory_util::singleton<std::remove_reference_t<connection::make_type>>(
-      xutils::get_connection(), xutils::get_connection_fd()));
+      xutils::get_connection().get(), xutils::get_connection_fd()));
 }
 
 /**

src/x11/ewmh.cpp

@@ -11,10 +11,9 @@ namespace ewmh_util {
       g_ewmh_connection = memory_util::make_malloc_ptr<xcb_ewmh_connection_t>(
           sizeof(xcb_ewmh_connection_t), bind(xcb_ewmh_connection_wipe, std::placeholders::_1));
 
-      auto* xconn = xutils::get_connection();
       auto* conn = g_ewmh_connection.get();
 
-      xcb_ewmh_init_atoms_replies(conn, xcb_ewmh_init_atoms(xconn, conn), nullptr);
+      xcb_ewmh_init_atoms_replies(conn, xcb_ewmh_init_atoms(xutils::get_connection().get(), conn), nullptr);
     }
 
     return g_ewmh_connection;

src/x11/fonts.cpp

@@ -21,12 +21,13 @@ array<wchar_t, XFT_MAXCHARS> g_xft_chars;
  * Create instance
  */
 font_manager::make_type font_manager::make() {
-  return factory_util::unique<font_manager>(connection::make(), logger::make());
+  return factory_util::unique<font_manager>(
+      connection::make(), logger::make(), xlib::get_display(), xlib::get_visual());
 }
 
 void fonttype_deleter::operator()(fonttype* f) {
   if (f->xft != nullptr) {
-    XftFontClose(xlib::get_display(), f->xft);
+    XftFontClose(xlib::get_display().get(), f->xft);
     free(f->xft);
   }
   if (f->ptr != XCB_NONE) {
@@ -35,22 +36,22 @@ void fonttype_deleter::operator()(fonttype* f) {
   delete f;
 }
 
-font_manager::font_manager(connection& conn, const logger& logger) : m_connection(conn), m_logger(logger) {
+font_manager::font_manager(connection& conn, const logger& logger, shared_ptr<Display>&& dsp, shared_ptr<Visual>&& vis)
-  if ((m_display = xlib::get_display()) == nullptr) {
+    : m_connection(conn)
-    throw application_error("Failed to get X display");
+    , m_logger(logger)
-  }
+    , m_display(forward<decltype(dsp)>(dsp))
-  m_visual = xlib::get_visual(conn.default_screen());
+    , m_visual(forward<decltype(vis)>(vis)) {
   m_colormap = xlib::create_colormap(conn.default_screen());
 }
 
 font_manager::~font_manager() {
-  if (m_display != nullptr) {
+  if (m_display) {
     if (m_xftcolor != nullptr) {
-      XftColorFree(m_display, m_visual, m_colormap, m_xftcolor);
+      XftColorFree(m_display.get(), m_visual.get(), m_colormap, m_xftcolor);
       free(m_xftcolor);
     }
     destroy_xftdraw();
-    XFreeColormap(m_display, m_colormap);
+    XFreeColormap(m_display.get(), m_colormap);
   }
 }
 
@@ -75,7 +76,8 @@ bool font_manager::load(const string& name, int8_t fontindex, int8_t offset_y) {
     f->ptr = XCB_NONE;
   }
 
-  if (f->ptr == XCB_NONE && (f->xft = XftFontOpenName(m_display, m_connection.default_screen(), name.c_str())) != nullptr) {
+  if (f->ptr == XCB_NONE &&
+      (f->xft = XftFontOpenName(m_display.get(), m_connection.default_screen(), name.c_str())) != nullptr) {
     f->ascent = f->xft->ascent;
     f->descent = f->xft->descent;
     f->height = f->ascent + f->descent;
@@ -163,10 +165,10 @@ uint8_t font_manager::char_width(fonttype_pointer& font, uint16_t chr) {
 
   if (!g_xft_chars[index]) {
     XGlyphInfo gi;
-    FT_UInt glyph = XftCharIndex(m_display, font->xft, static_cast<FcChar32>(chr));
+    FT_UInt glyph = XftCharIndex(m_display.get(), font->xft, static_cast<FcChar32>(chr));
-    XftFontLoadGlyphs(m_display, font->xft, FcFalse, &glyph, 1);
+    XftFontLoadGlyphs(m_display.get(), font->xft, FcFalse, &glyph, 1);
-    XftGlyphExtents(m_display, font->xft, &glyph, 1, &gi);
+    XftGlyphExtents(m_display.get(), font->xft, &glyph, 1, &gi);
-    XftFontUnloadGlyphs(m_display, font->xft, &glyph, 1);
+    XftFontUnloadGlyphs(m_display.get(), font->xft, &glyph, 1);
     g_xft_chars[index] = chr;
     g_xft_widths[index] = gi.xOff;
     return gi.xOff;
@@ -187,7 +189,7 @@ XftDraw* font_manager::xftdraw() {
 
 void font_manager::create_xftdraw(xcb_pixmap_t pm) {
   destroy_xftdraw();
-  m_xftdraw = XftDrawCreate(m_display, pm, m_visual, m_colormap);
+  m_xftdraw = XftDrawCreate(m_display.get(), pm, m_visual.get(), m_colormap);
 }
 
 void font_manager::destroy_xftdraw() {
@@ -208,13 +210,13 @@ void font_manager::allocate_color(uint32_t color) {
 
 void font_manager::allocate_color(XRenderColor color) {
   if (m_xftcolor != nullptr) {
-    XftColorFree(m_display, m_visual, m_colormap, m_xftcolor);
+    XftColorFree(m_display.get(), m_visual.get(), m_colormap, m_xftcolor);
     free(m_xftcolor);
   }
 
   m_xftcolor = static_cast<XftColor*>(malloc(sizeof(XftColor)));
 
-  if (!XftColorAllocValue(m_display, m_visual, m_colormap, &color, m_xftcolor)) {
+  if (!XftColorAllocValue(m_display.get(), m_visual.get(), m_colormap, &color, m_xftcolor)) {
     m_logger.err("Failed to allocate color");
   }
 }
@@ -258,7 +260,7 @@ bool font_manager::open_xcb_font(fonttype_pointer& fontptr, string fontname) {
 
 bool font_manager::has_glyph(fonttype_pointer& font, uint16_t chr) {
   if (font->xft != nullptr) {
-    return static_cast<bool>(XftCharExists(m_display, font->xft, (FcChar32)chr));
+    return static_cast<bool>(XftCharExists(m_display.get(), font->xft, (FcChar32)chr));
   } else {
     if (chr < font->char_min || chr > font->char_max) {
       return false;

src/x11/tray_client.cpp

@@ -115,8 +115,8 @@ void tray_client::configure_notify(int16_t x, int16_t y) const {
   notify->height = m_height;
   notify->border_width = 0;
 
-  const char* data = reinterpret_cast<const char*>(notify.get());
+  uint32_t mask{XCB_EVENT_MASK_STRUCTURE_NOTIFY};
-  m_connection.send_event_checked(false, m_window, XCB_EVENT_MASK_STRUCTURE_NOTIFY, data);
+  m_connection.send_event_checked(false, m_window, mask, reinterpret_cast<const char*>(notify.get()));
 }
 
 POLYBAR_NS_END

src/x11/xlib.cpp

@@ -1,31 +1,45 @@
 #include "x11/xlib.hpp"
+#include "utils/factory.hpp"
 
 POLYBAR_NS
 
 namespace xlib {
-  Display* g_display = nullptr;
+  shared_ptr<Display> g_display_ptr;
-  XVisualInfo g_visual_info;
+  shared_ptr<Visual> g_visual_ptr;
 
-  /**
+  shared_ptr<Display> get_display() {
-   * Get pointer of Xlib Display
+    if (!g_display_ptr) {
-   */
+      // g_display_ptr = shared_ptr<Display>(XOpenDisplay(nullptr), bind(XCloseDisplay, placeholders::_1));
-  Display* get_display() {
+      g_display_ptr = shared_ptr<Display>(XOpenDisplay(nullptr), factory_util::null_deleter{});
-    if (g_display == nullptr) {
-      g_display = XOpenDisplay(nullptr);
     }
-    return g_display;
+    return g_display_ptr;
   }
 
-  Visual* get_visual(int screen) {
+  shared_ptr<Visual> get_visual(int screen) {
-    if (g_visual_info.visual == nullptr) {
+    if (!g_visual_ptr) {
-      XMatchVisualInfo(get_display(), screen, 32, TrueColor, &g_visual_info);
+      XVisualInfo info;
+      if (XMatchVisualInfo(get_display().get(), screen, 32, TrueColor, &info)) {
+        g_visual_ptr = shared_ptr<Visual>(info.visual, bind(XFree, placeholders::_1));
+      }
     }
 
-    return g_visual_info.visual;
+    return g_visual_ptr;
   }
 
   Colormap create_colormap(int screen) {
-    return XDefaultColormap(get_display(), screen);
+    return XDefaultColormap(get_display().get(), screen);
+  }
+
+  display_lock::display_lock(shared_ptr<Display>&& display) : m_display(forward<decltype(display)>(display)) {
+    XLockDisplay(m_display.get());
+  }
+
+  display_lock::~display_lock() {
+    XUnlockDisplay(m_display.get());
+  }
+
+  inline auto make_display_lock() {
+    return make_unique<display_lock>(get_display());
   }
 }
 

src/x11/xresources.cpp

@@ -13,21 +13,17 @@ POLYBAR_NS
  */
 xresource_manager::make_type xresource_manager::make() {
   return static_cast<xresource_manager::make_type>(
-      *factory_util::singleton<std::remove_reference_t<xresource_manager::make_type>>());
+      *factory_util::singleton<std::remove_reference_t<xresource_manager::make_type>>(xlib::get_display()));
 }
 
 /**
  * Construct manager instance
  */
-xresource_manager::xresource_manager() {
+xresource_manager::xresource_manager(shared_ptr<Display>&& dsp) : m_display(forward<decltype(dsp)>(dsp)) {
   XrmInitialize();
 
-  if ((m_display = xlib::get_display()) == nullptr) {
+  if ((m_manager = XResourceManagerString(m_display.get())) != nullptr) {
-    return;
+    m_db = XrmGetStringDatabase(m_manager);
-  } else if ((m_manager = XResourceManagerString(xlib::get_display())) == nullptr) {
-    return;
-  } else if ((m_db = XrmGetStringDatabase(m_manager)) == nullptr) {
-    return;
   }
 }
 
@@ -35,12 +31,12 @@ xresource_manager::xresource_manager() {
  * Deconstruct instance
  */
 xresource_manager::~xresource_manager() {
-  if (m_db != nullptr) {
-    XrmDestroyDatabase(m_db);
-  }
   if (m_manager != nullptr) {
     XFree(m_manager);
   }
+  if (m_db != nullptr) {
+    XrmDestroyDatabase(m_db);
+  }
 }
 
 /**

src/x11/xutils.cpp

@@ -13,22 +13,22 @@ namespace xutils {
   shared_ptr<int> g_connection_fd;
   shared_ptr<xcb_connection_t> g_connection_ptr;
 
-  xcb_connection_t* get_connection() {
+  shared_ptr<xcb_connection_t> get_connection() {
     if (!g_connection_ptr) {
-      Display* dsp{xlib::get_display()};
+      shared_ptr<Display> dsp{xlib::get_display()};
 
-      if (dsp != nullptr) {
+      if (dsp) {
-        XSetEventQueueOwner(dsp, XCBOwnsEventQueue);
+        XSetEventQueueOwner(dsp.get(), XCBOwnsEventQueue);
-        g_connection_ptr = shared_ptr<xcb_connection_t>(XGetXCBConnection(dsp), bind(xcb_disconnect, placeholders::_1));
+        g_connection_ptr = shared_ptr<xcb_connection_t>(XGetXCBConnection(dsp.get()), bind(xcb_disconnect, placeholders::_1));
       }
     }
 
-    return g_connection_ptr.get();
+    return g_connection_ptr;
   }
 
   int get_connection_fd() {
     if (!g_connection_fd) {
-      auto fd = xcb_get_file_descriptor(get_connection());
+      auto fd = xcb_get_file_descriptor(get_connection().get());
       g_connection_fd = shared_ptr<int>(new int{fd}, factory_util::fd_deleter{});
     }