Re: POSIX semaphore for GNU/Hurd

[James Morrison]

--- Ga�l Le Mignot <kilobug@freesurf.fr> wrote:
> 
> Hello,
> 
> While trying to compile abiword, and so gnome-vfs, on GNU/Hurd, I've
> seen that it uses POSIX semaphore, that we don't implement yet.>
 
> So, I've done an implementation of POSIX semaphores using a pthread_mutex
> and a pthread_cond. I didn't test it yet, but it compiles, and I'm pretty
> confident in it (but I'm sure of the correct behavior of the destroy
> function if some threads are locked on the semaphore).

 Thanks for this work, did you compile it with -Wall?

> 
> Anyway, here it is. I think it should be merged in libpthreads as soon
> as someone reviewed it.
> 
> > /* Copyright (C) 2000,02 Free Software Foundation, Inc.
>    This file is part of the GNU C Library.
>    Written by Ga�l Le Mignot <kilobug@hurdfr.org>
> 
>    The GNU C Library is free software; you can redistribute it and/or
>    modify it under the terms of the GNU Library General Public License as
>    published by the Free Software Foundation; either version 2 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
>    Library General Public License for more details.
> 
>    You should have received a copy of the GNU Library General Public
>    License along with the GNU C Library; see the file COPYING.LIB.  If not,
>    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>    Boston, MA 02111-1307, USA.  */
> 
> /*
>  *    POSIX Threads Extension: Semaphores                     <semaphore.c>
>  */
> 
> #include <semaphore.h>
> #include <stdlib.h>
> 
> /* Initialize the semaphore and set the initial value - as in LinuxThreads
>    pshared must be zero right now. */
> int 
> sem_init (sem_t *sem, int pshared, unsigned int value)
> {
>   if (pshared)
>     return -1;

 If you wanted to be consistant with LinuxThreads this should be
if (pshared)
  {
     errno = ENOSYS;
     return -1;
  }
However, I think it would be better to actually implement this feature.

>   
>   sem = malloc (sizeof (*sem));
>   if (sem == NULL)
>     return -1;
>   
>   sem->count = value;
>   pthread_cond_init (&sem->lock, NULL);
>   pthread_mutex_init (&sem->count_lock, NULL);
>   return 0;
> }
> 
> 
> /* Destroys the semaphore */
> int 
> sem_destroy (sem_t *sem)
> {
>   pthread_mutex_destroy (&sem->count_lock);
>   pthread_cond_destroy (&sem->lock);
>   free (sem);
>   return 0;
> }
> 
> /* 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);

  Why is there an unlock here?  while (1) is an infinite loop so this line
should not be reached.  Although,

while (!(sem->count))
  pthread_cond_wait (&sem->lock, &sem->count_lock);

sem->count--;
pthread_mutex_unlock (&sem->count_lock);
return 0;

 would be shorter.  The real question is, are either of these implementations
cancellable?  As far as I can tell both are because pthread_cond_wait is
cancellable.

> }
> 
> /* 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;
> }
> 
> /* Increments the count */
> int 
> sem_post (sem_t *sem)
> {
>   pthread_mutex_lock (&sem->count_lock);

 To be paranoid, this should also contain
 if (sem->count == SEM_VALUE_MAX)
   {
     pthread_mutex_unlock (&sem->count_lock);
     return -1;
   }

>   sem->count++;
>   pthread_cond_signal (&sem->lock);
>   pthread_mutex_unlock (&sem->count_lock);
>   return 0;
> }
> 
> /* Return the value of the semaphore */
> int 
> sem_getvalue (sem_t *sem, int *sval)
> {
>   pthread_mutex_lock (&sem->count_lock);
>   *sval = sem->count;
>   pthread_mutex_unlock (&sem->count_lock);
>   return 0;
> }
> 
> > /* Copyright (C) 2000,02 Free Software Foundation, Inc.
>    This file is part of the GNU C Library.
>    Written by Ga�l Le Mignot <kilobug@hurdfr.org>
> 
>    The GNU C Library is free software; you can redistribute it and/or
>    modify it under the terms of the GNU Library General Public License as
>    published by the Free Software Foundation; either version 2 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
>    Library General Public License for more details.
> 
>    You should have received a copy of the GNU Library General Public
>    License along with the GNU C Library; see the file COPYING.LIB.  If not,
>    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>    Boston, MA 02111-1307, USA.  */
> 
> /*
>  *    POSIX Threads Extension: Semaphores                     <semaphore.h>
>  */
> 
> #ifndef _SEMAPHORE_H
> #define _SEMAPHORE_H  1
> 
> #include <pthread.h>
> 
> __BEGIN_DECLS
> 
> struct __sem_t 
> {
>   unsigned int count;
>   pthread_mutex_t count_lock;
>   pthread_cond_t lock;
> };

 Somewhere in here
#define SEM_VALUE_MAX UINT_MAX 
should probably occur.

> 
> typedef struct __sem_t sem_t;
> 
> /* Initialize the semaphore and set the initial value - as in LinuxThreads
>    pshared must be zero right now. */

 How about for a comment:
/* Initialize the semaphore and set the initial value.  PSHARED must be set
to 0 processes sharing semaphores is not implemented yet. */

> extern int sem_init (sem_t *sem, int pshared, unsigned int value);
> 
> /* Destroys the semaphore */
> extern int sem_destroy (sem_t *sem);
> 
> /* Wait until the count is > 0, and then decrease it  */
> extern int sem_wait (sem_t *sem);
> 
> /* Non-blocking variant of sem_wait. Returns -1 if count == 0. */
> extern int sem_trywait (sem_t *sem);
> 
> /* Increments the count */
> extern int sem_post (sem_t *sem);
> 
> /* Return the value of the semaphore */
> extern int sem_getvalue (sem_t *sem, int *sval);
> 
> __END_DECLS
> 
> #endif /* pthread.h */
> > 
> 
> -- 
> Gael Le Mignot "Kilobug" - kilobug@freesurf.fr - http://kilobug.free.fr
> GSM         : 06.71.47.18.22 (in France)   ICQ UIN   : 7299959
> Fingerprint : 1F2C 9804 7505 79DF 95E6 7323 B66B F67B 7103 C5DA
> 
> Member of HurdFr: http://hurdfr.org - The GNU Hurd: http://hurd.gnu.org
> 


=====
James Morrison
   University of Waterloo
   Computer Science - Digital Hardware
   2A co-op
http://hurd.dyndns.org

Anyone referring to this as 'Open Source' shall be eaten by a GNU

__________________________________________________
Do you Yahoo!?
Y! Web Hosting - Let the expert host your web site
http://webhosting.yahoo.com/