[cairo] [PATCH 1/2] xcb: Reorganize SHM allocation

[Alexander Volkov]

Introduce _cairo_xcb_connection_shm_{create,destroy}_segment
functions that create/destroy SHM segments and attach/detach
them to the X server's resources. Remove needless
_cairo_xcb_connection_shm_{attach,detach}.

This is a preparation for the use of FD-passing SHM functions
that create/destroy and register/unregister a segment of SHM
in one action.

Signed-off-by: Alexander Volkov <***@rusbitech.ru>
---
src/cairo-xcb-connection-shm.c | 58 ++++++++++++++++++++++++----------
src/cairo-xcb-connection.c | 52 ++++++------------------------
src/cairo-xcb-private.h | 24 +++++++++-----
src/cairo-xcb-shm.c | 52 ++++++++++--------------------
4 files changed, 84 insertions(+), 102 deletions(-)

diff --git a/src/cairo-xcb-connection-shm.c b/src/cairo-xcb-connection-shm.c
index 7720bbbd2..e00377cff 100644
--- a/src/cairo-xcb-connection-shm.c
+++ b/src/cairo-xcb-connection-shm.c
@@ -38,15 +38,48 @@
#include <xcb/xcbext.h>
#include <xcb/shm.h>

-uint32_t
-_cairo_xcb_connection_shm_attach (cairo_xcb_connection_t *connection,
- uint32_t id,
- cairo_bool_t readonly)
+#include <sys/shm.h>
+#include <errno.h>
+
+cairo_int_status_t
+_cairo_xcb_connection_shm_create_segment (cairo_xcb_connection_t *connection,
+ size_t size,
+ cairo_bool_t readonly,
+ cairo_xcb_shm_segment_info_t *shm)
{
- uint32_t segment = _cairo_xcb_connection_get_xid (connection);
- assert (connection->flags & CAIRO_XCB_HAS_SHM);
- xcb_shm_attach (connection->xcb_connection, segment, id, readonly);
- return segment;
+ xcb_void_cookie_t cookie;
+ xcb_generic_error_t *error;
+
+ shm->shmid = shmget (IPC_PRIVATE, size, IPC_CREAT | 0600);
+ if (shm->shmid == -1) {
+ return errno == ENOMEM ? CAIRO_STATUS_NO_MEMORY : CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ shm->shm = shmat (shm->shmid, NULL, 0);
+ shmctl (shm->shmid, IPC_RMID, NULL);
+ if (unlikely (shm->shm == (char *) -1))
+ return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+ shm->shmseg = _cairo_xcb_connection_get_xid (connection);
+ cookie = xcb_shm_attach_checked (connection->xcb_connection, shm->shmseg, shm->shmid, readonly);
+ error = xcb_request_check (connection->xcb_connection, cookie);
+ if (error != NULL) {
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ shmdt (shm->shm);
+ free (error);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ return CAIRO_INT_STATUS_SUCCESS;
+}
+
+void
+_cairo_xcb_connection_shm_destroy_segment (cairo_xcb_connection_t *connection,
+ cairo_xcb_shm_segment_info_t *shm)
+{
+ xcb_shm_detach (connection->xcb_connection, shm->shmseg);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ shmdt (shm->shm);
}

void
@@ -103,13 +136,4 @@ _cairo_xcb_connection_shm_get_image (cairo_xcb_connection_t *connection,
return CAIRO_STATUS_SUCCESS;
}

-void
-_cairo_xcb_connection_shm_detach (cairo_xcb_connection_t *connection,
- uint32_t segment)
-{
- assert (connection->flags & CAIRO_XCB_HAS_SHM);
- xcb_shm_detach (connection->xcb_connection, segment);
- _cairo_xcb_connection_put_xid (connection, segment);
-}
-
#endif /* CAIRO_HAS_XCB_SHM_FUNCTIONS */
diff --git a/src/cairo-xcb-connection.c b/src/cairo-xcb-connection.c
index 67897fa4e..67e2b11a0 100644
--- a/src/cairo-xcb-connection.c
+++ b/src/cairo-xcb-connection.c
@@ -441,51 +441,13 @@ _cairo_xcb_connection_query_cairo (cairo_xcb_connection_t *connection)
#endif

#if CAIRO_HAS_XCB_SHM_FUNCTIONS
-static cairo_bool_t
-can_use_shm (cairo_xcb_connection_t *connection)
-{
- cairo_bool_t success = TRUE;
- xcb_connection_t *c = connection->xcb_connection;
- xcb_void_cookie_t cookie[2];
- xcb_generic_error_t *error;
- int shmid;
- uint32_t shmseg;
- void *ptr;
-
- shmid = shmget (IPC_PRIVATE, 0x1000, IPC_CREAT | 0600);
- if (shmid == -1)
- return FALSE;
-
- ptr = shmat (shmid, NULL, 0);
- if (ptr == (char *) -1) {
- shmctl (shmid, IPC_RMID, NULL);
- return FALSE;
- }
-
- shmseg = _cairo_xcb_connection_get_xid (connection);
- cookie[0] = xcb_shm_attach_checked (c, shmseg, shmid, FALSE);
- cookie[1] = xcb_shm_detach_checked (c, shmseg);
- _cairo_xcb_connection_put_xid (connection, shmseg);
-
- error = xcb_request_check (c, cookie[0]);
- if (error != NULL)
- success = FALSE;
-
- error = xcb_request_check (c, cookie[1]);
- if (error != NULL)
- success = FALSE;
-
- shmctl (shmid, IPC_RMID, NULL);
- shmdt (ptr);
-
- return success;
-}
-
static void
_cairo_xcb_connection_query_shm (cairo_xcb_connection_t *connection)
{
xcb_connection_t *c = connection->xcb_connection;
xcb_shm_query_version_reply_t *version;
+ cairo_int_status_t status;
+ cairo_xcb_shm_segment_info_t seg_info;

version = xcb_shm_query_version_reply (c, xcb_shm_query_version (c), 0);
if (version == NULL)
@@ -493,8 +455,14 @@ _cairo_xcb_connection_query_shm (cairo_xcb_connection_t *connection)

free (version);

- if (can_use_shm (connection))
- connection->flags |= CAIRO_XCB_HAS_SHM;
+ connection->flags |= CAIRO_XCB_HAS_SHM;
+ status = _cairo_xcb_connection_shm_create_segment (connection, 0x1000, FALSE, &seg_info);
+ if (status != CAIRO_INT_STATUS_SUCCESS) {
+ connection->flags &= ~CAIRO_XCB_HAS_SHM;
+ return;
+ }
+
+ _cairo_xcb_connection_shm_destroy_segment(connection, &seg_info);
}
#endif

diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
index f5d5a4c81..4e417ddff 100644
--- a/src/cairo-xcb-private.h
+++ b/src/cairo-xcb-private.h
@@ -73,10 +73,17 @@ typedef struct _cairo_xcb_font cairo_xcb_font_t;
typedef struct _cairo_xcb_screen cairo_xcb_screen_t;
typedef struct _cairo_xcb_surface cairo_xcb_surface_t;
typedef struct _cairo_xcb_picture cairo_xcb_picture_t;
+typedef struct _cairo_xcb_shm_segment_info cairo_xcb_shm_segment_info_t;
typedef struct _cairo_xcb_shm_mem_pool cairo_xcb_shm_mem_pool_t;
typedef struct _cairo_xcb_shm_info cairo_xcb_shm_info_t;
typedef struct _cairo_xcb_resources cairo_xcb_resources_t;

+struct _cairo_xcb_shm_segment_info {
+ int shmid;
+ uint32_t shmseg;
+ void *shm;
+};
+
struct _cairo_xcb_shm_info {
cairo_xcb_connection_t *connection;
uint32_t shm;
@@ -564,10 +571,15 @@ _cairo_xcb_shm_image_create (cairo_xcb_connection_t *connection,
cairo_xcb_shm_info_t **shm_info_out);

#if CAIRO_HAS_XCB_SHM_FUNCTIONS
-cairo_private uint32_t
-_cairo_xcb_connection_shm_attach (cairo_xcb_connection_t *connection,
- uint32_t id,
- cairo_bool_t readonly);
+cairo_private cairo_int_status_t
+_cairo_xcb_connection_shm_create_segment (cairo_xcb_connection_t *connection,
+ size_t size,
+ cairo_bool_t readonly,
+ cairo_xcb_shm_segment_info_t *shm);
+
+cairo_private void
+_cairo_xcb_connection_shm_destroy_segment (cairo_xcb_connection_t *connection,
+ cairo_xcb_shm_segment_info_t *shm);

cairo_private void
_cairo_xcb_connection_shm_put_image (cairo_xcb_connection_t *connection,
@@ -594,10 +606,6 @@ _cairo_xcb_connection_shm_get_image (cairo_xcb_connection_t *connection,
uint16_t height,
uint32_t shmseg,
uint32_t offset);
-
-cairo_private void
-_cairo_xcb_connection_shm_detach (cairo_xcb_connection_t *connection,
- uint32_t segment);
#else
static inline void
_cairo_xcb_connection_shm_put_image (cairo_xcb_connection_t *connection,
diff --git a/src/cairo-xcb-shm.c b/src/cairo-xcb-shm.c
index 2be2dac5b..074d311ca 100644
--- a/src/cairo-xcb-shm.c
+++ b/src/cairo-xcb-shm.c
@@ -61,12 +61,8 @@ typedef enum {
} shm_wait_type_t;

struct _cairo_xcb_shm_mem_pool {
- int shmid;
- uint32_t shmseg;
- void *shm;
-
+ cairo_xcb_shm_segment_info_t shm;
cairo_mempool_t mem;
-
cairo_list_t link;
};

@@ -75,7 +71,6 @@ _cairo_xcb_shm_mem_pool_destroy (cairo_xcb_shm_mem_pool_t *pool)
{
cairo_list_del (&pool->link);

- shmdt (pool->shm);
_cairo_mempool_fini (&pool->mem);

free (pool);
@@ -104,7 +99,7 @@ _cairo_xcb_shm_info_finalize (cairo_xcb_shm_info_t *shm_info)
&connection->shm_pools, link)
{
if (pool->mem.free_bytes == pool->mem.max_bytes) {
- _cairo_xcb_connection_shm_detach (connection, pool->shmseg);
+ _cairo_xcb_connection_shm_destroy_segment (connection, &pool->shm);
_cairo_xcb_shm_mem_pool_destroy (pool);
}
}
@@ -196,8 +191,7 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
}
/* scan for old, unused pools */
if (pool->mem.free_bytes == pool->mem.max_bytes) {
- _cairo_xcb_connection_shm_detach (connection,
- pool->shmseg);
+ _cairo_xcb_connection_shm_destroy_segment (connection, &pool->shm);
_cairo_xcb_shm_mem_pool_destroy (pool);
} else {
shm_allocated += pool->mem.max_bytes;
@@ -221,45 +215,32 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
bytes <<= 3;

do {
- pool->shmid = shmget (IPC_PRIVATE, bytes, IPC_CREAT | 0600);
- if (pool->shmid != -1)
+ status = _cairo_xcb_connection_shm_create_segment (connection, bytes, FALSE, &pool->shm);
+ if (status == CAIRO_INT_STATUS_SUCCESS)
break;

/* If the allocation failed because we asked for too much memory, we try
* again with a smaller request, as long as our allocation still fits. */
bytes >>= 1;
- if (errno != EINVAL || bytes < size)
+ if (status == CAIRO_INT_STATUS_UNSUPPORTED || bytes < size)
break;
} while (TRUE);
- if (pool->shmid == -1) {
- int err = errno;
- if (! (err == EINVAL || err == ENOMEM))
- connection->flags &= ~CAIRO_XCB_HAS_SHM;
- free (pool);
- CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
- return CAIRO_INT_STATUS_UNSUPPORTED;
- }

- pool->shm = shmat (pool->shmid, NULL, 0);
- if (unlikely (pool->shm == (char *) -1)) {
- shmctl (pool->shmid, IPC_RMID, NULL);
+ if (unlikely (status)) {
free (pool);
CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
- return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
}

- status = _cairo_mempool_init (&pool->mem, pool->shm, bytes,
+ status = _cairo_mempool_init (&pool->mem, pool->shm.shm, bytes,
minbits, maxbits - minbits + 1);
if (unlikely (status)) {
- shmdt (pool->shm);
+ _cairo_xcb_connection_shm_destroy_segment (connection, &pool->shm);
free (pool);
CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
return status;
}

- pool->shmseg = _cairo_xcb_connection_shm_attach (connection, pool->shmid, FALSE);
- shmctl (pool->shmid, IPC_RMID, NULL);
-
cairo_list_add (&pool->link, &connection->shm_pools);
mem = _cairo_mempool_alloc (&pool->mem, size);

@@ -273,9 +254,9 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,

shm_info->connection = connection;
shm_info->pool = pool;
- shm_info->shm = pool->shmseg;
+ shm_info->shm = pool->shm.shmseg;
shm_info->size = size;
- shm_info->offset = (char *) mem - (char *) pool->shm;
+ shm_info->offset = (char *) mem - (char *) pool->shm.shm;
shm_info->mem = mem;
shm_info->sync.sequence = XCB_NONE;

@@ -284,8 +265,7 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
&connection->shm_pools, link)
{
if (pool->mem.free_bytes == pool->mem.max_bytes) {
- _cairo_xcb_connection_shm_detach (connection,
- pool->shmseg);
+ _cairo_xcb_connection_shm_destroy_segment (connection, &pool->shm);
_cairo_xcb_shm_mem_pool_destroy (pool);
}
}
@@ -328,9 +308,11 @@ _cairo_xcb_connection_shm_mem_pools_fini (cairo_xcb_connection_t *connection)
{
assert (cairo_list_is_empty (&connection->shm_pending));
while (! cairo_list_is_empty (&connection->shm_pools)) {
- _cairo_xcb_shm_mem_pool_destroy (cairo_list_first_entry (&connection->shm_pools,
+ cairo_xcb_shm_mem_pool_t *pool = cairo_list_first_entry (&connection->shm_pools,
cairo_xcb_shm_mem_pool_t,
- link));
+ link);
+ _cairo_xcb_connection_shm_destroy_segment (connection, &pool->shm);
+ _cairo_xcb_shm_mem_pool_destroy (pool);
}
}

[Alexander Volkov]

Shared memory created this way is visible only to the X server
and an application.

- it can used if the X server is running as a non-root user other
than a user an application is running as
- other applications of the same user can't access it

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=90512
Signed-off-by: Alexander Volkov <***@rusbitech.ru>
---
boilerplate/Makefile.win32.features | 12 ++++
build/Makefile.win32.features | 1 +
build/Makefile.win32.features-h | 3 +
build/configure.ac.features | 1 +
configure.ac | 11 ++++
src/Makefile.win32.features | 16 +++++
src/cairo-xcb-connection-shm.c | 98 ++++++++++++++++++++++-------
src/cairo-xcb-connection.c | 7 ++-
src/cairo-xcb-private.h | 4 +-
9 files changed, 130 insertions(+), 23 deletions(-)

diff --git a/boilerplate/Makefile.win32.features b/boilerplate/Makefile.win32.features
index 178d5b650..84df408d1 100644
--- a/boilerplate/Makefile.win32.features
+++ b/boilerplate/Makefile.win32.features
@@ -79,6 +79,18 @@ enabled_cairo_boilerplate_cxx_sources += $(cairo_boilerplate_xcb_shm_cxx_sources
enabled_cairo_boilerplate_sources += $(cairo_boilerplate_xcb_shm_sources)
endif

+supported_cairo_boilerplate_headers += $(cairo_boilerplate_xcb_shm_fd_headers)
+all_cairo_boilerplate_headers += $(cairo_boilerplate_xcb_shm_fd_headers)
+all_cairo_boilerplate_private += $(cairo_boilerplate_xcb_shm_fd_private)
+all_cairo_boilerplate_cxx_sources += $(cairo_boilerplate_xcb_shm_fd_cxx_sources)
+all_cairo_boilerplate_sources += $(cairo_boilerplate_xcb_shm_fd_sources)
+ifeq ($(CAIRO_HAS_XCB_SHM_FD_FUNCTIONS),1)
+enabled_cairo_boilerplate_headers += $(cairo_boilerplate_xcb_shm_fd_headers)
+enabled_cairo_boilerplate_private += $(cairo_boilerplate_xcb_shm_fd_private)
+enabled_cairo_boilerplate_cxx_sources += $(cairo_boilerplate_xcb_shm_fd_cxx_sources)
+enabled_cairo_boilerplate_sources += $(cairo_boilerplate_xcb_shm_fd_sources)
+endif
+
unsupported_cairo_boilerplate_headers += $(cairo_boilerplate_qt_headers)
all_cairo_boilerplate_headers += $(cairo_boilerplate_qt_headers)
all_cairo_boilerplate_private += $(cairo_boilerplate_qt_private)
diff --git a/build/Makefile.win32.features b/build/Makefile.win32.features
index b15c4488f..f8a1e441d 100644
--- a/build/Makefile.win32.features
+++ b/build/Makefile.win32.features
@@ -5,6 +5,7 @@ CAIRO_HAS_XLIB_XRENDER_SURFACE=0
CAIRO_HAS_XCB_SURFACE=0
CAIRO_HAS_XLIB_XCB_FUNCTIONS=0
CAIRO_HAS_XCB_SHM_FUNCTIONS=0
+CAIRO_HAS_XCB_SHM_FD_FUNCTIONS=0
CAIRO_HAS_QT_SURFACE=0
CAIRO_HAS_QUARTZ_SURFACE=0
CAIRO_HAS_QUARTZ_FONT=0
diff --git a/build/Makefile.win32.features-h b/build/Makefile.win32.features-h
index 5759b48a3..53984bb4c 100644
--- a/build/Makefile.win32.features-h
+++ b/build/Makefile.win32.features-h
@@ -20,6 +20,9 @@ endif
ifeq ($(CAIRO_HAS_XCB_SHM_FUNCTIONS),1)
@echo "#define CAIRO_HAS_XCB_SHM_FUNCTIONS 1" >> $(top_srcdir)/src/cairo-features.h
endif
+ifeq ($(CAIRO_HAS_XCB_SHM_FD_FUNCTIONS),1)
+ @echo "#define CAIRO_HAS_XCB_SHM_FD_FUNCTIONS 1" >> $(top_srcdir)/src/cairo-features.h
+endif
ifeq ($(CAIRO_HAS_QT_SURFACE),1)
@echo "#define CAIRO_HAS_QT_SURFACE 1" >> $(top_srcdir)/src/cairo-features.h
endif
diff --git a/build/configure.ac.features b/build/configure.ac.features
index e0a46069c..6c8a8f071 100644
--- a/build/configure.ac.features
+++ b/build/configure.ac.features
@@ -405,6 +405,7 @@ AC_DEFUN([CAIRO_REPORT],
echo " EGL functions: $use_egl"
echo " X11-xcb functions: $use_xlib_xcb"
echo " XCB-shm functions: $use_xcb_shm"
+ echo " XCB-shm FD-passing functions: $use_xcb_shm_fd"
echo ""
echo "The following features and utilities:"
echo " cairo-trace: $use_trace"
diff --git a/configure.ac b/configure.ac
index d78b2ed41..8c7fa4267 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,11 +182,22 @@ CAIRO_ENABLE_FUNCTIONS(xcb_shm, XCB/SHM, auto, [
xcb_shm_REQUIRES="xcb-shm"
PKG_CHECK_MODULES(xcb_shm, $xcb_shm_REQUIRES, ,
[use_xcb_shm="no (requires $xcb_shm http://xcb.freedesktop.org)"])
+
else
use_xcb_shm="no (requires --enable-xcb)"
fi
])

+CAIRO_ENABLE_FUNCTIONS(xcb_shm_fd, XCB/SHM FD-passing, auto, [
+ if test "x$use_xcb_shm" = "xyes"; then
+ xcb_shm_fd_REQUIRES="xcb-shm >= 1.9.3"
+ PKG_CHECK_MODULES(xcb_shm_fd, $xcb_shm_fd_REQUIRES, ,
+ [use_xcb_shm_fd="no (requires $xcb_shm_fd_REQUIRES http://xcb.freedesktop.org)"])
+ else
+ use_xcb_shm_fd="no (requires --enable-xcb-shm)"
+ fi
+])
+
dnl ===========================================================================

CAIRO_ENABLE_SURFACE_BACKEND(qt, Qt, no, [
diff --git a/src/Makefile.win32.features b/src/Makefile.win32.features
index 377d6dd12..163e2507c 100644
--- a/src/Makefile.win32.features
+++ b/src/Makefile.win32.features
@@ -101,6 +101,22 @@ ifeq ($(CAIRO_HAS_XCB_SHM_FUNCTIONS),1)
enabled_cairo_pkgconf += cairo-xcb-shm.pc
endif

+supported_cairo_headers += $(cairo_xcb_shm_fd_headers)
+all_cairo_headers += $(cairo_xcb_shm_fd_headers)
+all_cairo_private += $(cairo_xcb_shm_fd_private)
+all_cairo_cxx_sources += $(cairo_xcb_shm_fd_cxx_sources)
+all_cairo_sources += $(cairo_xcb_shm_fd_sources)
+ifeq ($(CAIRO_HAS_XCB_SHM_FD_FUNCTIONS),1)
+enabled_cairo_headers += $(cairo_xcb_shm_fd_headers)
+enabled_cairo_private += $(cairo_xcb_shm_fd_private)
+enabled_cairo_cxx_sources += $(cairo_xcb_shm_fd_cxx_sources)
+enabled_cairo_sources += $(cairo_xcb_shm_fd_sources)
+endif
+all_cairo_pkgconf += cairo-xcb-shm-fd.pc
+ifeq ($(CAIRO_HAS_XCB_SHM_FD_FUNCTIONS),1)
+enabled_cairo_pkgconf += cairo-xcb-shm-fd.pc
+endif
+
unsupported_cairo_headers += $(cairo_qt_headers)
all_cairo_headers += $(cairo_qt_headers)
all_cairo_private += $(cairo_qt_private)
diff --git a/src/cairo-xcb-connection-shm.c b/src/cairo-xcb-connection-shm.c
index e00377cff..bb73ca9b1 100644
--- a/src/cairo-xcb-connection-shm.c
+++ b/src/cairo-xcb-connection-shm.c
@@ -41,35 +41,86 @@
#include <sys/shm.h>
#include <errno.h>

+#if CAIRO_HAS_XCB_SHM_FD_FUNCTIONS
+#include <sys/mman.h>
+#include <unistd.h>
+#endif
+
cairo_int_status_t
_cairo_xcb_connection_shm_create_segment (cairo_xcb_connection_t *connection,
size_t size,
cairo_bool_t readonly,
cairo_xcb_shm_segment_info_t *shm)
{
- xcb_void_cookie_t cookie;
- xcb_generic_error_t *error;
-
- shm->shmid = shmget (IPC_PRIVATE, size, IPC_CREAT | 0600);
- if (shm->shmid == -1) {
- return errno == ENOMEM ? CAIRO_STATUS_NO_MEMORY : CAIRO_INT_STATUS_UNSUPPORTED;
- }
-
- shm->shm = shmat (shm->shmid, NULL, 0);
- shmctl (shm->shmid, IPC_RMID, NULL);
- if (unlikely (shm->shm == (char *) -1))
- return _cairo_error (CAIRO_STATUS_NO_MEMORY);
-
- shm->shmseg = _cairo_xcb_connection_get_xid (connection);
- cookie = xcb_shm_attach_checked (connection->xcb_connection, shm->shmseg, shm->shmid, readonly);
- error = xcb_request_check (connection->xcb_connection, cookie);
- if (error != NULL) {
- _cairo_xcb_connection_put_xid (connection, shm->shmseg);
- shmdt (shm->shm);
- free (error);
- return CAIRO_INT_STATUS_UNSUPPORTED;
+#if CAIRO_HAS_XCB_SHM_FD_FUNCTIONS
+ if (connection->flags & CAIRO_XCB_HAS_SHM_FD) {
+ xcb_shm_create_segment_cookie_t cookie;
+ xcb_shm_create_segment_reply_t *reply;
+ xcb_generic_error_t *error;
+ int *fds;
+ int err;
+
+ if (unlikely (size > UINT32_MAX))
+ return CAIRO_STATUS_NO_MEMORY;
+
+ shm->shmseg = _cairo_xcb_connection_get_xid (connection);
+ cookie = xcb_shm_create_segment (connection->xcb_connection, shm->shmseg, size, FALSE);
+ reply = xcb_shm_create_segment_reply (connection->xcb_connection, cookie, &error);
+ if (error) {
+ free (error);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ fds = xcb_shm_create_segment_reply_fds (connection->xcb_connection, reply);
+ if (reply->nfd != 1) {
+ for (int i = 0; i < reply->nfd; i++)
+ close (fds[i]);
+
+ free (reply);
+ xcb_shm_detach (connection->xcb_connection, shm->shmseg);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ shm->shm = mmap (NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fds[0], 0);
+ err = errno;
+ close (fds[0]);
+ free (reply);
+
+ if (shm->shm == MAP_FAILED) {
+ xcb_shm_detach (connection->xcb_connection, shm->shmseg);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return err == ENOMEM ? CAIRO_STATUS_NO_MEMORY : CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+ } else
+#endif
+ {
+ xcb_void_cookie_t cookie;
+ xcb_generic_error_t *error;
+
+ shm->shmid = shmget (IPC_PRIVATE, size, IPC_CREAT | 0600);
+ if (shm->shmid == -1) {
+ return errno == ENOMEM ? CAIRO_STATUS_NO_MEMORY : CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ shm->shm = shmat (shm->shmid, NULL, 0);
+ shmctl (shm->shmid, IPC_RMID, NULL);
+ if (unlikely (shm->shm == (char *) -1))
+ return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+ shm->shmseg = _cairo_xcb_connection_get_xid (connection);
+ cookie = xcb_shm_attach_checked (connection->xcb_connection, shm->shmseg, shm->shmid, readonly);
+ error = xcb_request_check (connection->xcb_connection, cookie);
+ if (error != NULL) {
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ shmdt (shm->shm);
+ free (error);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
}

+ shm->size = size;
return CAIRO_INT_STATUS_SUCCESS;
}

@@ -79,6 +130,11 @@ _cairo_xcb_connection_shm_destroy_segment (cairo_xcb_connection_t *connection,
{
xcb_shm_detach (connection->xcb_connection, shm->shmseg);
_cairo_xcb_connection_put_xid (connection, shm->shmseg);
+#if CAIRO_HAS_XCB_SHM_FD_FUNCTIONS
+ if (connection->flags & CAIRO_XCB_HAS_SHM_FD)
+ munmap (shm->shm, shm->size);
+ else
+#endif
shmdt (shm->shm);
}

diff --git a/src/cairo-xcb-connection.c b/src/cairo-xcb-connection.c
index 67e2b11a0..2f7645cbe 100644
--- a/src/cairo-xcb-connection.c
+++ b/src/cairo-xcb-connection.c
@@ -453,12 +453,17 @@ _cairo_xcb_connection_query_shm (cairo_xcb_connection_t *connection)
if (version == NULL)
return;

+ connection->flags |= CAIRO_XCB_HAS_SHM;
+ if ((version->major_version == 1 && version->minor_version >= 2) ||
+ version->major_version > 1)
+ connection->flags |= CAIRO_XCB_HAS_SHM_FD;
+
free (version);

- connection->flags |= CAIRO_XCB_HAS_SHM;
status = _cairo_xcb_connection_shm_create_segment (connection, 0x1000, FALSE, &seg_info);
if (status != CAIRO_INT_STATUS_SUCCESS) {
connection->flags &= ~CAIRO_XCB_HAS_SHM;
+ connection->flags &= ~CAIRO_XCB_HAS_SHM_FD;
return;
}

diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
index 4e417ddff..8a3b9715a 100644
--- a/src/cairo-xcb-private.h
+++ b/src/cairo-xcb-private.h
@@ -82,6 +82,7 @@ struct _cairo_xcb_shm_segment_info {
int shmid;
uint32_t shmseg;
void *shm;
+ size_t size;
};

struct _cairo_xcb_shm_info {
@@ -271,7 +272,8 @@ enum {
CAIRO_XCB_RENDER_HAS_FILTER_GOOD = 0x0400,
CAIRO_XCB_RENDER_HAS_FILTER_BEST = 0x0800,

- CAIRO_XCB_HAS_SHM = 0x80000000,
+ CAIRO_XCB_HAS_SHM = 0x40000000,
+ CAIRO_XCB_HAS_SHM_FD = 0x80000000,

CAIRO_XCB_RENDER_MASK = CAIRO_XCB_HAS_RENDER |
CAIRO_XCB_RENDER_HAS_FILL_RECTANGLES |

[Chris Wilson]

Quoting Alexander Volkov (2018-04-06 13:33:37)

Post by Alexander Volkov
+ if (connection->flags & CAIRO_XCB_HAS_SHM_FD) {
+ xcb_shm_create_segment_cookie_t cookie;
+ xcb_shm_create_segment_reply_t *reply;
+ xcb_generic_error_t *error;
+ int *fds;
+ int err;
+
+ if (unlikely (size > UINT32_MAX))
+ return CAIRO_STATUS_NO_MEMORY;

That's not the protocol limit, surely? It may well be, just a bit
surprising if it was.

Post by Alexander Volkov
+ shm->shmseg = _cairo_xcb_connection_get_xid (connection);
+ cookie = xcb_shm_create_segment (connection->xcb_connection, shm->shmseg, size, FALSE);
+ reply = xcb_shm_create_segment_reply (connection->xcb_connection, cookie, &error);
+ if (error) {
+ free (error);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ fds = xcb_shm_create_segment_reply_fds (connection->xcb_connection, reply);
+ if (reply->nfd != 1) {
+ for (int i = 0; i < reply->nfd; i++)
+ close (fds[i]);
+
+ free (reply);
+ xcb_shm_detach (connection->xcb_connection, shm->shmseg);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ shm->shm = mmap (NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fds[0], 0);

A roundtrip on allocation. That doesn't seem like something we would want
in a frequent path? And creating temporary surfaces is frequent enough.
That would suggest to me that we want an allocation cache... And packing
into a larger object?
-Chris

[Alexander Volkov]

Post by Chris Wilson
Quoting Alexander Volkov (2018-04-06 13:33:37)

Post by Alexander Volkov
+ if (connection->flags & CAIRO_XCB_HAS_SHM_FD) {
+ xcb_shm_create_segment_cookie_t cookie;
+ xcb_shm_create_segment_reply_t *reply;
+ xcb_generic_error_t *error;
+ int *fds;
+ int err;
+
+ if (unlikely (size > UINT32_MAX))
+ return CAIRO_STATUS_NO_MEMORY;

That's not the protocol limit, surely? It may well be, just a bit
surprising if it was.

Unfortunately it is. The size argument of xcb_shm_create_segment() is of
type uint32_t.

Post by Chris Wilson

Post by Alexander Volkov
+ shm->shmseg = _cairo_xcb_connection_get_xid (connection);
+ cookie = xcb_shm_create_segment (connection->xcb_connection, shm->shmseg, size, FALSE);
+ reply = xcb_shm_create_segment_reply (connection->xcb_connection, cookie, &error);
+ if (error) {
+ free (error);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ fds = xcb_shm_create_segment_reply_fds (connection->xcb_connection, reply);
+ if (reply->nfd != 1) {
+ for (int i = 0; i < reply->nfd; i++)
+ close (fds[i]);
+
+ free (reply);
+ xcb_shm_detach (connection->xcb_connection, shm->shmseg);
+ _cairo_xcb_connection_put_xid (connection, shm->shmseg);
+ return CAIRO_INT_STATUS_UNSUPPORTED;
+ }
+
+ shm->shm = mmap (NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fds[0], 0);

A roundtrip on allocation. That doesn't seem like something we would want
in a frequent path? And creating temporary surfaces is frequent enough.
That would suggest to me that we want an allocation cache... And packing
into a larger object?

Well, it seems that it would be better to use xcb_shm_attach_fd()
instead of xcb_shm_create_segment().
It will also allow to avoid UINT32_MAX restriction.
Maybe I'll try it next week.

[Alexander Volkov]

Post by Alexander Volkov
Well, it seems that it would be better to use xcb_shm_attach_fd()
instead of xcb_shm_create_segment().
It will also allow to avoid UINT32_MAX restriction.
Maybe I'll try it next week.

First we need a function that could create memory files correctly:
https://lists.freedesktop.org/archives/xcb/2018-April/011095.html