[PATCH 1/2] xcb: Reorganize SHM allocation

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 <[email protected]>
---
 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);
     }
 }
 
-- 
2.17.0

-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo