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.vol...@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 -- Happiness can't be found -- it finds you. - Majic _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel