[1/2,roland/namedsem] Refactor shm_{open,unlink} code to separate Linux-specific directory choice from POSIX-generic code.
diff mbox

Message ID 20141204192939.9EE322C3A08@topped-with-meat.com
State New
Headers show

Commit Message

Roland McGrath Dec. 4, 2014, 7:29 p.m. UTC
The sysdeps/posix/ implementation of shm_open had some bit rot relative to
things (not entirely Linux-specific) done in the sysdeps/unix/sysv/linux/
implementation.  The only thing actually Linux-specific is the code for
deciding the directory name that's usually /dev/shm/.  So this factors that
out into an internal function that can separately have its own sysdeps
variants, leaving the meat shm_open itself common in sysdeps/posix/ again.
There should be no change to any behavior.

Tested x86_64-linux-gnu.  If I don't hear any feedback by the end of the
week, I'll commit this early next week.  But I'd appreciate some review.


Thanks,
Roland


2014-12-04  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/posix/shm-directory.c: New file.
	* sysdeps/posix/shm-directory.h: New file.
	* sysdeps/posix/Makefile [($(subdir) = rt] (librt-routines): Add it.
	* sysdeps/posix/shm_open.c: Use SHM_GET_NAME.
	Use O_NOFOLLOW and O_CLOEXEC if available.  Transmute EISDIR to EINVAL.
	* sysdeps/posix/shm_unlink.c: Use SHM_GET_NAME.
	Transmute EPERM to EACCES.
	* sysdeps/unix/sysv/linux/shm-directory.c: New file, most code taken
	from ...
	* sysdeps/unix/sysv/linux/shm_open.c: ... here.  File removed.
	* sysdeps/unix/sysv/linux/shm_unlink.c: File removed.

Patch
diff mbox

diff --git a/sysdeps/posix/Makefile b/sysdeps/posix/Makefile
index b58aa6a..8e5f7c3 100644
--- a/sysdeps/posix/Makefile
+++ b/sysdeps/posix/Makefile
@@ -3,3 +3,7 @@  L_tmpnam  = 20
 TMP_MAX   = 238328
 L_ctermid = 9
 L_cuserid = 9
+
+ifeq ($(subdir),rt)
+librt-routines += shm-directory
+endif
diff --git a/sysdeps/posix/shm-directory.c b/sysdeps/posix/shm-directory.c
new file mode 100644
index 0000000..aea5f96
--- /dev/null
+++ b/sysdeps/posix/shm-directory.c
@@ -0,0 +1,35 @@ 
+/* Determine directory for shm/sem files.  Generic POSIX version.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include "shm-directory.h"
+#include <unistd.h>
+
+#if _POSIX_MAPPED_FILES
+
+# include <paths.h>
+
+# define SHMDIR (_PATH_DEV "shm/")
+
+const char *
+__shm_directory (size_t *len)
+{
+  *len = sizeof SHMDIR - 1;
+  return SHMDIR;
+}
+
+#endif
diff --git a/sysdeps/posix/shm-directory.h b/sysdeps/posix/shm-directory.h
new file mode 100644
index 0000000..1c4d965
--- /dev/null
+++ b/sysdeps/posix/shm-directory.h
@@ -0,0 +1,63 @@ 
+/* Header for directory for shm/sem files.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _SHM_DIRECTORY_H
+
+#include <errno.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+
+extern const char *__shm_directory (size_t *len);
+
+/* This defines local variables SHM_DIR and SHM_DIRLEN, giving the
+   directory prefix (with trailing slash) and length (not including '\0'
+   terminator) of the directory used for shm files.  If that cannot be
+   determined, it sets errno to ENOSYS and returns RETVAL_FOR_INVALID.
+
+   This uses the local variable NAME as an lvalue, and increments it past
+   any leading slashes.  It then defines the local variable NAMELEN, giving
+   strlen (NAME) + 1.  If NAME is invalid, it sets errno to
+   ERRNO_FOR_INVALID and returns RETVAL_FOR_INVALID.  Finally, it defines
+   the local variable SHM_NAME, giving the absolute file name of the shm
+   file corresponding to NAME.  */
+
+#define SHM_GET_NAME(errno_for_invalid, retval_for_invalid)		      \
+  size_t shm_dirlen;							      \
+  const char *shm_dir = __shm_directory (&shm_dirlen);			      \
+  /* If we don't know what directory to use, there is nothing we can do.  */  \
+  if (__glibc_unlikely (shm_dir == NULL))				      \
+    {									      \
+      __set_errno (ENOSYS);						      \
+      return retval_for_invalid;					      \
+    }									      \
+  /* Construct the filename.  */					      \
+  while (name[0] == '/')						      \
+    ++name;								      \
+  size_t namelen = strlen (name) + 1;					      \
+  /* Validate the filename.  */						      \
+  if (namelen == 1 || namelen >= NAME_MAX || strchr (name, '/') != NULL)      \
+    {									      \
+      __set_errno (errno_for_invalid);					      \
+      return retval_for_invalid;					      \
+    }									      \
+  char *shm_name = __alloca (shm_dirlen + namelen);			      \
+  __mempcpy (__mempcpy (shm_name, shm_dir, shm_dirlen), name, namelen)
+
+#endif	/* shm-directory.h */
diff --git a/sysdeps/posix/shm_open.c b/sysdeps/posix/shm_open.c
index 456b3d8..064fecf 100644
--- a/sysdeps/posix/shm_open.c
+++ b/sysdeps/posix/shm_open.c
@@ -19,50 +19,41 @@ 
 #include <unistd.h>
 
 #if ! _POSIX_MAPPED_FILES
-#include <rt/shm_open.c>
+
+# include <rt/shm_open.c>
 
 #else
 
-#include <errno.h>
-#include <sys/mman.h>
-#include <fcntl.h>
-#include <string.h>
-#include <stdlib.h>
-#include <paths.h>
+# include <fcntl.h>
+# include <shm-directory.h>
 
-#define SHMDIR	(_PATH_DEV "shm/")
 
 /* Open shared memory object.  */
 int
 shm_open (const char *name, int oflag, mode_t mode)
 {
-  size_t namelen;
-  char *fname;
-  int fd;
-
-  /* Construct the filename.  */
-  while (name[0] == '/')
-    ++name;
-
-  if (name[0] == '\0')
-    {
-      /* The name "/" is not supported.  */
-      __set_errno (EINVAL);
-      return -1;
-    }
-
-  namelen = strlen (name);
-  fname = (char *) __alloca (sizeof SHMDIR - 1 + namelen + 1);
-  __mempcpy (__mempcpy (fname, SHMDIR, sizeof SHMDIR - 1),
-	     name, namelen + 1);
-
-  fd = open (name, oflag, mode);
+  SHM_GET_NAME (EINVAL, -1);
+
+# ifdef O_NOFOLLOW
+  oflag |= O_NOFOLLOW;
+# endif
+# ifdef O_CLOEXEC
+  oflag |= O_CLOEXEC;
+# endif
+  int fd = open (shm_name, oflag, mode);
+  if (fd == -1 && __glibc_unlikely (errno == EISDIR))
+    /* It might be better to fold this error with EINVAL since
+       directory names are just another example for unsuitable shared
+       object names and the standard does not mention EISDIR.  */
+    __set_errno (EINVAL);
+
+# ifndef O_CLOEXEC
   if (fd != -1)
     {
       /* We got a descriptor.  Now set the FD_CLOEXEC bit.  */
       int flags = fcntl (fd, F_GETFD, 0);
 
-      if (__builtin_expect (flags, 0) != -1)
+      if (__glibc_likely (flags != -1))
 	{
 	  flags |= FD_CLOEXEC;
 	  flags = fcntl (fd, F_SETFD, flags);
@@ -77,8 +68,9 @@  shm_open (const char *name, int oflag, mode_t mode)
 	  __set_errno (save_errno);
 	}
     }
+# endif
 
   return fd;
 }
 
-#endif
+#endif  /* _POSIX_MAPPED_FILES */
diff --git a/sysdeps/posix/shm_unlink.c b/sysdeps/posix/shm_unlink.c
index dc94e16..81c3a02 100644
--- a/sysdeps/posix/shm_unlink.c
+++ b/sysdeps/posix/shm_unlink.c
@@ -24,37 +24,20 @@ 
 #else
 
 #include <errno.h>
-#include <sys/mman.h>
 #include <string.h>
-#include <stdlib.h>
-#include <paths.h>
+#include "shm-directory.h"
 
-#define SHMDIR	(_PATH_DEV "shm/")
 
 /* Remove shared memory object.  */
 int
 shm_unlink (const char *name)
 {
-  size_t namelen;
-  char *fname;
-
-  /* Construct the filename.  */
-  while (name[0] == '/')
-    ++name;
-
-  if (name[0] == '\0')
-    {
-      /* The name "/" is not supported.  */
-      __set_errno (EINVAL);
-      return -1;
-    }
-
-  namelen = strlen (name);
-  fname = (char *) __alloca (sizeof SHMDIR - 1 + namelen + 1);
-  __mempcpy (__mempcpy (fname, SHMDIR, sizeof SHMDIR - 1),
-	     name, namelen + 1);
-
-  return unlink (name);
+  SHM_GET_NAME (ENOENT, -1);
+
+  int result = unlink (shm_name);
+  if (result < 0 && errno == EPERM)
+    __set_errno (EACCES);
+  return result;
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/shm-directory.c b/sysdeps/unix/sysv/linux/shm-directory.c
new file mode 100644
index 0000000..9340f9c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/shm-directory.c
@@ -0,0 +1,144 @@ 
+/* Determine directory for shm/sem files.  Linux version.
+   Copyright (C) 2000-2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include "shm-directory.h"
+
+#include <errno.h>
+#include <mntent.h>
+#include <paths.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/statfs.h>
+#include <bits/libc-lock.h>
+#include "linux_fsinfo.h"
+
+
+/* Mount point of the shared memory filesystem.  */
+static struct
+{
+  char *dir;
+  size_t dirlen;
+} mountpoint;
+
+/* This is the default directory.  */
+static const char defaultdir[] = "/dev/shm/";
+
+/* Protect the `mountpoint' variable above.  */
+__libc_once_define (static, once);
+
+
+/* Determine where the shmfs is mounted (if at all).  */
+static void
+where_is_shmfs (void)
+{
+  char buf[512];
+  struct statfs f;
+  struct mntent resmem;
+  struct mntent *mp;
+  FILE *fp;
+
+  /* The canonical place is /dev/shm.  This is at least what the
+     documentation tells everybody to do.  */
+  if (__statfs (defaultdir, &f) == 0 && (f.f_type == SHMFS_SUPER_MAGIC
+                                         || f.f_type == RAMFS_MAGIC))
+    {
+      /* It is in the normal place.  */
+      mountpoint.dir = (char *) defaultdir;
+      mountpoint.dirlen = sizeof (defaultdir) - 1;
+
+      return;
+    }
+
+  /* OK, do it the hard way.  Look through the /proc/mounts file and if
+     this does not exist through /etc/fstab to find the mount point.  */
+  fp = __setmntent ("/proc/mounts", "r");
+  if (__glibc_unlikely (fp == NULL))
+    {
+      fp = __setmntent (_PATH_MNTTAB, "r");
+      if (__glibc_unlikely (fp == NULL))
+        /* There is nothing we can do.  Blind guesses are not helpful.  */
+        return;
+    }
+
+  /* Now read the entries.  */
+  while ((mp = __getmntent_r (fp, &resmem, buf, sizeof buf)) != NULL)
+    /* The original name is "shm" but this got changed in early Linux
+       2.4.x to "tmpfs".  */
+    if (strcmp (mp->mnt_type, "tmpfs") == 0
+        || strcmp (mp->mnt_type, "shm") == 0)
+      {
+        /* Found it.  There might be more than one place where the
+           filesystem is mounted but one is enough for us.  */
+        size_t namelen;
+
+        /* First make sure this really is the correct entry.  At least
+           some versions of the kernel give wrong information because
+           of the implicit mount of the shmfs for SysV IPC.  */
+        if (__statfs (mp->mnt_dir, &f) != 0 || (f.f_type != SHMFS_SUPER_MAGIC
+                                                && f.f_type != RAMFS_MAGIC))
+          continue;
+
+        namelen = strlen (mp->mnt_dir);
+
+        if (namelen == 0)
+          /* Hum, maybe some crippled entry.  Keep on searching.  */
+          continue;
+
+        mountpoint.dir = (char *) malloc (namelen + 2);
+        if (mountpoint.dir != NULL)
+          {
+            char *cp = __mempcpy (mountpoint.dir, mp->mnt_dir, namelen);
+            if (cp[-1] != '/')
+              *cp++ = '/';
+            *cp = '\0';
+            mountpoint.dirlen = cp - mountpoint.dir;
+          }
+
+        break;
+      }
+
+  /* Close the stream.  */
+  __endmntent (fp);
+}
+
+
+const char *
+__shm_directory (size_t *len)
+{
+  /* Determine where the shmfs is mounted.  */
+  __libc_once (once, where_is_shmfs);
+
+  /* If we don't know the mount points there is nothing we can do.  Ever.  */
+  if (__glibc_unlikely (mountpoint.dir == NULL))
+    {
+      __set_errno (ENOSYS);
+      return NULL;
+    }
+
+  *len = mountpoint.dirlen;
+  return mountpoint.dir;
+}
+
+
+/* Make sure the table is freed if we want to free everything before
+   exiting.  */
+libc_freeres_fn (freeit)
+{
+  if (mountpoint.dir != defaultdir)
+    free (mountpoint.dir);
+}
diff --git a/sysdeps/unix/sysv/linux/shm_open.c b/sysdeps/unix/sysv/linux/shm_open.c
deleted file mode 100644
index a0fa958..0000000
--- a/sysdeps/unix/sysv/linux/shm_open.c
+++ /dev/null
@@ -1,226 +0,0 @@ 
-/* Copyright (C) 2000-2014 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <fcntl.h>
-#include <mntent.h>
-#include <paths.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <sys/mman.h>
-#include <sys/statfs.h>
-#include <bits/libc-lock.h>
-#include "linux_fsinfo.h"
-
-
-/* Mount point of the shared memory filesystem.  */
-static struct
-{
-  char *dir;
-  size_t dirlen;
-} mountpoint;
-
-/* This is the default directory.  */
-static const char defaultdir[] = "/dev/shm/";
-
-/* Protect the `mountpoint' variable above.  */
-__libc_once_define (static, once);
-
-
-/* Determine where the shmfs is mounted (if at all).  */
-static void
-where_is_shmfs (void)
-{
-  char buf[512];
-  struct statfs f;
-  struct mntent resmem;
-  struct mntent *mp;
-  FILE *fp;
-
-  /* The canonical place is /dev/shm.  This is at least what the
-     documentation tells everybody to do.  */
-  if (__statfs (defaultdir, &f) == 0 && (f.f_type == SHMFS_SUPER_MAGIC
-					 || f.f_type == RAMFS_MAGIC))
-    {
-      /* It is in the normal place.  */
-      mountpoint.dir = (char *) defaultdir;
-      mountpoint.dirlen = sizeof (defaultdir) - 1;
-
-      return;
-    }
-
-  /* OK, do it the hard way.  Look through the /proc/mounts file and if
-     this does not exist through /etc/fstab to find the mount point.  */
-  fp = __setmntent ("/proc/mounts", "r");
-  if (__glibc_unlikely (fp == NULL))
-    {
-      fp = __setmntent (_PATH_MNTTAB, "r");
-      if (__glibc_unlikely (fp == NULL))
-	/* There is nothing we can do.  Blind guesses are not helpful.  */
-	return;
-    }
-
-  /* Now read the entries.  */
-  while ((mp = __getmntent_r (fp, &resmem, buf, sizeof buf)) != NULL)
-    /* The original name is "shm" but this got changed in early Linux
-       2.4.x to "tmpfs".  */
-    if (strcmp (mp->mnt_type, "tmpfs") == 0
-	|| strcmp (mp->mnt_type, "shm") == 0)
-      {
-	/* Found it.  There might be more than one place where the
-           filesystem is mounted but one is enough for us.  */
-	size_t namelen;
-
-	/* First make sure this really is the correct entry.  At least
-	   some versions of the kernel give wrong information because
-	   of the implicit mount of the shmfs for SysV IPC.  */
-	if (__statfs (mp->mnt_dir, &f) != 0 || (f.f_type != SHMFS_SUPER_MAGIC
-						&& f.f_type != RAMFS_MAGIC))
-	  continue;
-
-	namelen = strlen (mp->mnt_dir);
-
-	if (namelen == 0)
-	  /* Hum, maybe some crippled entry.  Keep on searching.  */
-	  continue;
-
-	mountpoint.dir = (char *) malloc (namelen + 2);
-	if (mountpoint.dir != NULL)
-	  {
-	    char *cp = __mempcpy (mountpoint.dir, mp->mnt_dir, namelen);
-	    if (cp[-1] != '/')
-	      *cp++ = '/';
-	    *cp = '\0';
-	    mountpoint.dirlen = cp - mountpoint.dir;
-	  }
-
-	break;
-      }
-
-  /* Close the stream.  */
-  __endmntent (fp);
-}
-
-
-/* Open shared memory object.  This implementation assumes the shmfs
-   implementation introduced in the late 2.3.x kernel series to be
-   available.  Normally the filesystem will be mounted at /dev/shm but
-   we fall back on searching for the actual mount point should opening
-   such a file fail.  */
-int
-shm_open (const char *name, int oflag, mode_t mode)
-{
-  size_t namelen;
-  char *fname;
-  int fd;
-
-  /* Determine where the shmfs is mounted.  */
-  __libc_once (once, where_is_shmfs);
-
-  /* If we don't know the mount points there is nothing we can do.  Ever.  */
-  if (mountpoint.dir == NULL)
-    {
-      __set_errno (ENOSYS);
-      return -1;
-    }
-
-  /* Construct the filename.  */
-  while (name[0] == '/')
-    ++name;
-
-  namelen = strlen (name);
-
-  /* Validate the filename.  */
-  if (name[0] == '\0' || namelen > NAME_MAX || strchr (name, '/') != NULL)
-    {
-      __set_errno (EINVAL);
-      return -1;
-    }
-
-  fname = (char *) alloca (mountpoint.dirlen + namelen + 1);
-  __mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
-	     name, namelen + 1);
-
-  /* And get the file descriptor.
-     XXX Maybe we should test each descriptor whether it really is for a
-     file on the shmfs.  If this is what should be done the whole function
-     should be revamped since we can determine whether shmfs is available
-     while trying to open the file, all in one turn.  */
-  fd = open (fname, oflag | O_CLOEXEC | O_NOFOLLOW, mode);
-  if (fd == -1 && __glibc_unlikely (errno == EISDIR))
-    /* It might be better to fold this error with EINVAL since
-       directory names are just another example for unsuitable shared
-       object names and the standard does not mention EISDIR.  */
-    __set_errno (EINVAL);
-
-  return fd;
-}
-
-
-/* Unlink a shared memory object.  */
-int
-shm_unlink (const char *name)
-{
-  size_t namelen;
-  char *fname;
-
-  /* Determine where the shmfs is mounted.  */
-  __libc_once (once, where_is_shmfs);
-
-  if (mountpoint.dir == NULL)
-    {
-      /* We cannot find the shmfs.  If `name' is really a shared
-	 memory object it must have been created by another process
-	 and we have no idea where that process found the mountpoint.  */
-      __set_errno (ENOENT);
-      return -1;
-    }
-
-  /* Construct the filename.  */
-  while (name[0] == '/')
-    ++name;
-
-  namelen = strlen (name);
-
-  /* Validate the filename.  */
-  if (name[0] == '\0' || namelen > NAME_MAX || strchr (name, '/') != NULL)
-    {
-      __set_errno (ENOENT);
-      return -1;
-    }
-
-  fname = (char *) alloca (mountpoint.dirlen + namelen + 1);
-  __mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
-	     name, namelen + 1);
-
-  /* And remove the file.  */
-  int ret = unlink (fname);
-  if (ret < 0 && errno == EPERM)
-    __set_errno (EACCES);
-  return ret;
-}
-
-
-/* Make sure the table is freed if we want to free everything before
-   exiting.  */
-libc_freeres_fn (freeit)
-{
-  if (mountpoint.dir != defaultdir)
-    free (mountpoint.dir);
-}
diff --git a/sysdeps/unix/sysv/linux/shm_unlink.c b/sysdeps/unix/sysv/linux/shm_unlink.c
deleted file mode 100644
index 55cece2..0000000
--- a/sysdeps/unix/sysv/linux/shm_unlink.c
+++ /dev/null
@@ -1 +0,0 @@ 
-/* This function is for technical reason defined in shm_open.c.  */