[PATCH weston 1/3] Rewrite shm buffer file allocation v2

Pekka Paalanen ppaalanen at gmail.com
Wed Jun 6 06:59:43 PDT 2012


We had duplicated code in many places, using hardcoded paths for
temporary files into more than one path. Some cases did not bother with
O_CLOEXEC, and all hardcoded paths that might not exist.

Add an OS helper function for creating a unique anonymous file with
close-on-exec semantics. The helper uses $XDG_RUNTIME_DIR as the
directory for a file.

This patch unifies the buffer file creation in both Weston and the
clients.

As simple clients are better not linking to libshared, as it would
require e.g. Cairo, they pull the OS compatibility code directly.

Android does not have mkostemp(), so a configure test is added for it,
and a fallback used if it is not available.

Changes in v2:
	remove all the alternate possible directory definitions and use
	XDG_RUNTIME_DIR only, and fail is it is not set.

Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
---
 clients/Makefile.am       |    8 +++-
 clients/screenshot.c      |   17 +++------
 clients/simple-shm.c      |   17 +++------
 clients/simple-touch.c    |   17 +++------
 clients/window.c          |   13 ++------
 configure.ac              |    2 +
 shared/os-compatibility.c |   75 +++++++++++++++++++++++++++++++++++++++++++++
 shared/os-compatibility.h |    7 ++++
 src/compositor.c          |   31 ++++--------------
 9 files changed, 118 insertions(+), 69 deletions(-)

diff --git a/clients/Makefile.am b/clients/Makefile.am
index 573cf59..cf25a0b 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -27,11 +27,15 @@ simple_egl_SOURCES = simple-egl.c
 simple_egl_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS)
 simple_egl_LDADD = $(SIMPLE_CLIENT_LIBS) -lm
 
-simple_shm_SOURCES = simple-shm.c
+simple_shm_SOURCES = simple-shm.c 		\
+	../shared/os-compatibility.c		\
+	../shared/os-compatibility.h
 simple_shm_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS)
 simple_shm_LDADD = $(SIMPLE_CLIENT_LIBS)
 
-simple_touch_SOURCES = simple-touch.c
+simple_touch_SOURCES = simple-touch.c		\
+	../shared/os-compatibility.c		\
+	../shared/os-compatibility.h
 simple_touch_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS)
 simple_touch_LDADD = $(SIMPLE_CLIENT_LIBS)
 endif
diff --git a/clients/screenshot.c b/clients/screenshot.c
index e023199..894c4aa 100644
--- a/clients/screenshot.c
+++ b/clients/screenshot.c
@@ -33,6 +33,7 @@
 
 #include <wayland-client.h>
 #include "screenshooter-client-protocol.h"
+#include "../shared/os-compatibility.h"
 
 /* The screenshooter is a good example of a custom object exposed by
  * the compositor and serves as a test bed for implementing client
@@ -127,28 +128,22 @@ handle_global(struct wl_display *display, uint32_t id,
 static struct wl_buffer *
 create_shm_buffer(int width, int height, void **data_out)
 {
-	char filename[] = "/tmp/wayland-shm-XXXXXX";
 	struct wl_shm_pool *pool;
 	struct wl_buffer *buffer;
 	int fd, size, stride;
 	void *data;
 
-	fd = mkstemp(filename);
-	if (fd < 0) {
-		fprintf(stderr, "open %s failed: %m\n", filename);
-		return NULL;
-	}
 	stride = width * 4;
 	size = stride * height;
-	if (ftruncate(fd, size) < 0) {
-		fprintf(stderr, "ftruncate failed: %m\n");
-		close(fd);
+
+	fd = os_create_anonymous_file(size);
+	if (fd < 0) {
+		fprintf(stderr, "creating a buffer file for %d B failed: %m\n",
+			size);
 		return NULL;
 	}
 
 	data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-	unlink(filename);
-
 	if (data == MAP_FAILED) {
 		fprintf(stderr, "mmap failed: %m\n");
 		close(fd);
diff --git a/clients/simple-shm.c b/clients/simple-shm.c
index cb79126..252e040 100644
--- a/clients/simple-shm.c
+++ b/clients/simple-shm.c
@@ -31,6 +31,7 @@
 #include <signal.h>
 
 #include <wayland-client.h>
+#include "../shared/os-compatibility.h"
 
 struct display {
 	struct wl_display *display;
@@ -55,28 +56,22 @@ static struct wl_buffer *
 create_shm_buffer(struct display *display,
 		  int width, int height, uint32_t format, void **data_out)
 {
-	char filename[] = "/tmp/wayland-shm-XXXXXX";
 	struct wl_shm_pool *pool;
 	struct wl_buffer *buffer;
 	int fd, size, stride;
 	void *data;
 
-	fd = mkstemp(filename);
-	if (fd < 0) {
-		fprintf(stderr, "open %s failed: %m\n", filename);
-		return NULL;
-	}
 	stride = width * 4;
 	size = stride * height;
-	if (ftruncate(fd, size) < 0) {
-		fprintf(stderr, "ftruncate failed: %m\n");
-		close(fd);
+
+	fd = os_create_anonymous_file(size);
+	if (fd < 0) {
+		fprintf(stderr, "creating a buffer file for %d B failed: %m\n",
+			size);
 		return NULL;
 	}
 
 	data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-	unlink(filename);
-
 	if (data == MAP_FAILED) {
 		fprintf(stderr, "mmap failed: %m\n");
 		close(fd);
diff --git a/clients/simple-touch.c b/clients/simple-touch.c
index a00cc90..517a956 100644
--- a/clients/simple-touch.c
+++ b/clients/simple-touch.c
@@ -32,6 +32,7 @@
 #include <GLES2/gl2.h>
 #include <wayland-client.h>
 #include <wayland-egl.h>
+#include "../shared/os-compatibility.h"
 
 struct touch {
 	struct wl_display *display;
@@ -55,26 +56,20 @@ static void
 create_shm_buffer(struct touch *touch)
 {
 	struct wl_shm_pool *pool;
-	char filename[] = "/tmp/wayland-shm-XXXXXX";
 	int fd, size, stride;
 
-	fd = mkstemp(filename);
-	if (fd < 0) {
-		fprintf(stderr, "open %s failed: %m\n", filename);
-		exit(1);
-	}
 	stride = touch->width * 4;
 	size = stride * touch->height;
-	if (ftruncate(fd, size) < 0) {
-		fprintf(stderr, "ftruncate failed: %m\n");
-		close(fd);
+
+	fd = os_create_anonymous_file(size);
+	if (fd < 0) {
+		fprintf(stderr, "creating a buffer file for %d B failed: %m\n",
+			size);
 		exit(1);
 	}
 
 	touch->data =
 		mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-	unlink(filename);
-
 	if (touch->data == MAP_FAILED) {
 		fprintf(stderr, "mmap failed: %m\n");
 		close(fd);
diff --git a/clients/window.c b/clients/window.c
index 36d8613..53022aa 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -424,24 +424,17 @@ shm_surface_data_destroy(void *p)
 static struct wl_shm_pool *
 make_shm_pool(struct display *display, int size, void **data)
 {
-	char filename[] = "/tmp/wayland-shm-XXXXXX";
 	struct wl_shm_pool *pool;
 	int fd;
 
-	fd = mkstemp(filename);
+	fd = os_create_anonymous_file(size);
 	if (fd < 0) {
-		fprintf(stderr, "open %s failed: %m\n", filename);
-		return NULL;
-	}
-	if (ftruncate(fd, size) < 0) {
-		fprintf(stderr, "ftruncate failed: %m\n");
-		close(fd);
+		fprintf(stderr, "creating a buffer file for %d B failed: %m\n",
+			size);
 		return NULL;
 	}
 
 	*data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-	unlink(filename);
-
 	if (*data == MAP_FAILED) {
 		fprintf(stderr, "mmap failed: %m\n");
 		close(fd);
diff --git a/configure.ac b/configure.ac
index 7d9b2a2..7dbd9f9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,8 @@ AC_SUBST(DLOPEN_LIBS)
 
 AC_CHECK_HEADERS([execinfo.h])
 
+AC_CHECK_FUNCS([mkostemp])
+
 PKG_CHECK_MODULES(COMPOSITOR,
 		  [wayland-server egl >= 7.10 glesv2 xkbcommon pixman-1])
 
diff --git a/shared/os-compatibility.c b/shared/os-compatibility.c
index 3c065e8..66934a8 100644
--- a/shared/os-compatibility.c
+++ b/shared/os-compatibility.c
@@ -20,12 +20,16 @@
  * OF THIS SOFTWARE.
  */
 
+#define _GNU_SOURCE
+
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <errno.h>
 #include <sys/epoll.h>
+#include <string.h>
+#include <stdlib.h>
 
 #include "os-compatibility.h"
 
@@ -93,3 +97,74 @@ os_epoll_create_cloexec(void)
 	fd = epoll_create(1);
 	return set_cloexec_or_close(fd);
 }
+
+static int
+create_tmpfile_cloexec(char *tmpname)
+{
+	int fd;
+
+#ifdef HAVE_MKOSTEMP
+	fd = mkostemp(tmpname, O_CLOEXEC);
+	if (fd >= 0)
+		unlink(tmpname);
+#else
+	fd = mkstemp(tmpname);
+	if (fd >= 0) {
+		fd = set_cloexec_or_close(fd);
+		unlink(tmpname);
+	}
+#endif
+
+	return fd;
+}
+
+/*
+ * Create a new, unique, anonymous file of the given size, and
+ * return the file descriptor for it. The file descriptor is set
+ * CLOEXEC. The file is immediately suitable for mmap()'ing
+ * the given size at offset zero.
+ *
+ * The file should not have a permanent backing store like a disk,
+ * but may have if XDG_RUNTIME_DIR is not properly implemented in OS.
+ *
+ * The file name is deleted from the file system.
+ *
+ * The file is suitable for buffer sharing between processes by
+ * transmitting the file descriptor over Unix sockets using the
+ * SCM_RIGHTS methods.
+ */
+int
+os_create_anonymous_file(off_t size)
+{
+	static const char template[] = "/weston-shared-XXXXXX";
+	const char *path;
+	char *name;
+	int fd;
+
+	path = getenv("XDG_RUNTIME_DIR");
+	if (!path) {
+		errno = ENOENT;
+		return -1;
+	}
+
+	name = malloc(strlen(path) + sizeof(template));
+	if (!name)
+		return -1;
+
+	strcpy(name, path);
+	strcat(name, template);
+
+	fd = create_tmpfile_cloexec(name);
+
+	free(name);
+
+	if (fd < 0)
+		return -1;
+
+	if (ftruncate(fd, size) < 0) {
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+}
diff --git a/shared/os-compatibility.h b/shared/os-compatibility.h
index 6ada7b6..b2021f1 100644
--- a/shared/os-compatibility.h
+++ b/shared/os-compatibility.h
@@ -23,6 +23,10 @@
 #ifndef OS_COMPATIBILITY_H
 #define OS_COMPATIBILITY_H
 
+#include <sys/types.h>
+
+#include "../config.h"
+
 #ifdef HAVE_EXECINFO_H
 #include <execinfo.h>
 #else
@@ -39,4 +43,7 @@ os_socketpair_cloexec(int domain, int type, int protocol, int *sv);
 int
 os_epoll_create_cloexec(void);
 
+int
+os_create_anonymous_file(off_t size);
+
 #endif /* OS_COMPATIBILITY_H */
diff --git a/src/compositor.c b/src/compositor.c
index 7cc176c..93eeb24 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -2279,7 +2279,6 @@ static void
 weston_xkb_info_new_keymap(struct weston_xkb_info *xkb_info)
 {
 	char *keymap_str;
-	char *path;
 
 	xkb_info->ctrl_mod = xkb_map_mod_get_index(xkb_info->keymap,
 						   XKB_MOD_NAME_CTRL);
@@ -2302,46 +2301,30 @@ weston_xkb_info_new_keymap(struct weston_xkb_info *xkb_info)
 	}
 	xkb_info->keymap_size = strlen(keymap_str) + 1;
 
-	/* Create a temporary file in /dev/shm to use for mapping the keymap,
-	 * and then unlink it as soon as we can. */
-	path = strdup("/dev/shm/weston-keymap-XXXXXX");
-	if (path == NULL) {
-		fprintf(stderr, "failed to allocate keymap path\n");
-		goto err_keymap_str;
-	}
-
-	xkb_info->keymap_fd = mkostemp(path, O_CLOEXEC);
+	xkb_info->keymap_fd = os_create_anonymous_file(xkb_info->keymap_size);
 	if (xkb_info->keymap_fd < 0) {
-		fprintf(stderr, "failed to create temporary keymap file\n");
-		goto err_path;
-	}
-	unlink(path);
-
-	if (ftruncate(xkb_info->keymap_fd, xkb_info->keymap_size) != 0) {
-		fprintf(stderr, "failed to enlarage temporary keymap file\n");
-		goto err_path;
+		fprintf(stderr,
+			"creating a keymap file for %lu bytes failed: %m\n",
+			(unsigned long) xkb_info->keymap_size);
+		goto err_keymap_str;
 	}
 
 	xkb_info->keymap_area = mmap(NULL, xkb_info->keymap_size,
 				     PROT_READ | PROT_WRITE,
 				     MAP_SHARED, xkb_info->keymap_fd, 0);
 	if (xkb_info->keymap_area == MAP_FAILED) {
-		fprintf(stderr, "failed to mmap() %lu bytes on %s\n",
-			(unsigned long) xkb_info->keymap_size,
-			path);
+		fprintf(stderr, "failed to mmap() %lu bytes\n",
+			(unsigned long) xkb_info->keymap_size);
 		goto err_dev_zero;
 	}
 	strcpy(xkb_info->keymap_area, keymap_str);
 	free(keymap_str);
-	free(path);
 
 	return;
 
 err_dev_zero:
 	close(xkb_info->keymap_fd);
 	xkb_info->keymap_fd = -1;
-err_path:
-	free(path);
 err_keymap_str:
 	free(keymap_str);
 	exit(EXIT_FAILURE);
-- 
1.7.3.4



More information about the wayland-devel mailing list