Fix a few things. I think I started down the sem inside a sem road because I misunderstood some minor point of the API. Turns out it's entirely possible to just map the semaphore and be done with it. We only need a flag to identify shared semaphores. This makes everything a little bit easier.
In the process of adding einval checking to sem_close, I noticed that the inval checks are all broken because they deref a pointer before checking for null. So fix all of those. Index: rthread.h =================================================================== RCS file: /cvs/src/lib/librthread/rthread.h,v retrieving revision 1.46 diff -u -p -r1.46 rthread.h --- rthread.h 18 Nov 2013 23:10:48 -0000 1.46 +++ rthread.h 20 Nov 2013 19:04:23 -0000 @@ -64,7 +64,7 @@ struct __sem { struct _spinlock lock; volatile int waitcount; volatile int value; - struct __sem *shared; + int shared; }; TAILQ_HEAD(pthread_queue, pthread); Index: rthread_sem.c =================================================================== RCS file: /cvs/src/lib/librthread/rthread_sem.c,v retrieving revision 1.12 diff -u -p -r1.12 rthread_sem.c --- rthread_sem.c 20 Nov 2013 03:16:39 -0000 1.12 +++ rthread_sem.c 20 Nov 2013 19:04:23 -0000 @@ -58,10 +58,8 @@ _sem_wait(sem_t sem, int tryonly, const void *ident = (void *)&sem->waitcount; int r; - if (sem->shared) { - sem = sem->shared; + if (sem->shared) ident = SHARED_IDENT; - } _spinlock(&sem->lock); if (sem->value) { @@ -96,10 +94,8 @@ _sem_post(sem_t sem) void *ident = (void *)&sem->waitcount; int rv = 0; - if (sem->shared) { - sem = sem->shared; + if (sem->shared) ident = SHARED_IDENT; - } _spinlock(&sem->lock); sem->value++; @@ -119,7 +115,12 @@ sem_init(sem_t *semp, int pshared, unsig { sem_t sem, *sempshared; int i, oerrno; - char name[SEM_RANDOM_NAME_LEN]; + char name[SEM_RANDOM_NAME_LEN]; + + if (!semp) { + errno = EINVAL; + return (-1); + } if (pshared) { while (1) { @@ -145,9 +146,9 @@ sem_init(sem_t *semp, int pshared, unsig } sem = *sempshared; - free(sempshared); sem->value = value; *semp = sem; + free(sempshared); return (0); } @@ -173,11 +174,10 @@ sem_destroy(sem_t *semp) { sem_t sem; - if (!semp || !*semp) { + if (!semp || !(sem = *semp)) { errno = EINVAL; return (-1); } - sem = *semp; if (sem->waitcount) { #define MSG "sem_destroy on semaphore with waiters!\n" @@ -187,11 +187,11 @@ sem_destroy(sem_t *semp) return (-1); } - if (sem->shared) - munmap(sem->shared, SEM_MMAP_SIZE); - *semp = NULL; - free(sem); + if (sem->shared) + munmap(sem, SEM_MMAP_SIZE); + else + free(sem); return (0); } @@ -199,16 +199,13 @@ sem_destroy(sem_t *semp) int sem_getvalue(sem_t *semp, int *sval) { - sem_t sem = *semp; + sem_t sem; - if (!semp || !*semp) { + if (!semp || !(sem = *semp)) { errno = EINVAL; return (-1); } - if (sem->shared) - sem = sem->shared; - _spinlock(&sem->lock); *sval = sem->value; _spinunlock(&sem->lock); @@ -219,9 +216,9 @@ sem_getvalue(sem_t *semp, int *sval) int sem_post(sem_t *semp) { - sem_t sem = *semp; + sem_t sem; - if (!semp || !*semp) { + if (!semp || !(sem = *semp)) { errno = EINVAL; return (-1); } @@ -234,11 +231,11 @@ sem_post(sem_t *semp) int sem_wait(sem_t *semp) { - sem_t sem = *semp; pthread_t self = pthread_self(); + sem_t sem; int r; - if (!semp || !*semp) { + if (!semp || !(sem = *semp)) { errno = EINVAL; return (-1); } @@ -258,11 +255,11 @@ sem_wait(sem_t *semp) int sem_timedwait(sem_t *semp, const struct timespec *abstime) { - sem_t sem = *semp; pthread_t self = pthread_self(); + sem_t sem; int r; - if (!semp || !*semp) { + if (!semp || !(sem = *semp)) { errno = EINVAL; return (-1); } @@ -282,10 +279,10 @@ sem_timedwait(sem_t *semp, const struct int sem_trywait(sem_t *semp) { - sem_t sem = *semp; + sem_t sem; int r; - if (!semp || !*semp) { + if (!semp || !(sem = *semp)) { errno = EINVAL; return (-1); } @@ -316,7 +313,7 @@ sem_open(const char *name, int oflag, .. char sempath[SEM_PATH_SIZE]; struct stat sb; int fd, oerrno; - sem_t sem, shared; + sem_t sem; sem_t *semp = SEM_FAILED; if (oflag & ~(O_CREAT | O_EXCL)) { @@ -338,7 +335,17 @@ sem_open(const char *name, int oflag, .. errno = EPERM; return (semp); } - if (sb.st_size < SEM_MMAP_SIZE) { + if (sb.st_size != SEM_MMAP_SIZE) { + if (!(oflag & O_CREAT)) { + close(fd); + errno = EINVAL; + return (semp); + } + if (sb.st_size != 0) { + close(fd); + errno = EINVAL; + return (semp); + } if (ftruncate(fd, SEM_MMAP_SIZE) == -1) { oerrno = errno; close(fd); @@ -347,25 +354,23 @@ sem_open(const char *name, int oflag, .. return (semp); } } - shared = mmap(NULL, SEM_MMAP_SIZE, PROT_READ | PROT_WRITE, + sem = mmap(NULL, SEM_MMAP_SIZE, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED | MAP_HASSEMAPHORE, fd, 0); oerrno = errno; close(fd); - if (shared == MAP_FAILED) { + if (sem == MAP_FAILED) { errno = oerrno; return (semp); } + sem->shared = 1; semp = malloc(sizeof(*semp)); - sem = calloc(1, sizeof(*sem)); - if (!semp || !sem) { - free(sem); + if (!semp) { free(semp); - munmap(shared, SEM_MMAP_SIZE); + munmap(sem, SEM_MMAP_SIZE); errno = ENOSPC; return (SEM_FAILED); } *semp = sem; - sem->shared = shared; return (semp); } @@ -373,10 +378,14 @@ sem_open(const char *name, int oflag, .. int sem_close(sem_t *semp) { - sem_t sem = *semp; + sem_t sem; + + if (!semp || !(sem = *semp) || !sem->shared) { + errno = EINVAL; + return (-1); + } - munmap(sem->shared, SEM_MMAP_SIZE); - free(sem); + munmap(sem, SEM_MMAP_SIZE); free(semp); return (0); @@ -390,4 +399,3 @@ sem_unlink(const char *name) makesempath(name, sempath, sizeof(sempath)); return (unlink(sempath)); } -