I was working on a make jobserver implementation that uses POSIX
semaphores as job tokens instead of a complicated socket-based approach.
Initially I used named semaphores, which work fine, except if child
processes with less privileges need to also open the named semaphore
(eg. 'make build' as root executing 'su build -c make'). For that reason
I wanted to use an unnamed semaphore (sem_init()) which is stored in shm
-- that way I could leave the shm fd open and pass it to children.
But unfortunately, sem_t is currently just a pointer to the opaque
struct __sem, and sem_int() always calloc()s the storage for the struct.
This means the application cannot control where unnamed semaphores are
stored, so I can't put it in shm.
I might be missing something here, but I don't see why sem_t is not just
typedef'd to a non-opaque struct. The equivalent structs have
definitions in semaphore.h on FreeBSD and glibc (though glibc doesn't
expose the members in this definition, it seems to be provided just for
sizing). NetBSD uses an opaque struct which is allocated, *except* in
the sem_init() pshared==1 case, where it instead returns a kernel
semaphore identifier cast into a sem_t*; I find that to be somewhat
convoluted.
So, diff below makes struct __sem non-opaque and removes the indirect
allocations, so that the application is required to provide storage and
can therefore control where it's stored (which could be eg. shm).
After bootstrap (installing new header, building and installing the new
library) this survived base and xenocara 'make build' on amd64.
---
include/semaphore.h | 11 +++--
lib/libc/include/thread_private.h | 7 ---
lib/librthread/rthread.h | 4 +-
lib/librthread/rthread_sem.c | 70 +++++++++--------------------
lib/librthread/rthread_sem_compat.c | 68 +++++++++-------------------
lib/librthread/shlib_version | 4 +-
6 files changed, 53 insertions(+), 111 deletions(-)
diff --git a/include/semaphore.h b/include/semaphore.h
index 21dc1a25bed..98471644357 100644
--- a/include/semaphore.h
+++ b/include/semaphore.h
@@ -38,10 +38,15 @@
#define _SEMAPHORE_H_
#include <sys/cdefs.h>
+#include <machine/spinlock.h>
-/* Opaque type definition. */
-struct __sem;
-typedef struct __sem *sem_t;
+struct __sem {
+ _atomic_lock_t lock;
+ volatile int waitcount;
+ volatile int value;
+ int shared;
+};
+typedef struct __sem sem_t;
struct timespec;
#define SEM_FAILED ((sem_t *)0)
diff --git a/lib/libc/include/thread_private.h
b/lib/libc/include/thread_private.h
index 98dfaa63223..f39094e0274 100644
--- a/lib/libc/include/thread_private.h
+++ b/lib/libc/include/thread_private.h
@@ -273,13 +273,6 @@ __END_HIDDEN_DECLS
#define _SPINLOCK_UNLOCKED _ATOMIC_LOCK_UNLOCKED
-struct __sem {
- _atomic_lock_t lock;
- volatile int waitcount;
- volatile int value;
- int shared;
-};
-
TAILQ_HEAD(pthread_queue, pthread);
#ifdef FUTEX
diff --git a/lib/librthread/rthread.h b/lib/librthread/rthread.h
index 1af4509a9a0..89e39c9e0b6 100644
--- a/lib/librthread/rthread.h
+++ b/lib/librthread/rthread.h
@@ -79,8 +79,8 @@ struct pthread_spinlock {
(((size) + (_thread_pagesize - 1)) & ~(_thread_pagesize - 1))
__BEGIN_HIDDEN_DECLS
-int _sem_wait(sem_t, int, const struct timespec *, int *);
-int _sem_post(sem_t);
+int _sem_wait(sem_t *, int, const struct timespec *, int *);
+int _sem_post(sem_t *);
void _rthread_init(void);
struct stack *_rthread_alloc_stack(pthread_t);
diff --git a/lib/librthread/rthread_sem.c b/lib/librthread/rthread_sem.c
index bd96769dc39..984e5fb0047 100644
--- a/lib/librthread/rthread_sem.c
+++ b/lib/librthread/rthread_sem.c
@@ -55,7 +55,7 @@
* Internal implementation of semaphores
*/
int
-_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
+_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
int *delayed_cancel)
{
unsigned int val;
@@ -86,7 +86,7 @@ _sem_wait(sem_t sem, int can_eintr, const struct timespec
*abstime,
/* always increment count */
int
-_sem_post(sem_t sem)
+_sem_post(sem_t *sem)
{
membar_exit_before_atomic();
atomic_inc_int(&sem->value);
@@ -98,10 +98,8 @@ _sem_post(sem_t sem)
* exported semaphores
*/
int
-sem_init(sem_t *semp, int pshared, unsigned int value)
+sem_init(sem_t *sem, int pshared, unsigned int value)
{
- sem_t sem;
-
if (value > SEM_VALUE_MAX) {
errno = EINVAL;
return (-1);
@@ -142,26 +140,19 @@ sem_init(sem_t *semp, int pshared, unsigned int value)
#endif
}
- sem = calloc(1, sizeof(*sem));
- if (!sem) {
- errno = ENOSPC;
- return (-1);
- }
+ bzero(sem, sizeof(*sem));
sem->value = value;
- *semp = sem;
return (0);
}
int
-sem_destroy(sem_t *semp)
+sem_destroy(sem_t *sem)
{
- sem_t sem;
-
if (!_threads_ready) /* for SEM_MMAP_SIZE */
_rthread_init();
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -174,21 +165,18 @@ sem_destroy(sem_t *semp)
return (-1);
}
- *semp = NULL;
if (sem->shared)
munmap(sem, SEM_MMAP_SIZE);
else
- free(sem);
+ bzero(sem, sizeof(*sem));
return (0);
}
int
-sem_getvalue(sem_t *semp, int *sval)
+sem_getvalue(sem_t *sem, int *sval)
{
- sem_t sem;
-
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -199,11 +187,9 @@ sem_getvalue(sem_t *semp, int *sval)
}
int
-sem_post(sem_t *semp)
+sem_post(sem_t *sem)
{
- sem_t sem;
-
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -214,11 +200,10 @@ sem_post(sem_t *semp)
}
int
-sem_wait(sem_t *semp)
+sem_wait(sem_t *sem)
{
struct tib *tib = TIB_GET();
pthread_t self;
- sem_t sem;
int error;
PREP_CANCEL_POINT(tib);
@@ -226,7 +211,7 @@ sem_wait(sem_t *semp)
_rthread_init();
self = tib->tib_thread;
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -246,15 +231,14 @@ sem_wait(sem_t *semp)
}
int
-sem_timedwait(sem_t *semp, const struct timespec *abstime)
+sem_timedwait(sem_t *sem, const struct timespec *abstime)
{
struct tib *tib = TIB_GET();
pthread_t self;
- sem_t sem;
int error;
PREP_CANCEL_POINT(tib);
- if (!semp || !(sem = *semp) || abstime == NULL ||
+ if (!sem || abstime == NULL ||
abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) {
errno = EINVAL;
return (-1);
@@ -279,12 +263,11 @@ sem_timedwait(sem_t *semp, const struct timespec *abstime)
}
int
-sem_trywait(sem_t *semp)
+sem_trywait(sem_t *sem)
{
- sem_t sem;
unsigned int val;
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -316,7 +299,7 @@ sem_open(const char *name, int oflag, ...)
{
char sempath[SEM_PATH_SIZE];
struct stat sb;
- sem_t sem, *semp;
+ sem_t *sem;
unsigned int value = 0;
int created = 0, fd;
@@ -382,34 +365,23 @@ sem_open(const char *name, int oflag, ...)
errno = EINVAL;
return (SEM_FAILED);
}
- semp = malloc(sizeof(*semp));
- if (!semp) {
- munmap(sem, SEM_MMAP_SIZE);
- errno = ENOSPC;
- return (SEM_FAILED);
- }
if (created) {
sem->value = value;
sem->shared = 1;
}
- *semp = sem;
- return (semp);
+ return (sem);
}
int
-sem_close(sem_t *semp)
+sem_close(sem_t *sem)
{
- sem_t sem;
-
- if (!semp || !(sem = *semp) || !sem->shared) {
+ if (!sem || !sem->shared) {
errno = EINVAL;
return (-1);
}
- *semp = NULL;
munmap(sem, SEM_MMAP_SIZE);
- free(semp);
return (0);
}
diff --git a/lib/librthread/rthread_sem_compat.c
b/lib/librthread/rthread_sem_compat.c
index 81309d5f28e..91c888cd49b 100644
--- a/lib/librthread/rthread_sem_compat.c
+++ b/lib/librthread/rthread_sem_compat.c
@@ -53,7 +53,7 @@
* Internal implementation of semaphores
*/
int
-_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
+_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
int *delayed_cancel)
{
void *ident = (void *)&sem->waitcount;
@@ -87,7 +87,7 @@ _sem_wait(sem_t sem, int can_eintr, const struct timespec
*abstime,
/* always increment count */
int
-_sem_post(sem_t sem)
+_sem_post(sem_t *sem)
{
void *ident = (void *)&sem->waitcount;
int rv = 0;
@@ -109,10 +109,8 @@ _sem_post(sem_t sem)
* exported semaphores
*/
int
-sem_init(sem_t *semp, int pshared, unsigned int value)
+sem_init(sem_t *sem, int pshared, unsigned int value)
{
- sem_t sem;
-
if (value > SEM_VALUE_MAX) {
errno = EINVAL;
return (-1);
@@ -153,27 +151,20 @@ sem_init(sem_t *semp, int pshared, unsigned int value)
#endif
}
- sem = calloc(1, sizeof(*sem));
- if (!sem) {
- errno = ENOSPC;
- return (-1);
- }
+ bzero(sem, sizeof(*sem));
sem->lock = _SPINLOCK_UNLOCKED;
sem->value = value;
- *semp = sem;
return (0);
}
int
-sem_destroy(sem_t *semp)
+sem_destroy(sem_t *sem)
{
- sem_t sem;
-
if (!_threads_ready) /* for SEM_MMAP_SIZE */
_rthread_init();
- if (!semp || !(sem = *semp)) {
+ if (!semp) {
errno = EINVAL;
return (-1);
}
@@ -186,21 +177,18 @@ sem_destroy(sem_t *semp)
return (-1);
}
- *semp = NULL;
if (sem->shared)
munmap(sem, SEM_MMAP_SIZE);
else
- free(sem);
+ bzero(sem, sizeof(*sem));
return (0);
}
int
-sem_getvalue(sem_t *semp, int *sval)
+sem_getvalue(sem_t *sem, int *sval)
{
- sem_t sem;
-
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -213,11 +201,9 @@ sem_getvalue(sem_t *semp, int *sval)
}
int
-sem_post(sem_t *semp)
+sem_post(sem_t *sem)
{
- sem_t sem;
-
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -228,11 +214,10 @@ sem_post(sem_t *semp)
}
int
-sem_wait(sem_t *semp)
+sem_wait(sem_t *sem)
{
struct tib *tib = TIB_GET();
pthread_t self;
- sem_t sem;
int r;
PREP_CANCEL_POINT(tib);
@@ -258,15 +243,14 @@ sem_wait(sem_t *semp)
}
int
-sem_timedwait(sem_t *semp, const struct timespec *abstime)
+sem_timedwait(sem_t *sem, const struct timespec *abstime)
{
struct tib *tib = TIB_GET();
pthread_t self;
- sem_t sem;
int r;
PREP_CANCEL_POINT(tib);
- if (!semp || !(sem = *semp) || abstime == NULL ||
+ if (!sem || abstime == NULL ||
abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) {
errno = EINVAL;
return (-1);
@@ -289,12 +273,11 @@ sem_timedwait(sem_t *semp, const struct timespec *abstime)
}
int
-sem_trywait(sem_t *semp)
+sem_trywait(sem_t *sem)
{
- sem_t sem;
int r;
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -330,7 +313,7 @@ sem_open(const char *name, int oflag, ...)
{
char sempath[SEM_PATH_SIZE];
struct stat sb;
- sem_t sem, *semp;
+ sem_t *sem;
unsigned int value = 0;
int created = 0, fd;
@@ -396,35 +379,24 @@ sem_open(const char *name, int oflag, ...)
errno = EINVAL;
return (SEM_FAILED);
}
- semp = malloc(sizeof(*semp));
- if (!semp) {
- munmap(sem, SEM_MMAP_SIZE);
- errno = ENOSPC;
- return (SEM_FAILED);
- }
if (created) {
sem->lock = _SPINLOCK_UNLOCKED;
sem->value = value;
sem->shared = 1;
}
- *semp = sem;
- return (semp);
+ return (sem);
}
int
-sem_close(sem_t *semp)
+sem_close(sem_t *sem)
{
- sem_t sem;
-
- if (!semp || !(sem = *semp) || !sem->shared) {
+ if (!semp || !sem->shared) {
errno = EINVAL;
return (-1);
}
- *semp = NULL;
munmap(sem, SEM_MMAP_SIZE);
- free(semp);
return (0);
}
diff --git a/lib/librthread/shlib_version b/lib/librthread/shlib_version
index 72168dfd16a..54ef0c4cc0c 100644
--- a/lib/librthread/shlib_version
+++ b/lib/librthread/shlib_version
@@ -1,2 +1,2 @@
-major=26
-minor=1
+major=27
+minor=0
--
Lauri Tirkkonen | lotheac @ IRCnet
