Discussion:
[Xcb] [PATCH] Introduce xcb_aux_alloc_shm() to create anonymous files in RAM
Alexander Volkov
2018-04-10 11:38:05 UTC
Permalink
... which then can be mapped with mmap().
It is intended to be used in conjunction with xcb_shm_attach_fd()
to access the created shared memory from a client and the X server.

Signed-off-by: Alexander Volkov <***@rusbitech.ru>
---
configure.ac | 47 +++++++++++++++++++++++++++++
src/xcb_aux.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/xcb_aux.h | 3 ++
3 files changed, 133 insertions(+)

diff --git a/configure.ac b/configure.ac
index 1fe1561..81e1f6b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,10 +7,57 @@ AC_CONFIG_HEADERS([config.h])
AC_CONFIG_MACRO_DIR([m4])
AM_INIT_AUTOMAKE([foreign dist-bzip2])

+AC_USE_SYSTEM_EXTENSIONS
+
XCB_UTIL_COMMON([1.4], [1.6])

AC_CHECK_FUNCS_ONCE(vasprintf)

+AC_CHECK_FUNCS(memfd_create mkostemp)
+
+AC_CHECK_DECLS([__NR_memfd_create], [], [], [[#include <asm/unistd.h>]])
+
+AC_CHECK_HEADERS([sys/memfd.h], [AC_DEFINE([HAVE_MEMFD_H], 1, [Has sys/memfd.h header])])
+
+AC_ARG_WITH(shared-memory-dir, AS_HELP_STRING([--with-shared-memory-dir=PATH], [Path to directory in a world-writable temporary directory for anonymous shared memory (default: auto)]),
+[],
+[with_shared_memory_dir=yes])
+
+shmdirs="/run/shm /dev/shm /var/tmp /tmp"
+
+case x"$with_shared_memory_dir" in
+xyes)
+ for dir in $shmdirs; do
+ case x"$with_shared_memory_dir" in
+ xyes)
+ echo Checking temp dir "$dir"
+ if test -d "$dir"; then
+ with_shared_memory_dir="$dir"
+ fi
+ ;;
+ esac
+ done
+ ;;
+x/*)
+ ;;
+xno)
+ ;;
+*)
+ AC_MSG_ERROR([Invalid directory specified for --with-shared-memory-dir: $with_shared_memory_dir])
+ ;;
+esac
+
+case x"$with_shared_memory_dir" in
+xyes)
+ AC_MSG_ERROR([No directory found for shared memory temp files.])
+ ;;
+xno)
+ ;;
+*)
+ AC_DEFINE_UNQUOTED(SHMDIR, ["$with_shared_memory_dir"], [Directory for shared memory temp files])
+ ;;
+esac
+
AC_CONFIG_FILES([Makefile
src/Makefile
xcb-atom.pc
diff --git a/src/xcb_aux.c b/src/xcb_aux.c
index b6f64f8..e8f7ba9 100644
--- a/src/xcb_aux.c
+++ b/src/xcb_aux.c
@@ -33,9 +33,39 @@
#include "config.h"
#endif

+#if !HAVE_MEMFD_CREATE
+#if HAVE_DECL___NR_MEMFD_CREATE
+#include <unistd.h>
+#include <sys/syscall.h>
+static int memfd_create(const char *name,
+ unsigned int flags)
+{
+ return syscall(__NR_memfd_create, name, flags);
+}
+#define HAVE_MEMFD_CREATE 1
+#endif
+#endif
+
+#if HAVE_MEMFD_CREATE
+
+/* Get defines for the memfd_create syscall, using the
+ * header if available, or just defining the constants otherwise
+ */
+
+#if HAVE_MEMFD_H
+#include <sys/memfd.h>
+#else
+/* flags for memfd_create(2) (unsigned int) */
+#define MFD_CLOEXEC 0x0001U
+#define MFD_ALLOW_SEALING 0x0002U
+#endif
+
+#endif
+
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <fcntl.h>

#include <xcb/xcb.h>
#include "xcb_aux.h"
@@ -376,3 +406,56 @@ xcb_aux_clear_window(xcb_connection_t * dpy,
{
return xcb_clear_area(dpy, 0, w, 0, 0, 0, 0);
}
+
+/* SHM related functions */
+
+/**
+ * xcb_aux_alloc_shm:
+ *
+ * Creates an anonymous file of the required size in RAM.
+ * This function is intended for use in conjunction with
+ * xcb_shm_attach_fd().
+ *
+ * Return value: the file descriptor, or -1 on failure
+ * (in which case, errno will be set as appropriate).
+ **/
+int
+xcb_aux_alloc_shm(size_t size)
+{
+ char template[] = SHMDIR "/shmfd-XXXXXX";
+ int fd;
+
+#if HAVE_MEMFD_CREATE
+ fd = memfd_create("xcb_aux", MFD_CLOEXEC|MFD_ALLOW_SEALING);
+ if (fd < 0)
+#endif
+ {
+#ifdef O_TMPFILE
+ fd = open(SHMDIR, O_TMPFILE|O_RDWR|O_CLOEXEC|O_EXCL, 0666);
+ if (fd < 0)
+#endif
+ {
+#ifndef HAVE_MKOSTEMP
+ int flags;
+ fd = mkstemp(template);
+#else
+ fd = mkostemp(template, O_CLOEXEC);
+#endif
+ if (fd < 0)
+ return fd;
+ unlink(template);
+#ifndef HAVE_MKOSTEMP
+ flags = fcntl(fd, F_GETFD);
+ if (flags != -1) {
+ flags |= FD_CLOEXEC;
+ (void) fcntl(fd, F_SETFD, &flags);
+ }
+#endif
+ }
+ }
+ if (ftruncate(fd, size) < 0) {
+ close(fd);
+ return -1;
+ }
+ return fd;
+}
diff --git a/src/xcb_aux.h b/src/xcb_aux.h
index da4fb85..75b6e66 100644
--- a/src/xcb_aux.h
+++ b/src/xcb_aux.h
@@ -206,6 +206,9 @@ xcb_void_cookie_t
xcb_aux_clear_window(xcb_connection_t * dpy,
xcb_window_t w);

+int
+xcb_aux_alloc_shm(size_t size);
+
#ifdef __cplusplus
}
#endif
--
2.17.0
Uli Schlachter
2018-04-13 06:47:53 UTC
Permalink
Post by Alexander Volkov
... which then can be mapped with mmap().
It is intended to be used in conjunction with xcb_shm_attach_fd()
to access the created shared memory from a client and the X server.
---
configure.ac | 47 +++++++++++++++++++++++++++++
src/xcb_aux.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/xcb_aux.h | 3 ++
3 files changed, 133 insertions(+)
diff --git a/configure.ac b/configure.ac
index 1fe1561..81e1f6b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,10 +7,57 @@ AC_CONFIG_HEADERS([config.h])
AC_CONFIG_MACRO_DIR([m4])
AM_INIT_AUTOMAKE([foreign dist-bzip2])
+AC_USE_SYSTEM_EXTENSIONS
+
XCB_UTIL_COMMON([1.4], [1.6])
AC_CHECK_FUNCS_ONCE(vasprintf)
+AC_CHECK_FUNCS(memfd_create mkostemp)
+
+AC_CHECK_DECLS([__NR_memfd_create], [], [], [[#include <asm/unistd.h>]])
+
+AC_CHECK_HEADERS([sys/memfd.h], [AC_DEFINE([HAVE_MEMFD_H], 1, [Has sys/memfd.h header])])
+
+AC_ARG_WITH(shared-memory-dir, AS_HELP_STRING([--with-shared-memory-dir=PATH], [Path to directory in a world-writable temporary directory for anonymous shared memory (default: auto)]),
Default is yes, not auto. See next two lines.
Post by Alexander Volkov
+[],
+[with_shared_memory_dir=yes])
+
+shmdirs="/run/shm /dev/shm /var/tmp /tmp"
+
+case x"$with_shared_memory_dir" in
+xyes)
+ for dir in $shmdirs; do
+ case x"$with_shared_memory_dir" in
+ xyes)
How many shells are there that do not like "break"? :-/
(And does autoconf provide some way to make this loop nicer?)
Post by Alexander Volkov
+ echo Checking temp dir "$dir"
+ if test -d "$dir"; then
+ with_shared_memory_dir="$dir"
+ fi
+ ;;
+ esac
+ done
+ ;;
+x/*)
+ ;;
+xno)
+ ;;
+*)
+ AC_MSG_ERROR([Invalid directory specified for --with-shared-memory-dir: $with_shared_memory_dir])
+ ;;
+esac
+
+case x"$with_shared_memory_dir" in
+xyes)
+ AC_MSG_ERROR([No directory found for shared memory temp files.])
+ ;;
+xno)
+ ;;
+*)
+ AC_DEFINE_UNQUOTED(SHMDIR, ["$with_shared_memory_dir"], [Directory for shared memory temp files])
+ ;;
+esac
+
AC_CONFIG_FILES([Makefile
src/Makefile
xcb-atom.pc
So, if I do --with-shared-memory-dir=/foobar, configure will tell me
that I specified an invalid directory? Why?? And why does this flag
exist at all, i.e. what would be uses for it?

Also, why does the existence of a directory at compile time have any
significance at run time? What about cross compiling? Building a distro
package with /run/shm existing and then running it on another system
that does not have /run/shm?

I really think that this compile time check is plain wrong.

Also, is it really worth it to have all these different cases?

- Linux with updated glibc that provides a memfd_create() wrapper
- Linux with older glibc where this code provides its own wrapper
- Older Linux with memfd_create() (-> mkostemp())
- Anything else (-> mkstemp())

How about dropping the second one (own syscall wrapper if glibc does not
provide one). Oh and how about making sys/memfd.h required instead of
providing the defines here instead, if needed.

What would be the downside of this? How many people would complain?
Post by Alexander Volkov
diff --git a/src/xcb_aux.c b/src/xcb_aux.c
index b6f64f8..e8f7ba9 100644
--- a/src/xcb_aux.c
+++ b/src/xcb_aux.c
@@ -33,9 +33,39 @@
#include "config.h"
#endif
+#if !HAVE_MEMFD_CREATE
+#if HAVE_DECL___NR_MEMFD_CREATE
+#include <unistd.h>
+#include <sys/syscall.h>
+static int memfd_create(const char *name,
+ unsigned int flags)
+{
+ return syscall(__NR_memfd_create, name, flags);
+}
+#define HAVE_MEMFD_CREATE 1
+#endif
+#endif
+
+#if HAVE_MEMFD_CREATE
+
+/* Get defines for the memfd_create syscall, using the
+ * header if available, or just defining the constants otherwise
+ */
+
+#if HAVE_MEMFD_H
+#include <sys/memfd.h>
+#else
+/* flags for memfd_create(2) (unsigned int) */
+#define MFD_CLOEXEC 0x0001U
+#define MFD_ALLOW_SEALING 0x0002U
+#endif
This looks quite linux-specific. Is it worth adding a linux-specific #if
in here in case any other platform ever implements this API as well and
assigns flags differently?

Also, why MFD_ALLOW_SEALING?

[...]
Post by Alexander Volkov
+int
+xcb_aux_alloc_shm(size_t size)
Should the documentation mention anything about sealing? close-on-exec?
Post by Alexander Volkov
+{
+ char template[] = SHMDIR "/shmfd-XXXXXX";
This uses shmfd, but...
Post by Alexander Volkov
+ int fd;
+
+#if HAVE_MEMFD_CREATE
+ fd = memfd_create("xcb_aux", MFD_CLOEXEC|MFD_ALLOW_SEALING);
[...]

...this uses xcb_aux.

Shouldn't they best use the same template name?

Cheers,
Uli
--
Happiness can't be found -- it finds you.
- Majic
Alexander Volkov
2018-04-17 10:52:06 UTC
Permalink
Post by Uli Schlachter
Post by Alexander Volkov
+[],
+[with_shared_memory_dir=yes])
+
+shmdirs="/run/shm /dev/shm /var/tmp /tmp"
+
+case x"$with_shared_memory_dir" in
+xyes)
+ for dir in $shmdirs; do
+ case x"$with_shared_memory_dir" in
+ xyes)
How many shells are there that do not like "break"? :-/
(And does autoconf provide some way to make this loop nicer?)
So, if I do --with-shared-memory-dir=/foobar, configure will tell me
that I specified an invalid directory? Why?? And why does this flag
exist at all, i.e. what would be uses for it?
Also, why does the existence of a directory at compile time have any
significance at run time? What about cross compiling? Building a distro
package with /run/shm existing and then running it on another system
that does not have /run/shm?
I really think that this compile time check is plain wrong.
Well, I just copied it from
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/configure.ac
The same code is used in the X server:
https://cgit.freedesktop.org/xorg/xserver/tree/configure.ac#n1121
Somehow it's working there...
Post by Uli Schlachter
Also, is it really worth it to have all these different cases?
- Linux with updated glibc that provides a memfd_create() wrapper
- Linux with older glibc where this code provides its own wrapper
- Older Linux with memfd_create() (-> mkostemp())
- Anything else (-> mkstemp())
How about dropping the second one (own syscall wrapper if glibc does not
provide one). Oh and how about making sys/memfd.h required instead of
providing the defines here instead, if needed.
What would be the downside of this? How many people would complain?
memfd_create() was introduced only in the latest glibc 2.27,
there are many systems where it's not available yet.
I'd prefer not to drop the wrapper.
Post by Uli Schlachter
Post by Alexander Volkov
+
+#if HAVE_MEMFD_CREATE
+
+/* Get defines for the memfd_create syscall, using the
+ * header if available, or just defining the constants otherwise
+ */
+
+#if HAVE_MEMFD_H
+#include <sys/memfd.h>
+#else
+/* flags for memfd_create(2) (unsigned int) */
+#define MFD_CLOEXEC 0x0001U
+#define MFD_ALLOW_SEALING 0x0002U
+#endif
This looks quite linux-specific. Is it worth adding a linux-specific #if
in here in case any other platform ever implements this API as well and
assigns flags differently?
I copied this from
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/src/xshmfence_alloc.c
Yes, it seems to be better to make sys/memfd.h required.
Post by Uli Schlachter
Also, why MFD_ALLOW_SEALING?
[...]
Post by Alexander Volkov
+int
+xcb_aux_alloc_shm(size_t size)
Should the documentation mention anything about sealing? close-on-exec?
Yes, it is worth mentioning them.
Besides the size of the file should be fixed with F_ADD_SEALS.
Post by Uli Schlachter
Post by Alexander Volkov
+{
+ char template[] = SHMDIR "/shmfd-XXXXXX";
This uses shmfd, but...
Post by Alexander Volkov
+ int fd;
+
+#if HAVE_MEMFD_CREATE
+ fd = memfd_create("xcb_aux", MFD_CLOEXEC|MFD_ALLOW_SEALING);
[...]
...this uses xcb_aux.
Shouldn't they best use the same template name?
Agree.
Julien Cristau
2018-04-17 13:26:38 UTC
Permalink
Post by Alexander Volkov
Post by Uli Schlachter
Post by Alexander Volkov
+[],
+[with_shared_memory_dir=yes])
+
+shmdirs="/run/shm /dev/shm /var/tmp /tmp"
+
+case x"$with_shared_memory_dir" in
+xyes)
+    for dir in $shmdirs; do
+        case x"$with_shared_memory_dir" in
+        xyes)
How many shells are there that do not like "break"? :-/
(And does autoconf provide some way to make this loop nicer?)
So, if I do --with-shared-memory-dir=/foobar, configure will tell me
that I specified an invalid directory? Why?? And why does this flag
exist at all, i.e. what would be uses for it?
Also, why does the existence of a directory at compile time have any
significance at run time? What about cross compiling? Building a distro
package with /run/shm existing and then running it on another system
that does not have /run/shm?
I really think that this compile time check is plain wrong.
Well, I just copied it from
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/configure.ac
https://cgit.freedesktop.org/xorg/xserver/tree/configure.ac#n1121
Somehow it's working there...
What's the use of replicating the code from libxshmfence instead of just
calling into that library? Or are they somehow subtly different (and if
so why)?

Cheers,
Julien
Alexander Volkov
2018-04-17 14:32:09 UTC
Permalink
Post by Julien Cristau
What's the use of replicating the code from libxshmfence instead of
just calling into that library?  Or are they somehow subtly different
(and if so why)?
See
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/src/xshmfence_alloc.c
xshmfence_alloc_shm() allocated memory and initializes it for xshmfence
structure,
while introduced xcb_aux_alloc_shm() is a general-purpose function that
allocates
memory segment of a specified size.

Loading...