* sem_timedwait.c (sem_timedwait): Tighten checks for

  unreasonable abstime values - that would result in
  unexpected timeout values.

* w32_CancelableWait.c (ptw32_cancelable_wait):
  Tighten up return value checking and add comments.

3 files changed, 150 insertions, 116 deletions

diff --git a/ChangeLog b/ChangeLog
index 8a36181..10eb3ca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2002-07-30  Ross Johnson  <rpj@ise.canberra.edu.au>

+

+	* sem_timedwait.c (sem_timedwait): Tighten checks for

+	unreasonable abstime values - that would result in

+	unexpected timeout values.

+

+	* w32_CancelableWait.c (ptw32_cancelable_wait):

+	Tighten up return value checking and add comments.

+

 2002-06-04  Rob Fanner  <rfanner@stonethree.com>

 

 	* sem_getvalue.c (sem_getvalue): The Johnson M. Hart

@@ -7,7 +16,7 @@
 	value. There is a small probability that this may

 	block other threads, but only momentarily.

 

-2002-06-03  Ross Johnson  <rpj@digit.ise.canberra.edu.au>

+2002-06-03  Ross Johnson  <rpj@ise.canberra.edu.au>

 

 	* sem_init.c (sem_init): Initialise Win32 semaphores

 	to _POSIX_SEM_VALUE_MAX (which this implementation

diff --git a/sem_timedwait.c b/sem_timedwait.c
index 11178f2..a6fc753 100644
--- a/sem_timedwait.c
+++ b/sem_timedwait.c
@@ -116,63 +116,95 @@ sem_timedwait (sem_t * sem, const struct timespec * abstime)
   else
     {
       if (abstime == NULL)
-	{
-	  milliseconds = INFINITE;
-	}
+				{
+					milliseconds = INFINITE;
+				}
       else
-	{
-	  /* 
-	   * Calculate timeout as milliseconds from current system time. 
-	   */
-
-	  /* get current system time */
-
+				{
+					milliseconds = 0;
+
+					/* 
+					 * Calculate timeout as milliseconds from current system time. 
+					 */
+					
+					/* get current system time */
+					
 #ifdef NEED_FTIME
 
-	  {
-	    FILETIME ft;
-	    SYSTEMTIME st;
-
-	    GetSystemTime(&st);
-	    SystemTimeToFileTime(&st, &ft);
-	    /*
-	     * GetSystemTimeAsFileTime(&ft); would be faster,
-	     * but it does not exist on WinCE
-	     */
-
-	    ptw32_filetime_to_timespec(&ft, &currSysTime);
-	  }
-
-	  /*
-	   * subtract current system time from abstime
-	   */
-	  milliseconds = (abstime->tv_sec - currSysTime.tv_sec) * MILLISEC_PER_SEC;
-	  milliseconds += ((abstime->tv_nsec - currSysTime.tv_nsec)
-                           + (NANOSEC_PER_MILLISEC/2)) / NANOSEC_PER_MILLISEC;
-
+					{
+						FILETIME ft;
+						SYSTEMTIME st;
+
+						GetSystemTime(&st);
+						SystemTimeToFileTime(&st, &ft);
+						/*
+						 * GetSystemTimeAsFileTime(&ft); would be faster,
+						 * but it does not exist on WinCE
+						 */
+						
+						ptw32_filetime_to_timespec(&ft, &currSysTime);
+					}
+					
+					/*
+					 * subtract current system time from abstime in a way that checks
+					 * that abstime is never in the past, or is never equivalent to the
+					 * defined INFINITE value (0xFFFFFFFF).
+					 */
+					if (abstime->tv_sec >= currSysTime.tv_sec)
+						{
+							DWORD tmpMilliseconds;
+							DWORD tmpCurrMilliseconds;
+							
+							tmpMilliseconds = (abstime->tv_sec - currSysTime.tv_sec) * MILLISEC_PER_SEC;
+							tmpMilliseconds += ((abstime->tv_nsec + (NANOSEC_PER_MILLISEC/2))
+								/ NANOSEC_PER_MILLISEC);
+							tmpCurrMilliseconds = ((currSysTime.tv_nsec + (NANOSEC_PER_MILLISEC/2))
+								/ NANOSEC_PER_MILLISEC);
+							if (tmpMilliseconds > tmpCurrMilliseconds)
+								{
+									milliseconds = tmpMilliseconds - tmpCurrMilliseconds;
+									if (milliseconds == INFINITE)
+										{
+											milliseconds--;
+										}
+								}
+						}
+					
 #else /* NEED_FTIME */
-	  _ftime(&currSysTime);
-
-	  /*
-	   * subtract current system time from abstime
-	   */
-	  milliseconds = (abstime->tv_sec - currSysTime.time) * MILLISEC_PER_SEC;
-	  milliseconds += ((abstime->tv_nsec + (NANOSEC_PER_MILLISEC/2)) / NANOSEC_PER_MILLISEC)
-            - currSysTime.millitm;
-
+					_ftime(&currSysTime);
+					
+					/*
+					 * subtract current system time from abstime in a way that checks
+					 * that abstime is never in the past, or is never equivalent to the
+					 * defined INFINITE value (0xFFFFFFFF).
+					 */
+					if (abstime->tv_sec >= currSysTime.time)
+						{
+							DWORD tmpMilliseconds;
+							
+							tmpMilliseconds = (abstime->tv_sec - currSysTime.time) * MILLISEC_PER_SEC;
+							tmpMilliseconds += ((abstime->tv_nsec + (NANOSEC_PER_MILLISEC/2))
+								/ NANOSEC_PER_MILLISEC);
+							if (tmpMilliseconds > currSysTime.millitm)
+								{
+									milliseconds = tmpMilliseconds - currSysTime.millitm;
+									if (milliseconds == INFINITE)
+										{
+											milliseconds--;
+										}
+								}
+						}
+					
 #endif /* NEED_FTIME */
-
-
-	  if (((int) milliseconds) < 0)
-	    milliseconds = 0;
-	}
-
+					
+				}
+			
 #ifdef NEED_SEM
-
+			
       result = (pthreadCancelableTimedWait ((*sem)->event, milliseconds));
-
+			
 #else /* NEED_SEM */
-
+			
       result = (pthreadCancelableTimedWait ((*sem)->sem, milliseconds));
 
 #endif
diff --git a/w32_CancelableWait.c b/w32_CancelableWait.c
index ba1c92a..2f84743 100644
--- a/w32_CancelableWait.c
+++ b/w32_CancelableWait.c
@@ -68,13 +68,13 @@ ptw32_cancelable_wait (HANDLE waitHandle, DWORD timeout)
        * Get cancelEvent handle
        */
       if (self->cancelState == PTHREAD_CANCEL_ENABLE)
-	{
+				{
 
-	  if ((handles[1] = self->cancelEvent) != NULL)
-	    {
-	      nHandles++;
-	    }
-	}
+					if ((handles[1] = self->cancelEvent) != NULL)
+						{
+							nHandles++;
+						}
+				}
     }
   else
     {
@@ -88,68 +88,61 @@ ptw32_cancelable_wait (HANDLE waitHandle, DWORD timeout)
 				    timeout);
 
 
-  if (status == WAIT_FAILED)
-    {
-      result = EINVAL;
-    }
-  else if (status == WAIT_TIMEOUT)
-    {
-      result = ETIMEDOUT;
-    }
-  else if (status == WAIT_ABANDONED_0)
-    {
-      result = EINVAL;
-    }
-  else
-    {
-      /*
-       * Either got the handle or the cancel event
-       * was signaled
-       */
-      switch (status - WAIT_OBJECT_0)
-	{
-
-	case 0:
-	  /*
-	   * Got the handle
-	   */
-	  result = 0;
-	  break;
-
-	case 1:
-	  /*
-	   * Got cancel request
-	   */
-	  ResetEvent (handles[1]);
-
-	  if (self != NULL && !self->implicit)
-	    {
-	      /*
-	       * Thread started with pthread_create.
-	       * Make sure we haven't been async-canceled in the meantime.
-	       */
-	      (void) pthread_mutex_lock(&self->cancelLock);
-	      if (self->state < PThreadStateCanceling)
+	switch (status - WAIT_OBJECT_0)
 		{
-		  self->state = PThreadStateCanceling;
-		  self->cancelState = PTHREAD_CANCEL_DISABLE;
-		  (void) pthread_mutex_unlock(&self->cancelLock);
-		  ptw32_throw(PTW32_EPS_CANCEL);
-
-		  /* Never reached */
+			case 0:
+				/*
+				 * Got the handle.
+				 * In the event that both handles are signalled, the smallest index
+				 * value (us) is returned. As it has been arranged, this ensures that
+				 * we don't drop a signal that we should act on (i.e. semaphore,
+				 * mutex, or condition variable etc).
+				 */
+				result = 0;
+				break;
+
+			case 1:
+				/*
+				 * Got cancel request.
+				 * In the event that both handles are signalled, the cancel will
+				 * be ignored (see case 0 comment).
+				 */
+				ResetEvent (handles[1]);
+
+				if (self != NULL && !self->implicit)
+					{
+						/*
+						 * Thread started with pthread_create.
+						 * Make sure we haven't been async-canceled in the meantime.
+						 */
+						(void) pthread_mutex_lock(&self->cancelLock);
+						if (self->state < PThreadStateCanceling)
+							{
+								self->state = PThreadStateCanceling;
+								self->cancelState = PTHREAD_CANCEL_DISABLE;
+								(void) pthread_mutex_unlock(&self->cancelLock);
+								ptw32_throw(PTW32_EPS_CANCEL);
+										
+								/* Never reached */
+							}
+						(void) pthread_mutex_unlock(&self->cancelLock);
+					}
+						
+				/* Should never get to here. */
+				result = EINVAL;
+				break;
+
+			default:
+				if (status == WAIT_TIMEOUT)
+					{
+						result = ETIMEDOUT;
+					}
+				else
+					{
+						result = EINVAL;
+					}
+				break;
 		}
-	      (void) pthread_mutex_unlock(&self->cancelLock);
-	    }
-
-	  /* Should never get to here. */
-	  result = EINVAL;
-	  break;
-
-	default:
-	  result = EINVAL;
-	  break;
-	}
-    }
 
   return (result);