Introduce xcb_aux_alloc_shm() to create anonymous files in RAM

Submitted by Alexander Volkov on April 10, 2018, 11:38 a.m.

Details

Message ID 20180410113805.28780-1-a.volkov@rusbitech.ru
State New
Headers show
Series "Introduce xcb_aux_alloc_shm() to create anonymous files in RAM" ( rev: 1 ) in XCB

Browsing this patch as part of:
"Introduce xcb_aux_alloc_shm() to create anonymous files in RAM" rev 1 in XCB
<< prev patch [1/1] next patch >>

Commit Message

Alexander Volkov April 10, 2018, 11:38 a.m.
... 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 <a.volkov@rusbitech.ru>
---
 configure.ac  | 47 +++++++++++++++++++++++++++++
 src/xcb_aux.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/xcb_aux.h |  3 ++
 3 files changed, 133 insertions(+)

Patch hide | download patch | download mbox

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

Comments

On 10.04.2018 13:38, Alexander Volkov wrote:
> ... 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 <a.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)]),

Default is yes, not auto. See next two lines.

> +[],
> +[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?)

> +			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?

> 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?

[...]
> +int
> +xcb_aux_alloc_shm(size_t size)

Should the documentation mention anything about sealing? close-on-exec?

> +{
> +	char	template[] = SHMDIR "/shmfd-XXXXXX";

This uses shmfd, but...

> +	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
13.04.2018 09:47, Uli Schlachter пишет:
>> +[],
>> +[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...

> 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.

>> +
>> +#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.

> Also, why MFD_ALLOW_SEALING?
>
> [...]
>> +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.

>> +{
>> +	char	template[] = SHMDIR "/shmfd-XXXXXX";
> This uses shmfd, but...
>
>> +	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.
On 04/17/2018 12:52 PM, Alexander Volkov wrote:
> 13.04.2018 09:47, Uli Schlachter пишет:
>>> +[],
>>> +[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...
> 
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
17.04.2018 16:26, 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.