xcb: Enhance SHM management code

- extract the creation of a shared memory segment into a separate
  function
- do extra checks for errors
- check that MIT-SHM extension is present once in QXcbConnection

Change-Id: I956bdf76b879ec5c95a7ed219a59ae722dc5afba
Reviewed-by: Gatis Paeglis <gatis.paeglis@qt.io>

3 files changed, 91 insertions, 44 deletions

diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp
index 1cf45c96d1..e69bc5cba5 100644
--- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp
+++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp
@@ -85,6 +85,9 @@ public:
     void preparePaint(const QRegion &region);
 
 private:
+    void createShmSegment(size_t segmentSize);
+    void destroyShmSegment();
+
     void destroy();
 
     void ensureGC(xcb_drawable_t dst);
@@ -173,39 +176,13 @@ QXcbShmImage::QXcbShmImage(QXcbScreen *screen, const QSize &size, uint depth, QI
                                    XCB_IMAGE_ORDER_MSB_FIRST,
                                    0, ~0, 0);
 
-    const int segmentSize = m_xcb_image->stride * m_xcb_image->height;
+    const size_t segmentSize = static_cast<size_t>(m_xcb_image->stride) * m_xcb_image->height;
     if (!segmentSize)
         return;
 
-    int id = shmget(IPC_PRIVATE, segmentSize, IPC_CREAT | 0600);
-    if (id == -1) {
-        qWarning("QXcbShmImage: shmget() failed (%d: %s) for size %d (%dx%d)",
-                 errno, strerror(errno), segmentSize, size.width(), size.height());
-    } else {
-        m_shm_info.shmaddr = m_xcb_image->data = (quint8 *)shmat(id, 0, 0);
-    }
-    m_shm_info.shmid = id;
-    m_shm_info.shmseg = xcb_generate_id(xcb_connection());
-
-    const xcb_query_extension_reply_t *shm_reply = xcb_get_extension_data(xcb_connection(), &xcb_shm_id);
-    bool shm_present = shm_reply != NULL && shm_reply->present;
-    xcb_generic_error_t *error = NULL;
-    if (shm_present)
-        error = xcb_request_check(xcb_connection(), xcb_shm_attach_checked(xcb_connection(), m_shm_info.shmseg, m_shm_info.shmid, false));
-    if (!shm_present || error || id == -1) {
-        free(error);
+    createShmSegment(segmentSize);
 
-        if (id != -1) {
-            shmdt(m_shm_info.shmaddr);
-            shmctl(m_shm_info.shmid, IPC_RMID, 0);
-        }
-        m_shm_info.shmaddr = 0;
-
-        m_xcb_image->data = (uint8_t *)malloc(segmentSize);
-    } else {
-        if (shmctl(m_shm_info.shmid, IPC_RMID, 0) == -1)
-            qWarning("QXcbBackingStore: Error while marking the shared memory segment to be destroyed");
-    }
+    m_xcb_image->data = m_shm_info.shmaddr ? m_shm_info.shmaddr : (uint8_t *)malloc(segmentSize);
 
     m_hasAlpha = QImage::toPixelFormat(format).alphaUsage() == QPixelFormat::UsesAlpha;
     if (!m_hasAlpha)
@@ -270,6 +247,60 @@ void QXcbShmImage::flushScrolledRegion(bool clientSideScroll)
     }
 }
 
+void QXcbShmImage::createShmSegment(size_t segmentSize)
+{
+    m_shm_info.shmaddr = nullptr;
+    if (!connection()->hasShm())
+        return;
+
+    const int id = shmget(IPC_PRIVATE, segmentSize, IPC_CREAT | 0600);
+    if (id == -1) {
+        qWarning("QXcbShmImage: shmget() failed (%d: %s) for size %zu",
+                 errno, strerror(errno), segmentSize);
+        return;
+    }
+
+    void *addr = shmat(id, 0, 0);
+    if (addr == (void *)-1) {
+        qWarning("QXcbShmImage: shmat() failed (%d: %s) for id %d",
+                 errno, strerror(errno), id);
+        return;
+    }
+
+    if (shmctl(id, IPC_RMID, 0) == -1)
+        qWarning("QXcbBackingStore: Error while marking the shared memory segment to be destroyed");
+
+    const auto seg = xcb_generate_id(xcb_connection());
+    auto cookie = xcb_shm_attach_checked(xcb_connection(), seg, id, false);
+    auto *error = xcb_request_check(xcb_connection(), cookie);
+    if (error) {
+        connection()->printXcbError("QXcbShmImage: xcb_shm_attach() failed with error", error);
+        free(error);
+        if (shmdt(addr) == -1) {
+            qWarning("QXcbShmImage: shmdt() failed (%d: %s) for %p",
+                     errno, strerror(errno), addr);
+        }
+        return;
+    }
+
+    m_shm_info.shmseg = seg;
+    m_shm_info.shmid = id; // unused
+    m_shm_info.shmaddr = static_cast<quint8 *>(addr);
+}
+
+void QXcbShmImage::destroyShmSegment()
+{
+    auto cookie = xcb_shm_detach_checked(xcb_connection(), m_shm_info.shmseg);
+    xcb_generic_error_t *error = xcb_request_check(xcb_connection(), cookie);
+    if (error)
+        connection()->printXcbError("QXcbShmImage: xcb_shm_detach() failed with error", error);
+
+    if (shmdt(m_shm_info.shmaddr) == -1) {
+        qWarning("QXcbShmImage: shmdt() failed (%d: %s) for %p",
+                 errno, strerror(errno), m_shm_info.shmaddr);
+    }
+}
+
 extern void qt_scrollRectInImage(QImage &img, const QRect &rect, const QPoint &offset);
 
 bool QXcbShmImage::scroll(const QRegion &area, int dx, int dy)
@@ -318,17 +349,11 @@ bool QXcbShmImage::scroll(const QRegion &area, int dx, int dy)
 
 void QXcbShmImage::destroy()
 {
-    const int segmentSize = m_xcb_image ? (m_xcb_image->stride * m_xcb_image->height) : 0;
-    if (segmentSize && m_shm_info.shmaddr)
-        xcb_shm_detach(xcb_connection(), m_shm_info.shmseg);
-
-    if (segmentSize) {
-        if (m_shm_info.shmaddr) {
-            shmdt(m_shm_info.shmaddr);
-            shmctl(m_shm_info.shmid, IPC_RMID, 0);
-        } else {
+    if (m_xcb_image->data) {
+        if (m_shm_info.shmaddr)
+            destroyShmSegment();
+        else
             free(m_xcb_image->data);
-        }
     }
 
     xcb_image_destroy(m_xcb_image);
diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp
index c5eae20266..e8bb97c6af 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection.cpp
@@ -582,6 +582,7 @@ QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, bool canGra
 
     initializeAllAtoms();
 
+    initializeShm();
     if (!qEnvironmentVariableIsSet("QT_XCB_NO_XRANDR"))
         initializeXRandr();
     if (!has_randr_extension)
@@ -961,14 +962,20 @@ void QXcbConnection::handleXcbError(xcb_generic_error_t *error)
     if (dispatcher && dispatcher->filterNativeEvent(m_nativeInterface->genericEventFilterType(), error, &result))
         return;
 
+    printXcbError("QXcbConnection: XCB error", error);
+}
+
+void QXcbConnection::printXcbError(const char *message, xcb_generic_error_t *error)
+{
     uint clamped_error_code = qMin<uint>(error->error_code, (sizeof(xcb_errors) / sizeof(xcb_errors[0])) - 1);
     uint clamped_major_code = qMin<uint>(error->major_code, (sizeof(xcb_protocol_request_codes) / sizeof(xcb_protocol_request_codes[0])) - 1);
 
-    qWarning("QXcbConnection: XCB error: %d (%s), sequence: %d, resource id: %d, major code: %d (%s), minor code: %d",
-           int(error->error_code), xcb_errors[clamped_error_code],
-           int(error->sequence), int(error->resource_id),
-           int(error->major_code), xcb_protocol_request_codes[clamped_major_code],
-           int(error->minor_code));
+    qWarning("%s: %d (%s), sequence: %d, resource id: %d, major code: %d (%s), minor code: %d",
+             message,
+             int(error->error_code), xcb_errors[clamped_error_code],
+             int(error->sequence), int(error->resource_id),
+             int(error->major_code), xcb_protocol_request_codes[clamped_major_code],
+             int(error->minor_code));
 }
 
 static Qt::MouseButtons translateMouseButtons(int s)
@@ -2087,6 +2094,17 @@ void QXcbConnection::sync()
     free(xcb_get_input_focus_reply(xcb_connection(), cookie, 0));
 }
 
+void QXcbConnection::initializeShm()
+{
+    const xcb_query_extension_reply_t *reply = xcb_get_extension_data(m_connection, &xcb_shm_id);
+    if (!reply || !reply->present) {
+        qWarning("QXcbConnection: MIT-SHM extension is not present on the X server.");
+        return;
+    }
+
+    has_shm = true;
+}
+
 void QXcbConnection::initializeXFixes()
 {
     const xcb_query_extension_reply_t *reply = xcb_get_extension_data(m_connection, &xcb_xfixes_id);
diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
index 07df963ec5..b2713b8ac7 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.h
+++ b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -441,6 +441,7 @@ public:
     void sync();
 
     void handleXcbError(xcb_generic_error_t *error);
+    void printXcbError(const char *message, xcb_generic_error_t *error);
     void handleXcbEvent(xcb_generic_event_t *event);
     void printXcbEvent(const QLoggingCategory &log, const char *message,
                        xcb_generic_event_t *event) const;
@@ -478,6 +479,7 @@ public:
     bool hasXKB() const { return has_xkb; }
     bool hasXRender() const { return has_render_extension; }
     bool hasXInput2() const { return m_xi2Enabled; }
+    bool hasShm() const { return has_shm; }
 
     bool threadedEventHandling() const { return m_reader->isRunning(); }
 
@@ -545,6 +547,7 @@ private slots:
 private:
     void initializeAllAtoms();
     void sendConnectionEvent(QXcbAtom::Atom atom, uint id = 0);
+    void initializeShm();
     void initializeXFixes();
     void initializeXRender();
     void initializeXRandr();
@@ -699,6 +702,7 @@ private:
     bool has_input_shape;
     bool has_xkb = false;
     bool has_render_extension = false;
+    bool has_shm = false;
 
     Qt::MouseButtons m_buttonState = 0;
     Qt::MouseButton m_button = Qt::NoButton;