pthread: Refactor semaphore code

The internal semaphore list code is moved to a specific file,
sem_routine.c, and the internal usage is simplified to only two
functions (one to insert a new semaphore and one to remove it
from the internal list).  There is no need to expose the
internal locking, neither how the semaphore mapping is implemented.

No functional or semantic change is expected, tested on
x86_64-linux-gnu.

8 files changed, 223 insertions, 208 deletions

diff --git a/htl/Makefile b/htl/Makefile
index 9adc95e..c15c1b1 100644
--- a/htl/Makefile
+++ b/htl/Makefile
@@ -131,6 +131,7 @@ libpthread-routines := pt-attr pt-attr-destroy pt-attr-getdetachstate	    \
 	sem_close sem-destroy sem-getvalue sem-init sem_open		    \
 	sem-post sem-timedwait sem-trywait sem_unlink			    \
 	sem-wait sem-waitfast						    \
+	sem_routines							    \
 									    \
 	cancellation							    \
 	cthreads-compat							    \
diff --git a/htl/semaphoreP.h b/htl/semaphoreP.h
index 3d6efcd..4b13f7b 100644
--- a/htl/semaphoreP.h
+++ b/htl/semaphoreP.h
@@ -21,27 +21,6 @@
 
 #define SEM_SHM_PREFIX  "sem."
 
-/* Keeping track of currently used mappings.  */
-struct inuse_sem
-{
-  dev_t dev;
-  ino_t ino;
-  int refcnt;
-  sem_t *sem;
-  char name[];
-};
-
-
-/* The search tree for existing mappings.  */
-extern void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-extern int __sem_mappings_lock attribute_hidden;
-
-
-/* Comparison function for search in tree with existing mappings.  */
-extern int __sem_search (const void *a, const void *b) attribute_hidden;
-
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
   /* This always is a shared semaphore.  */
diff --git a/nptl/Makefile b/nptl/Makefile
index c49134b..8fb7fee 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -137,7 +137,7 @@ libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
 		      pthread_once \
 		      old_pthread_atfork \
 		      pthread_getcpuclockid \
-		      sem_init sem_destroy \
+		      sem_init sem_destroy sem_routines \
 		      sem_open sem_close sem_unlink \
 		      sem_getvalue \
 		      sem_wait sem_timedwait sem_clockwait sem_post \
diff --git a/nptl/semaphoreP.h b/nptl/semaphoreP.h
index 3585af0..1b78614 100644
--- a/nptl/semaphoreP.h
+++ b/nptl/semaphoreP.h
@@ -22,27 +22,6 @@
 
 #define SEM_SHM_PREFIX  "sem."
 
-/* Keeping track of currently used mappings.  */
-struct inuse_sem
-{
-  dev_t dev;
-  ino_t ino;
-  int refcnt;
-  sem_t *sem;
-  char name[];
-};
-
-
-/* The search tree for existing mappings.  */
-extern void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-extern int __sem_mappings_lock attribute_hidden;
-
-
-/* Comparison function for search in tree with existing mappings.  */
-extern int __sem_search (const void *a, const void *b) attribute_hidden;
-
 static inline void __new_sem_open_init (struct new_sem *sem, unsigned value)
 {
 #if __HAVE_64B_ATOMICS
diff --git a/sysdeps/pthread/sem_close.c b/sysdeps/pthread/sem_close.c
index 8b6c9c6..6649196 100644
--- a/sysdeps/pthread/sem_close.c
+++ b/sysdeps/pthread/sem_close.c
@@ -17,65 +17,17 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
-#include <search.h>
-#include <sys/mman.h>
 #include "semaphoreP.h"
-
-struct walk_closure
-{
-  sem_t *the_sem;
-  struct inuse_sem *rec;
-};
-
-static void
-walker (const void *inodep, VISIT which, void *closure0)
-{
-  struct walk_closure *closure = closure0;
-  struct inuse_sem *nodep = *(struct inuse_sem **) inodep;
-
-  if (nodep->sem == closure->the_sem)
-    closure->rec = nodep;
-}
-
+#include <sem_routines.h>
 
 int
 sem_close (sem_t *sem)
 {
-  int result = 0;
-
-  /* Get the lock.  */
-  lll_lock (__sem_mappings_lock, LLL_PRIVATE);
-
-  /* Locate the entry for the mapping the caller provided.  */
-  struct inuse_sem *rec;
-  {
-    struct walk_closure closure = { .the_sem = sem, .rec = NULL };
-    __twalk_r (__sem_mappings, walker, &closure);
-    rec = closure.rec;
-  }
-  if  (rec != NULL)
+  if (!__sem_remove_mapping (sem))
     {
-      /* Check the reference counter.  If it is going to be zero, free
-	 all the resources.  */
-      if (--rec->refcnt == 0)
-	{
-	  /* Remove the record from the tree.  */
-	  (void) __tdelete (rec, &__sem_mappings, __sem_search);
-
-	  result = munmap (rec->sem, sizeof (sem_t));
-
-	  free (rec);
-	}
-    }
-  else
-    {
-      /* This is no valid semaphore.  */
-      result = -1;
       __set_errno (EINVAL);
+      return -1;
     }
 
-  /* Release the lock.  */
-  lll_unlock (__sem_mappings_lock, LLL_PRIVATE);
-
-  return result;
+  return 0;
 }
diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c
index d666414..028d76a 100644
--- a/sysdeps/pthread/sem_open.c
+++ b/sysdeps/pthread/sem_open.c
@@ -16,127 +16,17 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
 #include <fcntl.h>
-#include <pthread.h>
-#include <search.h>
 #include <semaphore.h>
 #include <stdarg.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <unistd.h>
 #include <sys/mman.h>
-#include <sys/stat.h>
 #include "semaphoreP.h"
 #include <shm-directory.h>
+#include <sem_routines.h>
 #include <futex-internal.h>
 #include <libc-lock.h>
 
-/* Comparison function for search of existing mapping.  */
-int
-attribute_hidden
-__sem_search (const void *a, const void *b)
-{
-  const struct inuse_sem *as = (const struct inuse_sem *) a;
-  const struct inuse_sem *bs = (const struct inuse_sem *) b;
-
-  if (as->ino != bs->ino)
-    /* Cannot return the difference the type is larger than int.  */
-    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
-
-  if (as->dev != bs->dev)
-    /* Cannot return the difference the type is larger than int.  */
-    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
-
-  return strcmp (as->name, bs->name);
-}
-
-
-/* The search tree for existing mappings.  */
-void *__sem_mappings attribute_hidden;
-
-/* Lock to protect the search tree.  */
-int __sem_mappings_lock attribute_hidden = LLL_LOCK_INITIALIZER;
-
-
-/* Search for existing mapping and if possible add the one provided.  */
-static sem_t *
-check_add_mapping (const char *name, int fd, sem_t *existing)
-{
-  size_t namelen = strlen (name);
-  sem_t *result = SEM_FAILED;
-
-  /* Get the information about the file.  */
-  struct stat64 st;
-  if (__fstat64 (fd, &st) == 0)
-    {
-      /* Get the lock.  */
-      lll_lock (__sem_mappings_lock, LLL_PRIVATE);
-
-      /* Search for an existing mapping given the information we have.  */
-      struct inuse_sem *fake;
-      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
-      memcpy (fake->name, name, namelen);
-      fake->dev = st.st_dev;
-      fake->ino = st.st_ino;
-
-      struct inuse_sem **foundp = __tfind (fake, &__sem_mappings,
-					   __sem_search);
-      if (foundp != NULL)
-	{
-	  /* There is already a mapping.  Use it.  */
-	  result = (*foundp)->sem;
-	  ++(*foundp)->refcnt;
-	}
-      else
-	{
-	  /* We haven't found a mapping.  Install ione.  */
-	  struct inuse_sem *newp;
-
-	  newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
-	  if (newp != NULL)
-	    {
-	      /* If the caller hasn't provided any map it now.  */
-	      if (existing == SEM_FAILED)
-		existing = (sem_t *) mmap (NULL, sizeof (sem_t),
-					   PROT_READ | PROT_WRITE, MAP_SHARED,
-					   fd, 0);
-
-	      newp->dev = st.st_dev;
-	      newp->ino = st.st_ino;
-	      newp->refcnt = 1;
-	      newp->sem = existing;
-	      memcpy (newp->name, name, namelen);
-
-	      /* Insert the new value.  */
-	      if (existing != MAP_FAILED
-		  && __tsearch (newp, &__sem_mappings, __sem_search) != NULL)
-		/* Successful.  */
-		result = existing;
-	      else
-		/* Something went wrong while inserting the new
-		   value.  We fail completely.  */
-		free (newp);
-	    }
-	}
-
-      /* Release the lock.  */
-      lll_unlock (__sem_mappings_lock, LLL_PRIVATE);
-    }
-
-  if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
-    {
-      /* Do not disturb errno.  */
-      int save = errno;
-      munmap (existing, sizeof (sem_t));
-      errno = save;
-    }
-
-  return result;
-}
-
-
 sem_t *
 sem_open (const char *name, int oflag, ...)
 {
@@ -183,7 +73,7 @@ sem_open (const char *name, int oflag, ...)
       else
 	/* Check whether we already have this semaphore mapped and
 	   create one if necessary.  */
-	result = check_add_mapping (name, fd, SEM_FAILED);
+	result = __sem_check_add_mapping (name, fd, SEM_FAILED);
     }
   else
     {
@@ -294,7 +184,7 @@ sem_open (const char *name, int oflag, ...)
 	    /* Insert the mapping into the search tree.  This also
 	       determines whether another thread sneaked by and already
 	       added such a mapping despite the fact that we created it.  */
-	    result = check_add_mapping (name, fd, result);
+	    result = __sem_check_add_mapping (name, fd, result);
 	}
 
       /* Now remove the temporary name.  This should never fail.  If
diff --git a/sysdeps/pthread/sem_routines.c b/sysdeps/pthread/sem_routines.c
new file mode 100644
index 0000000..78d9364
--- /dev/null
+++ b/sysdeps/pthread/sem_routines.c
@@ -0,0 +1,187 @@
+/* Helper code for POSIX semaphore implementation.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <search.h>
+#include <semaphoreP.h>
+#include <sys/mman.h>
+#include <sem_routines.h>
+
+/* Keeping track of currently used mappings.  */
+struct inuse_sem
+{
+  dev_t dev;
+  ino_t ino;
+  int refcnt;
+  sem_t *sem;
+  char name[];
+};
+
+/* Comparison function for search of existing mapping.  */
+static int
+sem_search (const void *a, const void *b)
+{
+  const struct inuse_sem *as = (const struct inuse_sem *) a;
+  const struct inuse_sem *bs = (const struct inuse_sem *) b;
+
+  if (as->ino != bs->ino)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
+
+  if (as->dev != bs->dev)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
+
+  return strcmp (as->name, bs->name);
+}
+
+/* The search tree for existing mappings.  */
+static void *sem_mappings;
+
+/* Lock to protect the search tree.  */
+static int sem_mappings_lock = LLL_LOCK_INITIALIZER;
+
+
+/* Search for existing mapping and if possible add the one provided.  */
+sem_t *
+__sem_check_add_mapping (const char *name, int fd, sem_t *existing)
+{
+  size_t namelen = strlen (name);
+  sem_t *result = SEM_FAILED;
+
+  /* Get the information about the file.  */
+  struct stat64 st;
+  if (__fstat64 (fd, &st) == 0)
+    {
+      /* Get the lock.  */
+      lll_lock (sem_mappings_lock, LLL_PRIVATE);
+
+      /* Search for an existing mapping given the information we have.  */
+      struct inuse_sem *fake;
+      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
+      memcpy (fake->name, name, namelen);
+      fake->dev = st.st_dev;
+      fake->ino = st.st_ino;
+
+      struct inuse_sem **foundp = __tfind (fake, &sem_mappings, sem_search);
+      if (foundp != NULL)
+	{
+	  /* There is already a mapping.  Use it.  */
+	  result = (*foundp)->sem;
+	  ++(*foundp)->refcnt;
+	}
+      else
+	{
+	  /* We haven't found a mapping.  Install ione.  */
+	  struct inuse_sem *newp;
+
+	  newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
+	  if (newp != NULL)
+	    {
+	      /* If the caller hasn't provided any map it now.  */
+	      if (existing == SEM_FAILED)
+		existing = (sem_t *) mmap (NULL, sizeof (sem_t),
+					   PROT_READ | PROT_WRITE, MAP_SHARED,
+					   fd, 0);
+
+	      newp->dev = st.st_dev;
+	      newp->ino = st.st_ino;
+	      newp->refcnt = 1;
+	      newp->sem = existing;
+	      memcpy (newp->name, name, namelen);
+
+	      /* Insert the new value.  */
+	      if (existing != MAP_FAILED
+		  && __tsearch (newp, &sem_mappings, sem_search) != NULL)
+		/* Successful.  */
+		result = existing;
+	      else
+		/* Something went wrong while inserting the new
+		   value.  We fail completely.  */
+		free (newp);
+	    }
+	}
+
+      /* Release the lock.  */
+      lll_unlock (sem_mappings_lock, LLL_PRIVATE);
+    }
+
+  if (result != existing && existing != SEM_FAILED && existing != MAP_FAILED)
+    {
+      /* Do not disturb errno.  */
+      int save = errno;
+      munmap (existing, sizeof (sem_t));
+      errno = save;
+    }
+
+  return result;
+}
+
+struct walk_closure
+{
+  sem_t *the_sem;
+  struct inuse_sem *rec;
+};
+
+static void
+walker (const void *inodep, VISIT which, void *closure0)
+{
+  struct walk_closure *closure = closure0;
+  struct inuse_sem *nodep = *(struct inuse_sem **) inodep;
+
+  if (nodep->sem == closure->the_sem)
+    closure->rec = nodep;
+}
+
+bool
+__sem_remove_mapping (sem_t *sem)
+{
+  bool ret = true;
+
+  /* Get the lock.  */
+  lll_lock (sem_mappings_lock, LLL_PRIVATE);
+
+  /* Locate the entry for the mapping the caller provided.  */
+  struct inuse_sem *rec;
+  {
+    struct walk_closure closure = { .the_sem = sem, .rec = NULL };
+    __twalk_r (sem_mappings, walker, &closure);
+    rec = closure.rec;
+  }
+  if (rec != NULL)
+    {
+      /* Check the reference counter.  If it is going to be zero, free
+	 all the resources.  */
+      if (--rec->refcnt == 0)
+	{
+	  /* Remove the record from the tree.  */
+	  __tdelete (rec, &sem_mappings, sem_search);
+
+	  if (munmap (rec->sem, sizeof (sem_t)) == -1)
+	    ret = false;
+
+	  free (rec);
+	}
+    }
+  else
+    ret = false;
+
+  /* Release the lock.  */
+  lll_unlock (sem_mappings_lock, LLL_PRIVATE);
+
+  return ret;
+}
diff --git a/sysdeps/pthread/sem_routines.h b/sysdeps/pthread/sem_routines.h
new file mode 100644
index 0000000..25d3e88
--- /dev/null
+++ b/sysdeps/pthread/sem_routines.h
@@ -0,0 +1,27 @@
+/* Helper code for POSIX semaphore implementation.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SEM_ROUTINES_H
+#define _SEM_ROUTINES_H
+
+sem_t * __sem_check_add_mapping (const char *name, int fd, sem_t *existing)
+  attribute_hidden;
+
+bool __sem_remove_mapping (sem_t *sem) attribute_hidden;
+
+#endif