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));
}
-
