From d3bc1f938f6c73ce4dc6133e204ff7afdf8c1005 Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Wed, 14 Dec 2016 15:09:11 +0100 Subject: [PATCH] refactor(x11): Use shared_ptr for X pointers --- include/x11/fonts.hpp | 6 +++--- include/x11/xlib.hpp | 26 ++++++++--------------- include/x11/xresources.hpp | 4 ++-- include/x11/xutils.hpp | 2 +- src/main.cpp | 6 +++--- src/x11/connection.cpp | 2 +- src/x11/ewmh.cpp | 3 +-- src/x11/fonts.cpp | 40 +++++++++++++++++++----------------- src/x11/tray_client.cpp | 4 ++-- src/x11/xlib.cpp | 42 +++++++++++++++++++++++++------------- src/x11/xresources.cpp | 18 +++++++--------- src/x11/xutils.cpp | 14 ++++++------- 12 files changed, 84 insertions(+), 83 deletions(-) diff --git a/include/x11/fonts.hpp b/include/x11/fonts.hpp index 0d1e5090..4952d119 100644 --- a/include/x11/fonts.hpp +++ b/include/x11/fonts.hpp @@ -38,7 +38,7 @@ class font_manager { using make_type = unique_ptr; static make_type make(); - explicit font_manager(connection& conn, const logger& logger); + explicit font_manager(connection& conn, const logger& logger, shared_ptr&& dsp, shared_ptr&& 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}; - Visual* m_visual{nullptr}; + shared_ptr m_display{nullptr}; + shared_ptr m_visual{nullptr}; Colormap m_colormap{}; std::map m_fonts; diff --git a/include/x11/xlib.hpp b/include/x11/xlib.hpp index 7ba838f0..ae4f3d4f 100644 --- a/include/x11/xlib.hpp +++ b/include/x11/xlib.hpp @@ -7,34 +7,24 @@ POLYBAR_NS namespace xlib { - extern Display* g_display; - extern XVisualInfo g_visual_info; + shared_ptr get_display(); + shared_ptr get_visual(int screen = 0); - extern Display* get_display(); - extern Visual* get_visual(int screen = 0); - extern Colormap create_colormap(int screen = 0); + Colormap create_colormap(int screen = 0); /** * RAII wrapper for Xlib display locking */ - class xlib_lock { + class display_lock { public: - explicit xlib_lock(Display* display) { - XLockDisplay(display); - m_display = display; - } - - ~xlib_lock() { - XUnlockDisplay(m_display); - } + explicit display_lock(shared_ptr&& display); + ~display_lock(); protected: - Display* m_display; + shared_ptr m_display; }; - inline auto make_display_lock() { - return make_unique(get_display()); - } + inline auto make_display_lock(); } POLYBAR_NS_END diff --git a/include/x11/xresources.hpp b/include/x11/xresources.hpp index 65ba2243..2890889c 100644 --- a/include/x11/xresources.hpp +++ b/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&&); ~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 m_display; XrmDatabase m_db; char* m_manager{nullptr}; }; diff --git a/include/x11/xutils.hpp b/include/x11/xutils.hpp index 16fce2d8..2458e574 100644 --- a/include/x11/xutils.hpp +++ b/include/x11/xutils.hpp @@ -13,7 +13,7 @@ class connection; class config; namespace xutils { - xcb_connection_t* get_connection(); + shared_ptr get_connection(); int get_connection_fd(); void pack_values(uint32_t mask, const uint32_t* src, uint32_t* dest); diff --git a/src/main.cpp b/src/main.cpp index f12454da..1f0bcd75 100644 --- a/src/main.cpp +++ b/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 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(); diff --git a/src/x11/connection.cpp b/src/x11/connection.cpp index c120c51f..c9d42840 100644 --- a/src/x11/connection.cpp +++ b/src/x11/connection.cpp @@ -13,7 +13,7 @@ POLYBAR_NS */ connection::make_type connection::make() { return static_cast(*factory_util::singleton>( - xutils::get_connection(), xutils::get_connection_fd())); + xutils::get_connection().get(), xutils::get_connection_fd())); } /** diff --git a/src/x11/ewmh.cpp b/src/x11/ewmh.cpp index 050d22a6..68bd0132 100644 --- a/src/x11/ewmh.cpp +++ b/src/x11/ewmh.cpp @@ -11,10 +11,9 @@ namespace ewmh_util { g_ewmh_connection = memory_util::make_malloc_ptr( 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; diff --git a/src/x11/fonts.cpp b/src/x11/fonts.cpp index 4f5cb76a..57018534 100644 --- a/src/x11/fonts.cpp +++ b/src/x11/fonts.cpp @@ -21,12 +21,13 @@ array g_xft_chars; * Create instance */ font_manager::make_type font_manager::make() { - return factory_util::unique(connection::make(), logger::make()); + return factory_util::unique( + 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) { - if ((m_display = xlib::get_display()) == nullptr) { - throw application_error("Failed to get X display"); - } - m_visual = xlib::get_visual(conn.default_screen()); +font_manager::font_manager(connection& conn, const logger& logger, shared_ptr&& dsp, shared_ptr&& vis) + : m_connection(conn) + , m_logger(logger) + , m_display(forward(dsp)) + , m_visual(forward(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(chr)); - XftFontLoadGlyphs(m_display, font->xft, FcFalse, &glyph, 1); - XftGlyphExtents(m_display, font->xft, &glyph, 1, &gi); - XftFontUnloadGlyphs(m_display, font->xft, &glyph, 1); + FT_UInt glyph = XftCharIndex(m_display.get(), font->xft, static_cast(chr)); + XftFontLoadGlyphs(m_display.get(), font->xft, FcFalse, &glyph, 1); + XftGlyphExtents(m_display.get(), font->xft, &glyph, 1, &gi); + 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(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(XftCharExists(m_display, font->xft, (FcChar32)chr)); + return static_cast(XftCharExists(m_display.get(), font->xft, (FcChar32)chr)); } else { if (chr < font->char_min || chr > font->char_max) { return false; diff --git a/src/x11/tray_client.cpp b/src/x11/tray_client.cpp index 6db08c08..2f467b0a 100644 --- a/src/x11/tray_client.cpp +++ b/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(notify.get()); - m_connection.send_event_checked(false, m_window, XCB_EVENT_MASK_STRUCTURE_NOTIFY, data); + uint32_t mask{XCB_EVENT_MASK_STRUCTURE_NOTIFY}; + m_connection.send_event_checked(false, m_window, mask, reinterpret_cast(notify.get())); } POLYBAR_NS_END diff --git a/src/x11/xlib.cpp b/src/x11/xlib.cpp index 011627cb..2798287b 100644 --- a/src/x11/xlib.cpp +++ b/src/x11/xlib.cpp @@ -1,31 +1,45 @@ #include "x11/xlib.hpp" +#include "utils/factory.hpp" POLYBAR_NS namespace xlib { - Display* g_display = nullptr; - XVisualInfo g_visual_info; + shared_ptr g_display_ptr; + shared_ptr g_visual_ptr; - /** - * Get pointer of Xlib Display - */ - Display* get_display() { - if (g_display == nullptr) { - g_display = XOpenDisplay(nullptr); + shared_ptr get_display() { + if (!g_display_ptr) { + // g_display_ptr = shared_ptr(XOpenDisplay(nullptr), bind(XCloseDisplay, placeholders::_1)); + g_display_ptr = shared_ptr(XOpenDisplay(nullptr), factory_util::null_deleter{}); } - return g_display; + return g_display_ptr; } - Visual* get_visual(int screen) { - if (g_visual_info.visual == nullptr) { - XMatchVisualInfo(get_display(), screen, 32, TrueColor, &g_visual_info); + shared_ptr get_visual(int screen) { + if (!g_visual_ptr) { + XVisualInfo info; + if (XMatchVisualInfo(get_display().get(), screen, 32, TrueColor, &info)) { + g_visual_ptr = shared_ptr(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) : m_display(forward(display)) { + XLockDisplay(m_display.get()); + } + + display_lock::~display_lock() { + XUnlockDisplay(m_display.get()); + } + + inline auto make_display_lock() { + return make_unique(get_display()); } } diff --git a/src/x11/xresources.cpp b/src/x11/xresources.cpp index 538a124e..0660e2e1 100644 --- a/src/x11/xresources.cpp +++ b/src/x11/xresources.cpp @@ -13,21 +13,17 @@ POLYBAR_NS */ xresource_manager::make_type xresource_manager::make() { return static_cast( - *factory_util::singleton>()); + *factory_util::singleton>(xlib::get_display())); } /** * Construct manager instance */ -xresource_manager::xresource_manager() { +xresource_manager::xresource_manager(shared_ptr&& dsp) : m_display(forward(dsp)) { XrmInitialize(); - if ((m_display = xlib::get_display()) == nullptr) { - return; - } else if ((m_manager = XResourceManagerString(xlib::get_display())) == nullptr) { - return; - } else if ((m_db = XrmGetStringDatabase(m_manager)) == nullptr) { - return; + if ((m_manager = XResourceManagerString(m_display.get())) != nullptr) { + m_db = XrmGetStringDatabase(m_manager); } } @@ -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); + } } /** diff --git a/src/x11/xutils.cpp b/src/x11/xutils.cpp index 3dd0766e..5aceac78 100644 --- a/src/x11/xutils.cpp +++ b/src/x11/xutils.cpp @@ -13,22 +13,22 @@ namespace xutils { shared_ptr g_connection_fd; shared_ptr g_connection_ptr; - xcb_connection_t* get_connection() { + shared_ptr get_connection() { if (!g_connection_ptr) { - Display* dsp{xlib::get_display()}; + shared_ptr dsp{xlib::get_display()}; - if (dsp != nullptr) { - XSetEventQueueOwner(dsp, XCBOwnsEventQueue); - g_connection_ptr = shared_ptr(XGetXCBConnection(dsp), bind(xcb_disconnect, placeholders::_1)); + if (dsp) { + XSetEventQueueOwner(dsp.get(), XCBOwnsEventQueue); + g_connection_ptr = shared_ptr(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(new int{fd}, factory_util::fd_deleter{}); }