> int
> sem_init (sem_t *sem, int pshared, unsigned int value)
> {
> if (pshared)
> return -1;
Set errno to ENOTSUP.
> sem = malloc (sizeof (*sem));
> if (sem == NULL)
> return -1;
This is not going to work at all. SEM is a local variable; If you are
going to continue with this method, then you need to set *SEM to the
allocated buffer, not SEM.
> sem->count = value;
> pthread_cond_init (&sem->lock, NULL);
> pthread_mutex_init (&sem->count_lock, NULL);
> return 0;
I would rather have part of the sem_t structure exported (à la
pthread_mutex_t). The structure should contain an id and a union: the
former so that we can distinguish between semaphores created with
sem_init and those with sem_open; and the latter so that sem_open and
sem_close can store the file descriptor and other relevant
information.
> /* Wait until the count is > 0, and then decrease it */
> int
> sem_wait (sem_t *sem)
> {
> pthread_mutex_lock (&sem->count_lock);
> while (1)
> {
> if (sem->count)
> {
> sem->count--;
> pthread_mutex_unlock (&sem->count_lock);
> return 0;
> }
> pthread_cond_wait (&sem->lock, &sem->count_lock);
> }
> pthread_mutex_unlock (&sem->count_lock);
This unlock is never reached.
An alternative implementation and arguably faster would use atomic
integers and __atomic_dec (actually our current implementation of
__atomic_dec would need to be modified to return the resulting value).
If the result is less than zero, the thread waits. The post code
would do an atomic increment and if the result if less than or equal
to zero, a waiter would be signaled.
Also, there is no need to use condition variables: just use
__pthread_block and __pthread_wakeup: this completely avoids the mutex
and the overhead of the condition variable.
> /* Non-blocking variant of sem_wait. Returns -1 if count == 0. */
> int
> sem_trywait (sem_t *sem)
> {
> int res = -1;
>
> pthread_mutex_lock (&sem->count_lock);
> if (sem->count)
> {
> res = 0;
> sem->count--;
> }
> pthread_mutex_unlock (&sem->count_lock);
>
> return res;
> }
You need to set errno to EAGAIN on failure.
>
> /* Increments the count */
> int
> sem_post (sem_t *sem)
> {
> pthread_mutex_lock (&sem->count_lock);
> sem->count++;
> pthread_cond_signal (&sem->lock);
> pthread_mutex_unlock (&sem->count_lock);
> return 0;
> }
You only need to signal the condition if the count is now 1.
You have not included a semaphore.h file. All of these functions
should be in different files just like the rest of libpthread. It
should be easy to implement sem_timedwait.
You may want to consider implementing sem_open and sem_close in terms
of file locking and file change notifications, however, I will accept
patches that do not support this as long as there are ENOSYS stubs for
sem_open and sem_close.
Thanks, this is a good start.
_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-hurd
