Re: Patch to mutex.c

[Kaz Kylheku <kaz at ashi dot footprints dot net>]

On Wed, 27 Sep 2000 Neale.Ferguson@softwareAG-usa.com wrote:

> Date: Wed, 27 Sep 2000 11:59:54 +0200
> From: Neale.Ferguson@softwareAG-usa.com
> To: libc-alpha@sources.redhat.com
> Subject: Re: Patch to mutex.c
> 
> Sorry about raising this again, but does the comment below mean this patch will
> be included? I've included it on my system and running some POSIX conformance
> tests which it is now passing.
> 
> --- mutex.c     2000/09/20 14:35:30     1.1
> +++ mutex.c     2000/09/21 17:46:11
> @@ -165,15 +165,20 @@
>    case PTHREAD_MUTEX_RECURSIVE_NP:
>      if (mutex->__m_count > 0) {
>        mutex->__m_count--;
>        return 0;
>      }
> -    mutex->__m_owner = NULL;
> -    __pthread_unlock(&mutex->__m_lock);
> -    return 0;
> +    if (mutex->__m_owner != NULL) {
> +      mutex->__m_owner = NULL;
> +      __pthread_unlock(&mutex->__m_lock);
> +      return 0;
> +    }
> +    else
> +      return EPERM;
> +

This patch isn't quite right because EPERM must be returned if the mutex is not
owned by the calling thread, not just when it is not owned.  The above
will not detect the case when the mutex is owned by another thread.
You need the following test:

	if (mutex->__m_owner != thread_self())
	    return EPERM;

This test should be done first, before doing anything which may change the
state of the mutex. You don't want to be decrementing the lock count if
you're not the owner.

Inserting the above test as the first statement of the case is a complete
solution to the non-compliance.