[v2,roland/namedsem] NPTL: Refactor named semaphore code to use shm-directory.h
diff mbox

Message ID 20141212005208.A07DB2C3ADB@topped-with-meat.com
State New
Headers show

Commit Message

Roland McGrath Dec. 12, 2014, 12:52 a.m. UTC
I've committed the prerequisite patch already (posted last week as 1/2).
When I posted both patches last week, I thought I'd done full testing but
had somehow missed the localplt failures introduced by the second.  

This version of the patch fixes that, so there are truly no regressions on
x86_64-linux-gnu.  That required the libc-lockP.h changes (so __libc_once
can be used in libpthread), which seem like they could have unexpected
consequences (though I've seen none so far).  I'd still and again
appreciate fresh eyes on this change.  But if nobody responds at all,
then I'll probably commit it on Friday anyhow.


Thanks,
Roland


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

	* sysdeps/posix/shm-directory.h (SHM_GET_NAME): Take new argument
	PREFIX, string constant to insert between directory and name.
	* sysdeps/posix/shm_open.c: Update caller.
	* sysdeps/posix/shm_unlink.c: Likewise.
	* nptl/semaphoreP.h (struct mountpoint_info): Type removed.
	(__where_is_shmfs, mountpoint, __namedsem_once): Declarations removed.
	(SEM_SHM_PREFIX): New macro.
	* sysdeps/posix/Makefile (librt-routines): Add shm-directory only if
	[$(have-thread-library) = no].
	* nptl/Makefile (libpthread-routines): Add shm-directory.
	* nptl/Versions (GLIBC_PRIVATE): Add __shm_directory.
	* sysdeps/nptl/shm-directory.h: New file.
	* sysdeps/posix/shm-directory.c
	[IS_IN (libpthread)] (__shm_directory): Add hidden_def.
	* sysdeps/unix/sysv/linux/shm-directory.c: Likewise.
	* nptl/sem_open.c (check_add_mapping): Use munmap function rather than
	INTERNAL_SYSCALL.
	(__where_is_shmfs): Function removed.
	(mountpoint, defaultmount, defaultdir, __namedsem_once):
	Variables removed.
	(sem_open): Use __libc_close function rather than INTERNAL_SYSCALL.
	Use SHM_GET_NAME.
	* nptl/sem_unlink.c: Prototypify.  Use SHM_GET_NAME.

	* sysdeps/nptl/bits/libc-lockP.h [IS_IN (libpthread)]
	(PTFAVAIL, __libc_ptf_call, __libc_ptf_call_always): Define as
	unconditional for use inside libpthread.
	[IS_IN (libpthread)]: Include <nptl/pthreadP.h>.

Patch
diff mbox

--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -96,6 +96,7 @@  libpthread-routines = nptl-init vars events version \
 		      old_pthread_atfork pthread_atfork \
 		      pthread_getcpuclockid \
 		      pthread_clock_gettime pthread_clock_settime \
+		      shm-directory \
 		      sem_init sem_destroy \
 		      sem_open sem_close sem_unlink \
 		      sem_getvalue \
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -270,5 +270,6 @@  libpthread {
     __pthread_initialize_minimal;
     __pthread_clock_gettime; __pthread_clock_settime;
     __pthread_unwind; __pthread_get_minstack;
+    __shm_directory;
   }
 }
--- a/nptl/sem_open.c
+++ b/nptl/sem_open.c
@@ -18,8 +18,6 @@ 
 
 #include <errno.h>
 #include <fcntl.h>
-#include <mntent.h>
-#include <paths.h>
 #include <pthread.h>
 #include <search.h>
 #include <semaphore.h>
@@ -30,98 +28,8 @@ 
 #include <unistd.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
-#include <sys/statfs.h>
-#include <linux_fsinfo.h>
 #include "semaphoreP.h"
-
-
-
-/* Information about the mount point.  */
-struct mountpoint_info mountpoint attribute_hidden;
-
-/* This is the default mount point.  */
-static const char defaultmount[] = "/dev/shm";
-/* This is the default directory.  */
-static const char defaultdir[] = "/dev/shm/sem.";
-
-/* Protect the `mountpoint' variable above.  */
-pthread_once_t __namedsem_once attribute_hidden = PTHREAD_ONCE_INIT;
-
-
-/* Determine where the shmfs is mounted (if at all).  */
-void
-attribute_hidden
-__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 (defaultmount, &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 + 4 + 2);
-	if (mountpoint.dir != NULL)
-	  {
-	    char *cp = __mempcpy (mountpoint.dir, mp->mnt_dir, namelen);
-	    if (cp[-1] != '/')
-	      *cp++ = '/';
-	    cp = stpcpy (cp, "sem.");
-	    mountpoint.dirlen = cp - mountpoint.dir;
-	  }
-
-	break;
-      }
-
-  /* Close the stream.  */
-  __endmntent (fp);
-}
+#include <shm-directory.h>
 
 
 /* Comparison function for search of existing mapping.  */
@@ -217,8 +125,9 @@  check_add_mapping (const char *name, size_t namelen, int fd, sem_t *existing)
   if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
     {
       /* Do not disturb errno.  */
-      INTERNAL_SYSCALL_DECL (err);
-      INTERNAL_SYSCALL (munmap, err, 2, existing, sizeof (sem_t));
+      int save = errno;
+      munmap (existing, sizeof (sem_t));
+      errno = save;
     }
 
   return result;
@@ -228,42 +137,17 @@  check_add_mapping (const char *name, size_t namelen, int fd, sem_t *existing)
 sem_t *
 sem_open (const char *name, int oflag, ...)
 {
-  char *finalname;
-  sem_t *result = SEM_FAILED;
   int fd;
+  sem_t *result;
 
-  /* Determine where the shmfs is mounted.  */
-  __pthread_once (&__namedsem_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 SEM_FAILED;
-    }
-
-  /* Construct the filename.  */
-  while (name[0] == '/')
-    ++name;
-
-  if (name[0] == '\0')
-    {
-      /* The name "/" is not supported.  */
-      __set_errno (EINVAL);
-      return SEM_FAILED;
-    }
-  size_t namelen = strlen (name) + 1;
-
-  /* Create the name of the final file.  */
-  finalname = (char *) alloca (mountpoint.dirlen + namelen);
-  __mempcpy (__mempcpy (finalname, mountpoint.dir, mountpoint.dirlen),
-	     name, namelen);
+  /* Create the name of the final file in local variable SHM_NAME.  */
+  SHM_GET_NAME (EINVAL, SEM_FAILED, SEM_SHM_PREFIX);
 
   /* If the semaphore object has to exist simply open it.  */
   if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0)
     {
     try_again:
-      fd = __libc_open (finalname,
+      fd = __libc_open (shm_name,
 			(oflag & ~(O_CREAT|O_ACCMODE)) | O_NOFOLLOW | O_RDWR);
 
       if (fd == -1)
@@ -317,8 +201,8 @@  sem_open (const char *name, int oflag, ...)
       memset ((char *) &sem.initsem + sizeof (struct new_sem), '\0',
 	      sizeof (sem_t) - sizeof (struct new_sem));
 
-      tmpfname = (char *) alloca (mountpoint.dirlen + 6 + 1);
-      char *xxxxxx = __mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen);
+      tmpfname = __alloca (shm_dirlen + sizeof SEM_SHM_PREFIX + 6);
+      char *xxxxxx = __mempcpy (tmpfname, shm_dir, shm_dirlen);
 
       int retries = 0;
 #define NRETRIES 50
@@ -361,7 +245,7 @@  sem_open (const char *name, int oflag, ...)
 				       fd, 0)) != MAP_FAILED)
 	{
 	  /* Create the file.  Don't overwrite an existing file.  */
-	  if (link (tmpfname, finalname) != 0)
+	  if (link (tmpfname, shm_name) != 0)
 	    {
 	      /* Undo the mapping.  */
 	      (void) munmap (result, sizeof (sem_t));
@@ -402,8 +286,9 @@  sem_open (const char *name, int oflag, ...)
   if (fd != -1)
     {
       /* Do not disturb errno.  */
-      INTERNAL_SYSCALL_DECL (err);
-      INTERNAL_SYSCALL (close, err, 1, fd);
+      int save = errno;
+      __libc_close (fd);
+      errno = save;
     }
 
   return result;
--- a/nptl/sem_unlink.c
+++ b/nptl/sem_unlink.c
@@ -22,44 +22,16 @@ 
 #include <string.h>
 #include <unistd.h>
 #include "semaphoreP.h"
-
+#include <shm-directory.h>
 
 int
-sem_unlink (name)
-     const char *name;
+sem_unlink (const char *name)
 {
-  char *fname;
-  size_t namelen;
-
-  /* Determine where the shmfs is mounted.  */
-  __pthread_once (&__namedsem_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;
-
-  if (name[0] == '\0')
-    {
-      /* The name "/" is not supported.  */
-      __set_errno (ENOENT);
-      return -1;
-    }
-  namelen = strlen (name);
-
-  /* Create the name of the file.  */
-  fname = (char *) alloca (mountpoint.dirlen + namelen + 1);
-  __mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
-	     name, namelen + 1);
+  SHM_GET_NAME (ENOENT, -1, SEM_SHM_PREFIX);
 
   /* Now try removing it.  */
-  int ret = unlink (fname);
+  int ret = unlink (shm_name);
   if (ret < 0 && errno == EPERM)
     __set_errno (EACCES);
   return ret;
--- a/nptl/semaphoreP.h
+++ b/nptl/semaphoreP.h
@@ -19,13 +19,7 @@ 
 #include <semaphore.h>
 #include "pthreadP.h"
 
-
-/* Mount point of the shared memory filesystem.  */
-struct mountpoint_info
-{
-  char *dir;
-  size_t dirlen;
-};
+#define SEM_SHM_PREFIX  "sem."
 
 /* Keeping track of currently used mappings.  */
 struct inuse_sem
@@ -38,11 +32,6 @@  struct inuse_sem
 };
 
 
-/* Variables used in multiple interfaces.  */
-extern struct mountpoint_info mountpoint attribute_hidden;
-
-extern pthread_once_t __namedsem_once attribute_hidden;
-
 /* The search tree for existing mappings.  */
 extern void *__sem_mappings attribute_hidden;
 
@@ -50,9 +39,6 @@  extern void *__sem_mappings attribute_hidden;
 extern int __sem_mappings_lock attribute_hidden;
 
 
-/* Initializer for mountpoint.  */
-extern void __where_is_shmfs (void) attribute_hidden;
-
 /* Comparison function for search in tree with existing mappings.  */
 extern int __sem_search (const void *a, const void *b) attribute_hidden;
 
--- a/sysdeps/nptl/bits/libc-lockP.h
+++ b/sysdeps/nptl/bits/libc-lockP.h
@@ -34,6 +34,12 @@ 
 #include <tls.h>
 #include <pthread-functions.h>
 
+#if IS_IN (libpthread)
+/* This gets us the declarations of the __pthread_* internal names,
+   and hidden_proto for them.  */
+# include <nptl/pthreadP.h>
+#endif
+
 /* Mutex type.  */
 #if !IS_IN (libc) && !IS_IN (libpthread)
 typedef pthread_mutex_t __libc_lock_t;
@@ -114,6 +120,12 @@  typedef pthread_key_t __libc_key_t;
   (__libc_pthread_functions_init ? PTHFCT_CALL (ptr_##FUNC, ARGS) : ELSE)
 # define __libc_ptf_call_always(FUNC, ARGS) \
   PTHFCT_CALL (ptr_##FUNC, ARGS)
+#elif IS_IN (libpthread)
+# define PTFAVAIL(NAME) 1
+# define __libc_ptf_call(FUNC, ARGS, ELSE) \
+  FUNC ARGS
+# define __libc_ptf_call_always(FUNC, ARGS) \
+  FUNC ARGS
 #else
 # define PTFAVAIL(NAME) (NAME != NULL)
 # define __libc_ptf_call(FUNC, ARGS, ELSE) \
--- /dev/null
+++ b/sysdeps/nptl/shm-directory.h
@@ -0,0 +1,31 @@ 
+/* Header for directory for shm/sem files.  NPTL 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/>.  */
+
+#ifndef _SHM_DIRECTORY_H
+
+#include <sysdeps/posix/shm-directory.h>
+
+/* For NPTL the __shm_directory function lives in libpthread.
+   We don't want PLT calls from there.  But it's also used from
+   librt, so it cannot just be declared hidden.  */
+
+#if IS_IN (libpthread)
+hidden_proto (__shm_directory)
+#endif
+
+#endif  /* shm-directory.h */
--- a/sysdeps/posix/Makefile
+++ b/sysdeps/posix/Makefile
@@ -4,6 +4,8 @@  TMP_MAX   = 238328
 L_ctermid = 9
 L_cuserid = 9
 
-ifeq ($(subdir),rt)
+ifeq ($(subdir)|$(have-thread-library),rt|no)
+# With NPTL, this lives in libpthread so it can be used for sem_open too.
+# Without NPTL, it's just private in librt.
 librt-routines += shm-directory
 endif
--- a/sysdeps/posix/shm-directory.c
+++ b/sysdeps/posix/shm-directory.c
@@ -31,5 +31,8 @@  __shm_directory (size_t *len)
   *len = sizeof SHMDIR - 1;
   return SHMDIR;
 }
+# if IS_IN (libpthread)
+hidden_def (__shm_directory)
+# endif
 
 #endif
--- a/sysdeps/posix/shm-directory.h
+++ b/sysdeps/posix/shm-directory.h
@@ -36,9 +36,10 @@  extern const char *__shm_directory (size_t *len);
    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.  */
+   file corresponding to NAME.  PREFIX is a string constant used as a
+   prefix on NAME.  */
 
-#define SHM_GET_NAME(errno_for_invalid, retval_for_invalid)		      \
+#define SHM_GET_NAME(errno_for_invalid, retval_for_invalid, prefix)           \
   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.  */  \
@@ -57,7 +58,9 @@  extern const char *__shm_directory (size_t *len);
       __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)
+  char *shm_name = __alloca (shm_dirlen + sizeof prefix - 1 + namelen);	      \
+  __mempcpy (__mempcpy (__mempcpy (shm_name, shm_dir, shm_dirlen),	      \
+                        prefix, sizeof prefix - 1),			      \
+             name, namelen)
 
 #endif	/* shm-directory.h */
--- a/sysdeps/posix/shm_open.c
+++ b/sysdeps/posix/shm_open.c
@@ -32,7 +32,7 @@ 
 int
 shm_open (const char *name, int oflag, mode_t mode)
 {
-  SHM_GET_NAME (EINVAL, -1);
+  SHM_GET_NAME (EINVAL, -1, "");
 
 # ifdef O_NOFOLLOW
   oflag |= O_NOFOLLOW;
--- a/sysdeps/posix/shm_unlink.c
+++ b/sysdeps/posix/shm_unlink.c
@@ -32,7 +32,7 @@ 
 int
 shm_unlink (const char *name)
 {
-  SHM_GET_NAME (ENOENT, -1);
+  SHM_GET_NAME (ENOENT, -1, "");
 
   int result = unlink (shm_name);
   if (result < 0 && errno == EPERM)
--- a/sysdeps/unix/sysv/linux/shm-directory.c
+++ b/sysdeps/unix/sysv/linux/shm-directory.c
@@ -133,6 +133,9 @@  __shm_directory (size_t *len)
   *len = mountpoint.dirlen;
   return mountpoint.dir;
 }
+#if IS_IN (libpthread)
+hidden_def (__shm_directory)
+#endif
 
 
 /* Make sure the table is freed if we want to free everything before