From 95ec9180ad80638fc29da32c2da6fe54094c7a76 Mon Sep 17 00:00:00 2001 From: binary1248 Date: Fri, 6 Mar 2015 18:20:31 +0100 Subject: [PATCH] Wrapped XCB replies in scoped pointers. --- src/SFML/Window/Unix/InputImpl.cpp | 37 ++++----- src/SFML/Window/Unix/ScopedXcbPtr.hpp | 102 +++++++++++++++++++++++++ src/SFML/Window/Unix/ScopedXcbPtr.inl | 72 +++++++++++++++++ src/SFML/Window/Unix/VideoModeImpl.cpp | 39 ++++------ src/SFML/Window/Unix/WindowImplX11.cpp | 93 ++++++++-------------- 5 files changed, 234 insertions(+), 109 deletions(-) create mode 100644 src/SFML/Window/Unix/ScopedXcbPtr.hpp create mode 100644 src/SFML/Window/Unix/ScopedXcbPtr.inl diff --git a/src/SFML/Window/Unix/InputImpl.cpp b/src/SFML/Window/Unix/InputImpl.cpp index 8ac0925c..ecdf47fa 100644 --- a/src/SFML/Window/Unix/InputImpl.cpp +++ b/src/SFML/Window/Unix/InputImpl.cpp @@ -25,9 +25,10 @@ //////////////////////////////////////////////////////////// // Headers //////////////////////////////////////////////////////////// +#include // important to be included first (conflict with None) #include -#include #include +#include #include #include #include @@ -157,16 +158,13 @@ bool InputImpl::isKeyPressed(Keyboard::Key key) if (keycode != 0) { // Get the whole keyboard state - xcb_query_keymap_reply_t* keymap = xcb_query_keymap_reply(connection, xcb_query_keymap(connection), NULL); + ScopedXcbPtr keymap(xcb_query_keymap_reply(connection, xcb_query_keymap(connection), NULL)); // Close the connection with the X server CloseDisplay(display); // Check our keycode - bool isPressed = (keymap->keys[keycode / 8] & (1 << (keycode % 8))) != 0; - - free(keymap); - return isPressed; + return (keymap->keys[keycode / 8] & (1 << (keycode % 8))) != 0; } else { @@ -192,18 +190,17 @@ bool InputImpl::isMouseButtonPressed(Mouse::Button button) xcb_connection_t* connection = OpenConnection(); // Get pointer mask - xcb_query_pointer_reply_t* pointer = xcb_query_pointer_reply(connection, xcb_query_pointer(connection, XCBDefaultRootWindow(connection)), NULL); - uint16_t mask = pointer->mask; - free(pointer); + ScopedXcbPtr pointer(xcb_query_pointer_reply(connection, xcb_query_pointer(connection, XCBDefaultRootWindow(connection)), NULL)); + uint16_t buttons = pointer->mask; // Close the connection with the X server CloseConnection(connection); switch (button) { - case Mouse::Left: return mask & XCB_BUTTON_MASK_1; - case Mouse::Right: return mask & XCB_BUTTON_MASK_3; - case Mouse::Middle: return mask & XCB_BUTTON_MASK_2; + case Mouse::Left: return buttons & XCB_BUTTON_MASK_1; + case Mouse::Right: return buttons & XCB_BUTTON_MASK_3; + case Mouse::Middle: return buttons & XCB_BUTTON_MASK_2; case Mouse::XButton1: return false; // not supported by X case Mouse::XButton2: return false; // not supported by X default: return false; @@ -217,16 +214,12 @@ Vector2i InputImpl::getMousePosition() // Open a connection with the X server xcb_connection_t* connection = OpenConnection(); - xcb_query_pointer_reply_t* pointer = xcb_query_pointer_reply(connection, xcb_query_pointer(connection, XCBDefaultRootWindow(connection)), NULL); + ScopedXcbPtr pointer(xcb_query_pointer_reply(connection, xcb_query_pointer(connection, XCBDefaultRootWindow(connection)), NULL)); // Close the connection with the X server CloseConnection(connection); - // Prepare result. - Vector2i result(pointer->root_x, pointer->root_y); - free(pointer); - - return result; + return Vector2i(pointer->root_x, pointer->root_y); } @@ -239,16 +232,12 @@ Vector2i InputImpl::getMousePosition(const Window& relativeTo) // Open a connection with the X server xcb_connection_t* connection = OpenConnection(); - xcb_query_pointer_reply_t* pointer = xcb_query_pointer_reply(connection, xcb_query_pointer(connection, handle), NULL); + ScopedXcbPtr pointer(xcb_query_pointer_reply(connection, xcb_query_pointer(connection, handle), NULL)); // Close the connection with the X server CloseConnection(connection); - // Prepare result. - Vector2i result(pointer->win_x, pointer->win_y); - free(pointer); - - return result; + return Vector2i(pointer->win_x, pointer->win_y); } else { diff --git a/src/SFML/Window/Unix/ScopedXcbPtr.hpp b/src/SFML/Window/Unix/ScopedXcbPtr.hpp new file mode 100644 index 00000000..6c98065d --- /dev/null +++ b/src/SFML/Window/Unix/ScopedXcbPtr.hpp @@ -0,0 +1,102 @@ +//////////////////////////////////////////////////////////// +// +// SFML - Simple and Fast Multimedia Library +// Copyright (C) 2007-2015 Laurent Gomila (laurent@sfml-dev.org) +// +// This software is provided 'as-is', without any express or implied warranty. +// In no event will the authors be held liable for any damages arising from the use of this software. +// +// Permission is granted to anyone to use this software for any purpose, +// including commercial applications, and to alter it and redistribute it freely, +// subject to the following restrictions: +// +// 1. The origin of this software must not be misrepresented; +// you must not claim that you wrote the original software. +// If you use this software in a product, an acknowledgment +// in the product documentation would be appreciated but is not required. +// +// 2. Altered source versions must be plainly marked as such, +// and must not be misrepresented as being the original software. +// +// 3. This notice may not be removed or altered from any source distribution. +// +//////////////////////////////////////////////////////////// + +#ifndef SFML_SCOPEDXCBPTR_HPP +#define SFML_SCOPEDXCBPTR_HPP + +//////////////////////////////////////////////////////////// +// Headers +//////////////////////////////////////////////////////////// +#include + + +namespace sf +{ +namespace priv +{ +//////////////////////////////////////////////////////////// +/// \brief Scoped pointer that frees memory returned in XCB replies +/// +//////////////////////////////////////////////////////////// +template +class ScopedXcbPtr +{ +public: + //////////////////////////////////////////////////////////// + /// \brief Constructor + /// + /// \param pointer Pointer value to store + /// + //////////////////////////////////////////////////////////// + ScopedXcbPtr(T* pointer); + + //////////////////////////////////////////////////////////// + /// \brief Destructor, calls std::free() on the stored pointer + /// + //////////////////////////////////////////////////////////// + ~ScopedXcbPtr(); + + //////////////////////////////////////////////////////////// + /// \brief Structure dereference operator + /// + /// \return Stored pointer + /// + //////////////////////////////////////////////////////////// + T* operator ->() const; + + //////////////////////////////////////////////////////////// + /// \brief Address operator. + /// + /// \return Address of the stored pointer + /// + //////////////////////////////////////////////////////////// + T** operator &(); + + //////////////////////////////////////////////////////////// + /// \brief Check if stored pointer is valid + /// + /// \return true if stored pointer is valid + /// + //////////////////////////////////////////////////////////// + operator bool() const; + + //////////////////////////////////////////////////////////// + /// \brief Retrieve the stored pointer. + /// + /// \return The stored pointer + /// + //////////////////////////////////////////////////////////// + T* get() const; + +private: + T* m_pointer; ///< Stored pointer +}; + +#include + +} // namespace priv + +} // namespace sf + +#endif // SFML_SCOPEDXCBPTR_HPP diff --git a/src/SFML/Window/Unix/ScopedXcbPtr.inl b/src/SFML/Window/Unix/ScopedXcbPtr.inl new file mode 100644 index 00000000..6683a1e6 --- /dev/null +++ b/src/SFML/Window/Unix/ScopedXcbPtr.inl @@ -0,0 +1,72 @@ +//////////////////////////////////////////////////////////// +// +// SFML - Simple and Fast Multimedia Library +// Copyright (C) 2007-2015 Laurent Gomila (laurent@sfml-dev.org) +// +// This software is provided 'as-is', without any express or implied warranty. +// In no event will the authors be held liable for any damages arising from the use of this software. +// +// Permission is granted to anyone to use this software for any purpose, +// including commercial applications, and to alter it and redistribute it freely, +// subject to the following restrictions: +// +// 1. The origin of this software must not be misrepresented; +// you must not claim that you wrote the original software. +// If you use this software in a product, an acknowledgment +// in the product documentation would be appreciated but is not required. +// +// 2. Altered source versions must be plainly marked as such, +// and must not be misrepresented as being the original software. +// +// 3. This notice may not be removed or altered from any source distribution. +// +//////////////////////////////////////////////////////////// + + +//////////////////////////////////////////////////////////// +template +inline ScopedXcbPtr::ScopedXcbPtr(T* pointer) : +m_pointer(pointer) +{ + +} + + +//////////////////////////////////////////////////////////// +template +inline ScopedXcbPtr::~ScopedXcbPtr() +{ + std::free(m_pointer); +} + + +//////////////////////////////////////////////////////////// +template +inline T* ScopedXcbPtr::operator ->() const +{ + return m_pointer; +} + + +//////////////////////////////////////////////////////////// +template +inline T** ScopedXcbPtr::operator &() +{ + return &m_pointer; +} + + +//////////////////////////////////////////////////////////// +template +inline ScopedXcbPtr::operator bool() const +{ + return m_pointer != NULL; +} + + +//////////////////////////////////////////////////////////// +template +inline T* ScopedXcbPtr::get() const +{ + return m_pointer; +} diff --git a/src/SFML/Window/Unix/VideoModeImpl.cpp b/src/SFML/Window/Unix/VideoModeImpl.cpp index ce5bc1ea..cef67bcd 100644 --- a/src/SFML/Window/Unix/VideoModeImpl.cpp +++ b/src/SFML/Window/Unix/VideoModeImpl.cpp @@ -27,6 +27,7 @@ //////////////////////////////////////////////////////////// #include #include +#include #include #include #include @@ -49,7 +50,7 @@ std::vector VideoModeImpl::getFullscreenModes() // Check if the XRandR extension is present static const std::string RANDR = "RANDR"; - xcb_query_extension_reply_t* randr_ext = xcb_query_extension_reply( + ScopedXcbPtr randr_ext(xcb_query_extension_reply( connection, xcb_query_extension( connection, @@ -57,25 +58,25 @@ std::vector VideoModeImpl::getFullscreenModes() RANDR.c_str() ), NULL - ); + )); if (randr_ext->present) { // Get the current configuration - xcb_generic_error_t* error; - xcb_randr_get_screen_info_reply_t* config = xcb_randr_get_screen_info_reply( + ScopedXcbPtr error(NULL); + ScopedXcbPtr config(xcb_randr_get_screen_info_reply( connection, xcb_randr_get_screen_info( connection, screen->root ), &error - ); + )); if (!error) { // Get the available screen sizes - xcb_randr_screen_size_t* sizes = xcb_randr_get_screen_info_sizes(config); + xcb_randr_screen_size_t* sizes = xcb_randr_get_screen_info_sizes(config.get()); if (sizes && (config->nSizes > 0)) { // Get the list of supported depths @@ -100,10 +101,6 @@ std::vector VideoModeImpl::getFullscreenModes() // Failed to get the screen configuration err() << "Failed to retrieve the screen configuration while trying to get the supported video modes" << std::endl; } - - // Free the configuration instance - free(error); - free(config); } else { @@ -111,8 +108,6 @@ std::vector VideoModeImpl::getFullscreenModes() err() << "Failed to use the XRandR extension while trying to get the supported video modes" << std::endl; } - free(randr_ext); - // Close the connection with the X server CloseConnection(connection); @@ -133,7 +128,7 @@ VideoMode VideoModeImpl::getDesktopMode() // Check if the XRandR extension is present static const std::string RANDR = "RANDR"; - xcb_query_extension_reply_t* randr_ext = xcb_query_extension_reply( + ScopedXcbPtr randr_ext(xcb_query_extension_reply( connection, xcb_query_extension( connection, @@ -141,20 +136,20 @@ VideoMode VideoModeImpl::getDesktopMode() RANDR.c_str() ), NULL - ); + )); if (randr_ext->present) { // Get the current configuration - xcb_generic_error_t* error; - xcb_randr_get_screen_info_reply_t* config = xcb_randr_get_screen_info_reply( + ScopedXcbPtr error(NULL); + ScopedXcbPtr config(xcb_randr_get_screen_info_reply( connection, xcb_randr_get_screen_info( connection, screen->root ), &error - ); + )); if (!error) { @@ -162,8 +157,8 @@ VideoMode VideoModeImpl::getDesktopMode() xcb_randr_mode_t currentMode = config->sizeID; // Get the available screen sizes - int nbSizes = xcb_randr_get_screen_info_sizes_length(config); - xcb_randr_screen_size_t* sizes = xcb_randr_get_screen_info_sizes(config); + int nbSizes = xcb_randr_get_screen_info_sizes_length(config.get()); + xcb_randr_screen_size_t* sizes = xcb_randr_get_screen_info_sizes(config.get()); if (sizes && (nbSizes > 0)) desktopMode = VideoMode(sizes[currentMode].width, sizes[currentMode].height, screen->root_depth); } @@ -172,10 +167,6 @@ VideoMode VideoModeImpl::getDesktopMode() // Failed to get the screen configuration err() << "Failed to retrieve the screen configuration while trying to get the desktop video modes" << std::endl; } - - // Free the configuration instance - free(error); - free(config); } else { @@ -183,8 +174,6 @@ VideoMode VideoModeImpl::getDesktopMode() err() << "Failed to use the XRandR extension while trying to get the desktop video modes" << std::endl; } - free(randr_ext); - // Close the connection with the X server CloseConnection(connection); diff --git a/src/SFML/Window/Unix/WindowImplX11.cpp b/src/SFML/Window/Unix/WindowImplX11.cpp index b4188842..da838e3f 100644 --- a/src/SFML/Window/Unix/WindowImplX11.cpp +++ b/src/SFML/Window/Unix/WindowImplX11.cpp @@ -28,6 +28,7 @@ #include // important to be included first (conflict with None) #include #include +#include #include #include #include @@ -181,7 +182,7 @@ m_useSizeHints(false) // Create the window m_window = xcb_generate_id(m_connection); - xcb_generic_error_t* errptr = xcb_request_check( + ScopedXcbPtr errptr(xcb_request_check( m_connection, xcb_create_window_checked( m_connection, @@ -196,12 +197,9 @@ m_useSizeHints(false) XCB_CW_EVENT_MASK | XCB_CW_OVERRIDE_REDIRECT, value_list ) - ); + )); - bool createWindowFailed = (errptr != NULL); - free(errptr); - - if (createWindowFailed) + if (errptr) { err() << "Failed to create window" << std::endl; return; @@ -214,7 +212,7 @@ m_useSizeHints(false) if (!fullscreen) { static const std::string MOTIF_WM_HINTS = "_MOTIF_WM_HINTS"; - xcb_intern_atom_reply_t* hintsAtomReply = xcb_intern_atom_reply( + ScopedXcbPtr hintsAtomReply(xcb_intern_atom_reply( m_connection, xcb_intern_atom( m_connection, @@ -223,7 +221,7 @@ m_useSizeHints(false) MOTIF_WM_HINTS.c_str() ), NULL - ); + )); if (hintsAtomReply) { @@ -285,8 +283,6 @@ m_useSizeHints(false) sizeof(hints) / sizeof(hints.flags), reinterpret_cast(&hints) ); - - free(hintsAtomReply); } // This is a hack to force some windows managers to disable resizing @@ -442,30 +438,26 @@ Vector2i WindowImplX11::getPosition() const { ::Window topLevelWindow = m_window; ::Window nextWindow = topLevelWindow; - xcb_query_tree_reply_t* treeReply = NULL; // Get "top level" window, i.e. the window with the root window as its parent. while (nextWindow != m_screen->root) { topLevelWindow = nextWindow; - treeReply = xcb_query_tree_reply(m_connection, xcb_query_tree(m_connection, topLevelWindow), NULL); + ScopedXcbPtr treeReply(xcb_query_tree_reply(m_connection, xcb_query_tree(m_connection, topLevelWindow), NULL)); nextWindow = treeReply->parent; - free(treeReply); } - xcb_get_geometry_reply_t* geometryReply = xcb_get_geometry_reply( + ScopedXcbPtr geometryReply(xcb_get_geometry_reply( m_connection, xcb_get_geometry( m_connection, topLevelWindow ), NULL - ); - sf::Vector2i result(geometryReply->x, geometryReply->y); - free(geometryReply); + )); - return result; + return sf::Vector2i(geometryReply->x, geometryReply->y); } @@ -483,11 +475,9 @@ void WindowImplX11::setPosition(const Vector2i& position) //////////////////////////////////////////////////////////// Vector2u WindowImplX11::getSize() const { - xcb_get_geometry_reply_t* reply = xcb_get_geometry_reply(m_connection, xcb_get_geometry(m_connection, m_window), NULL); - Vector2u result(reply->width, reply->height); - free(reply); + ScopedXcbPtr reply(xcb_get_geometry_reply(m_connection, xcb_get_geometry(m_connection, m_window), NULL)); - return result; + return Vector2u(reply->width, reply->height); } @@ -565,7 +555,7 @@ void WindowImplX11::setIcon(unsigned int width, unsigned int height, const Uint8 xcb_gcontext_t iconGC = xcb_generate_id(m_connection); xcb_create_gc(m_connection, iconGC, iconPixmap, 0, NULL); - xcb_generic_error_t* errptr = xcb_request_check( + ScopedXcbPtr errptr(xcb_request_check( m_connection, xcb_put_image_checked( m_connection, @@ -581,14 +571,13 @@ void WindowImplX11::setIcon(unsigned int width, unsigned int height, const Uint8 sizeof(iconPixels), iconPixels ) - ); + )); xcb_free_gc(m_connection, iconGC); if (errptr) { err() << "Failed to set the window's icon: Error code " << static_cast(errptr->error_code) << std::endl; - free(errptr); return; } @@ -679,14 +668,15 @@ void WindowImplX11::requestFocus() // Check if window is viewable (not on other desktop, ...) // TODO: Check also if minimized - xcb_get_window_attributes_reply_t* attributes = xcb_get_window_attributes_reply( + ScopedXcbPtr attributes(xcb_get_window_attributes_reply( m_connection, xcb_get_window_attributes( m_connection, m_window ), NULL - ); + )); + if (!attributes) { sf::err() << "Failed to check if window is viewable while requesting focus" << std::endl; @@ -694,7 +684,6 @@ void WindowImplX11::requestFocus() } bool windowViewable = (attributes->map_state == XCB_MAP_STATE_VIEWABLE); - free(attributes); if (sfmlWindowFocused && windowViewable) { @@ -729,18 +718,15 @@ void WindowImplX11::requestFocus() //////////////////////////////////////////////////////////// bool WindowImplX11::hasFocus() const { - xcb_get_input_focus_reply_t* reply = xcb_get_input_focus_reply( + ScopedXcbPtr reply(xcb_get_input_focus_reply( m_connection, xcb_get_input_focus_unchecked( m_connection ), NULL - ); + )); - bool focused = (reply->focus == m_window); - free(reply); - - return focused; + return (reply->focus == m_window); } @@ -749,7 +735,7 @@ void WindowImplX11::switchToFullscreen(const VideoMode& mode) { // Check if the XRandR extension is present static const std::string RANDR = "RANDR"; - xcb_query_extension_reply_t* randr_ext = xcb_query_extension_reply( + ScopedXcbPtr randr_ext(xcb_query_extension_reply( m_connection, xcb_query_extension( m_connection, @@ -757,20 +743,20 @@ void WindowImplX11::switchToFullscreen(const VideoMode& mode) RANDR.c_str() ), NULL - ); + )); if (randr_ext->present) { // Get the current configuration - xcb_generic_error_t* error; - xcb_randr_get_screen_info_reply_t* config = xcb_randr_get_screen_info_reply( + ScopedXcbPtr error(NULL); + ScopedXcbPtr config(xcb_randr_get_screen_info_reply( m_connection, xcb_randr_get_screen_info( m_connection, m_screen->root ), &error - ); + )); if (!error) { @@ -778,7 +764,7 @@ void WindowImplX11::switchToFullscreen(const VideoMode& mode) m_oldVideoMode = config->sizeID; // Get the available screen sizes - xcb_randr_screen_size_t* sizes = xcb_randr_get_screen_info_sizes(config); + xcb_randr_screen_size_t* sizes = xcb_randr_get_screen_info_sizes(config.get()); if (sizes && (config->nSizes > 0)) { // Search a matching size @@ -810,18 +796,12 @@ void WindowImplX11::switchToFullscreen(const VideoMode& mode) // Failed to get the screen configuration err() << "Failed to get the current screen configuration for fullscreen mode, switching to window mode" << std::endl; } - - // Free the configuration instance - free(error); - free(config); } else { // XRandR extension is not supported: we cannot use fullscreen mode err() << "Fullscreen is not supported, switching to window mode" << std::endl; } - - free(randr_ext); } @@ -830,7 +810,7 @@ void WindowImplX11::initialize() { // Get the atoms for registering the close event static const std::string WM_DELETE_WINDOW_NAME = "WM_DELETE_WINDOW"; - xcb_intern_atom_reply_t* deleteWindowAtomReply = xcb_intern_atom_reply( + ScopedXcbPtr deleteWindowAtomReply(xcb_intern_atom_reply( m_connection, xcb_intern_atom( m_connection, @@ -839,10 +819,10 @@ void WindowImplX11::initialize() WM_DELETE_WINDOW_NAME.c_str() ), NULL - ); + )); static const std::string WM_PROTOCOLS_NAME = "WM_PROTOCOLS"; - xcb_intern_atom_reply_t* protocolsAtomReply = xcb_intern_atom_reply( + ScopedXcbPtr protocolsAtomReply(xcb_intern_atom_reply( m_connection, xcb_intern_atom( m_connection, @@ -851,7 +831,7 @@ void WindowImplX11::initialize() WM_PROTOCOLS_NAME.c_str() ), NULL - ); + )); if (protocolsAtomReply && deleteWindowAtomReply) { @@ -871,9 +851,6 @@ void WindowImplX11::initialize() std::cerr << "Failed to request WM_PROTOCOLS/WM_DELETE_WINDOW_NAME atoms." << std::endl; } - free(protocolsAtomReply); - free(deleteWindowAtomReply); - // Create the input context m_inputMethod = XOpenIM(m_display, NULL, NULL, NULL); @@ -945,15 +922,15 @@ void WindowImplX11::cleanup() if (fullscreenWindow == this) { // Get current screen info - xcb_generic_error_t* error; - xcb_randr_get_screen_info_reply_t* config = xcb_randr_get_screen_info_reply( + ScopedXcbPtr error(NULL); + ScopedXcbPtr config(xcb_randr_get_screen_info_reply( m_connection, xcb_randr_get_screen_info( m_connection, m_screen->root ), &error - ); + )); if (!error) { @@ -969,10 +946,6 @@ void WindowImplX11::cleanup() ); } - // Free the configuration instance - free(error); - free(config); - // Reset the fullscreen window fullscreenWindow = NULL; }