Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API

From: Nicolas Pitre <nico@fluxnic.net>
Date: Nov 5, 2009, 02:10 UTC
Message-ID: <alpine.LFD.2.00.0911042039200.10340@xanadu.home>
In-Reply-To: <4AF214D5.6050202@gmail.com>

On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:

Show 11 quoted lines

> +static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)
> +{
> +	cond->waiters = 0;
> +
> +	InitializeCriticalSection(&cond->waiters_lock);
> +
> +	cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
> +	if (!cond->sema)
> +		return 0; /* POSIX do not allow pthread_cond_init to fail */
> +	return 0;
> +}

Please use die("CreateSemaphore() failed") in the failure case instead of returning success.

However, my pthread_cond_init man page says:

[[[
RETURN VALUE
       If successful, the pthread_cond_destroy() and pthread_cond_init() func-
       tions  shall  return zero; otherwise, an error number shall be returned
       to indicate the error.

       The [EBUSY] and [EINVAL] error checks, if implemented, shall act as  if
       they  were performed immediately at the beginning of processing for the
       function and caused an error return prior to modifying the state of the
       condition variable specified by cond.

ERRORS
       The pthread_cond_destroy() function may fail if:

       EBUSY  The implementation has detected an attempt to destroy the object
              referenced by cond while it is referenced  (for  example,  while
              being used in a pthread_cond_wait() or pthread_cond_timedwait())
              by another thread.

       EINVAL The value specified by cond is invalid.

       The pthread_cond_init() function shall fail if:

       EAGAIN The system lacked the necessary resources (other than memory) to
              initialize another condition variable.

       ENOMEM Insufficient memory exists to initialize the condition variable.

       The pthread_cond_init() function may fail if:

       EBUSY  The implementation has detected an attempt to  reinitialize  the
              object referenced by cond, a previously initialized, but not yet
              destroyed, condition variable.

       EINVAL The value specified by attr is invalid.
]]]

I'm not advocating that you implement detailed error codes as we don't really care about specific errors. This is just to disagree with the "POSIX do not allow pthread_cond_init to fail" assertion. In any case, using die() to keep it simple is certainly better than blindly returning 0 on failure. However you could simply return ENOMEM and use the die() in init_threaded_search() instead.

Nicolas