[ros-diffs] [ion] 53056: [KERNEL32]: Cleanup and fix bugs in the TimerQueue implementation... mainly related to wrong/incorrect parameter checks and error codes.

Author: ion
Date: Thu Aug  4 00:54:00 2011
New Revision: 53056

URL: http://svn.reactos.org/svn/reactos?rev=53056&view=rev
Log:
[KERNEL32]: Cleanup and fix bugs in the TimerQueue implementation... mainly related to
wrong/incorrect parameter checks and error codes.

Modified:
    trunk/reactos/dll/win32/kernel32/client/timerqueue.c

Modified: trunk/reactos/dll/win32/kernel32/client/timerqueue.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/…
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/timerqueue.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/timerqueue.c [iso-8859-1] Thu Aug  4 00:54:00
2011
@@ -1,132 +1,112 @@
-/* $Id$
- *
- * COPYRIGHT:       See COPYING in the top level directory
- * PROJECT:         ReactOS system libraries
- * PURPOSE:         Timer Queue functions
- * FILE:            dll/win32/kernel32/misc/timerqueue.c
- * PROGRAMER:       Thomas Weidenmueller <w3seek(a)reactos.com>
- */
-
-/* INCLUDES ******************************************************************/
+/*
+ * PROJECT:         ReactOS Win32 Base API
+ * LICENSE:         See COPYING in the top level directory
+ * FILE:            dll/win32/kernel32/client/timerqueue.c
+ * PURPOSE:         Timer Queue Functions
+ * PROGRAMMERS:     Thomas Weidenmueller <w3seek(a)reactos.com>
+ */
+
+/* INCLUDES *******************************************************************/
 
 #include <k32.h>
 
 #define NDEBUG
 #include <debug.h>
 
-
-/* FUNCTIONS *****************************************************************/
-
-HANDLE DefaultTimerQueue = NULL;
-
-/*
- * Create the default timer queue for the current process. This function is only
- * called if CreateTimerQueueTimer() or SetTimerQueueTimer() is called.
- * However, ChangeTimerQueueTimer() fails with ERROR_INVALID_PARAMETER if the
- * default timer queue has not been created, because it assumes there has to be
- * a timer queue with a timer if it want's to be changed.
- */
-static BOOL
-IntCreateDefaultTimerQueue(VOID)
-{
-  NTSTATUS Status;
-
-  /* FIXME - make this thread safe */
-
-  /* create the timer queue */
-  Status = RtlCreateTimerQueue(&DefaultTimerQueue);
-  if(!NT_SUCCESS(Status))
-  {
-    BaseSetLastNTError(Status);
-    DPRINT1("Unable to create the default timer queue!\n");
-    return FALSE;
-  }
-
-  return TRUE;
-}
-
-/*
- * @implemented
- */
-BOOL
-WINAPI
-CancelTimerQueueTimer(HANDLE TimerQueue,
-                      HANDLE Timer)
-{
-  /* Since this function is not documented in PSDK and apparently does nothing
-     but delete the timer, we just do the same as DeleteTimerQueueTimer(), without
-     passing a completion event. */
-  NTSTATUS Status;
-
-  if(TimerQueue == NULL)
-  {
-    /* let's use the process' default timer queue. We assume the default timer
-       queue has been created with a previous call to CreateTimerQueueTimer() or
-       SetTimerQueueTimer(), otherwise this call wouldn't make much sense. */
-    if(!(TimerQueue = DefaultTimerQueue))
-    {
-      SetLastError(ERROR_INVALID_HANDLE);
-      return FALSE;
-    }
-  }
-
-  if(Timer == NULL)
-  {
-    SetLastError(ERROR_INVALID_PARAMETER);
-    return FALSE;
-  }
-
-  /* delete the timer */
-  Status = RtlDeleteTimer(TimerQueue, Timer, NULL);
-
-  if(!NT_SUCCESS(Status))
-  {
-    BaseSetLastNTError(Status);
-    return FALSE;
-  }
-
-  return TRUE;
-}
-
-/*
- * @implemented
- */
-BOOL
-WINAPI
-ChangeTimerQueueTimer(HANDLE TimerQueue,
-                      HANDLE Timer,
-                      ULONG DueTime,
-                      ULONG Period)
-{
-  NTSTATUS Status;
-
-  if(TimerQueue == NULL)
-  {
-    /* let's use the process' default timer queue. We assume the default timer
-       queue has been created with a previous call to CreateTimerQueueTimer() or
-       SetTimerQueueTimer(), otherwise this call wouldn't make much sense. */
-    if(!(TimerQueue = DefaultTimerQueue))
-    {
-      SetLastError(ERROR_INVALID_HANDLE);
-      return FALSE;
-    }
-  }
-
-  if(Timer == NULL)
-  {
-    SetLastError(ERROR_INVALID_PARAMETER);
-    return FALSE;
-  }
-
-  /* update the timer */
-  Status = RtlUpdateTimer(TimerQueue, Timer, DueTime, Period);
-  if(!NT_SUCCESS(Status))
-  {
-    BaseSetLastNTError(Status);
-    return FALSE;
-  }
-
-  return TRUE;
+/* GLOBALS ********************************************************************/
+
+HANDLE BasepDefaultTimerQueue = NULL;
+
+/* PRIVATE FUNCTIONS **********************************************************/
+
+/* FIXME - make this thread safe */
+BOOL
+WINAPI
+BasepCreateDefaultTimerQueue(VOID)
+{
+    NTSTATUS Status;
+
+    /* Create the timer queue */
+    Status = RtlCreateTimerQueue(&BasepDefaultTimerQueue);
+    if (!NT_SUCCESS(Status))
+    {
+        BaseSetLastNTError(Status);
+        DPRINT1("Unable to create the default timer queue!\n");
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+/* PUBLIC FUNCTIONS ***********************************************************/
+
+
+/*
+ * @implemented
+ */
+BOOL
+WINAPI
+CancelTimerQueueTimer(IN HANDLE TimerQueue,
+                      IN HANDLE Timer)
+{
+    NTSTATUS Status;
+
+    if (!TimerQueue)
+    {
+        /* Use the default timer queue */
+        TimerQueue = BasepDefaultTimerQueue;
+        if (!TimerQueue)
+        {
+            /* A timer is being cancelled before one was created... fail */
+            SetLastError(ERROR_INVALID_HANDLE);
+            return FALSE;
+        }
+    }
+
+    /* Delete the timer */
+    Status = RtlDeleteTimer(TimerQueue, Timer, NULL);
+    if (!NT_SUCCESS(Status))
+    {
+        BaseSetLastNTError(Status);
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+/*
+ * @implemented
+ */
+BOOL
+WINAPI
+ChangeTimerQueueTimer(IN HANDLE TimerQueue,
+                      IN HANDLE Timer,
+                      IN ULONG DueTime,
+                      IN ULONG Period)
+{
+    NTSTATUS Status;
+
+    if (!TimerQueue)
+    {
+        /* Use the default timer queue */
+        TimerQueue = BasepDefaultTimerQueue;
+        if (!TimerQueue)
+        {
+            /* A timer is being cancelled before one was created... fail */
+            SetLastError(ERROR_INVALID_PARAMETER);
+            return FALSE;
+        }
+    }
+
+    /* Delete the timer */
+    Status = RtlUpdateTimer(TimerQueue, Timer, DueTime, Period);
+    if (!NT_SUCCESS(Status))
+    {
+        BaseSetLastNTError(Status);
+        return FALSE;
+    }
+
+    return TRUE;
 }
 
 /*
@@ -136,173 +116,161 @@
 WINAPI
 CreateTimerQueue(VOID)
 {
-  HANDLE Handle;
-  NTSTATUS Status;
-
-  /* create the timer queue */
-  Status = RtlCreateTimerQueue(&Handle);
-  if(!NT_SUCCESS(Status))
-  {
-    BaseSetLastNTError(Status);
-    return NULL;
-  }
-
-  return Handle;
-}
-
-/*
- * @implemented
- */
-BOOL
-WINAPI
-CreateTimerQueueTimer(PHANDLE phNewTimer,
-                      HANDLE TimerQueue,
-                      WAITORTIMERCALLBACK Callback,
-                      PVOID Parameter,
-                      DWORD DueTime,
-                      DWORD Period,
-                      ULONG Flags)
-{
-  NTSTATUS Status;
-
-  /* windows seems not to test this parameter at all, so we'll try to clear it here
-     so we don't crash somewhere inside ntdll */
-  *phNewTimer = NULL;
-
-  if(TimerQueue == NULL)
-  {
-    /* the default timer queue is requested, try to create it if it hasn't been
already */
-    if(!(TimerQueue = DefaultTimerQueue))
-    {
-      if(!IntCreateDefaultTimerQueue())
-      {
-        /* IntCreateDefaultTimerQueue() set the last error code already, just fail */
-        return FALSE;
-      }
-      TimerQueue = DefaultTimerQueue;
-    }
-  }
-
-  /* !!! Win doesn't even check if Callback == NULL, so we don't, too!
That'll
-         raise a nice exception later... */
-
-  /* create the timer */
-  Status = RtlCreateTimer(TimerQueue, phNewTimer, Callback, Parameter, DueTime,
-                          Period, Flags);
-  if(!NT_SUCCESS(Status))
-  {
-    BaseSetLastNTError(Status);
-    return FALSE;
-  }
-
-  return TRUE;
-}
-
-/*
- * @implemented
- */
-BOOL
-WINAPI
-DeleteTimerQueue(HANDLE TimerQueue)
-{
-  NTSTATUS Status;
-
-  /* We don't allow the user to delete the default timer queue */
-  if(TimerQueue == NULL)
-  {
-    SetLastError(ERROR_INVALID_HANDLE);
-    return FALSE;
-  }
-
-  /* delete the timer queue */
-  Status = RtlDeleteTimerQueue(TimerQueue);
-  return NT_SUCCESS(Status);
-}
-
-/*
- * @implemented
- */
-BOOL
-WINAPI
-DeleteTimerQueueEx(HANDLE TimerQueue,
-                   HANDLE CompletionEvent)
-{
-  NTSTATUS Status;
-
-  /* We don't allow the user to delete the default timer queue */
-  if(TimerQueue == NULL)
-  {
-    SetLastError(ERROR_INVALID_HANDLE);
-    return FALSE;
-  }
-
-  /* delete the queue */
-  Status = RtlDeleteTimerQueueEx(TimerQueue, CompletionEvent);
-
-  if((CompletionEvent != INVALID_HANDLE_VALUE && Status == STATUS_PENDING) ||
-     !NT_SUCCESS(Status))
-  {
-    /* In case CompletionEvent == NULL, RtlDeleteTimerQueueEx() returns before
-       all callback routines returned. We set the last error code to STATUS_PENDING
-       and return FALSE. In case CompletionEvent == INVALID_HANDLE_VALUE we only
-       can get here if another error occured. In case CompletionEvent is something
-       else, we get here and fail, even though it isn't really an error (if Status ==
STATUS_PENDING).
-       We also handle all other failures the same way. */
-
-    BaseSetLastNTError(Status);
-    return FALSE;
-  }
-
-  return TRUE;
-}
-
-/*
- * @implemented
- */
-BOOL
-WINAPI
-DeleteTimerQueueTimer(HANDLE TimerQueue,
-                      HANDLE Timer,
-                      HANDLE CompletionEvent)
-{
-  NTSTATUS Status;
-
-  if(TimerQueue == NULL)
-  {
-    /* let's use the process' default timer queue. We assume the default timer
-       queue has been created with a previous call to CreateTimerQueueTimer() or
-       SetTimerQueueTimer(), otherwise this call wouldn't make much sense. */
-    if(!(TimerQueue = DefaultTimerQueue))
-    {
-      SetLastError(ERROR_INVALID_HANDLE);
-      return FALSE;
-    }
-  }
-
-  if(Timer == NULL)
-  {
-    SetLastError(ERROR_INVALID_PARAMETER);
-    return FALSE;
-  }
-
-  /* delete the timer */
-  Status = RtlDeleteTimer(TimerQueue, Timer, CompletionEvent);
-
-  if((CompletionEvent != INVALID_HANDLE_VALUE && Status == STATUS_PENDING) ||
-     !NT_SUCCESS(Status))
-  {
-    /* In case CompletionEvent == NULL, RtlDeleteTimer() returns before
-       the callback routine returned. We set the last error code to STATUS_PENDING
-       and return FALSE. In case CompletionEvent == INVALID_HANDLE_VALUE we only
-       can get here if another error occured. In case CompletionEvent is something
-       else, we get here and fail, even though it isn't really an error (if Status ==
STATUS_PENDING).
-       We also handle all other failures the same way. */
-
-    BaseSetLastNTError(Status);
-    return FALSE;
-  }
-
-  return TRUE;
+    HANDLE Handle;
+    NTSTATUS Status;
+
+    /* Create the timer queue */
+    Status = RtlCreateTimerQueue(&Handle);
+    if(!NT_SUCCESS(Status))
+    {
+        BaseSetLastNTError(Status);
+        return NULL;
+    }
+
+    return Handle;
+}
+
+/*
+ * @implemented
+ */
+BOOL
+WINAPI
+CreateTimerQueueTimer(OUT PHANDLE phNewTimer,
+                      IN HANDLE TimerQueue,
+                      IN WAITORTIMERCALLBACK Callback,
+                      IN PVOID Parameter,
+                      IN DWORD DueTime,
+                      IN DWORD Period,
+                      IN ULONG Flags)
+{
+    NTSTATUS Status;
+
+    /* Parameter is not optional -- clear it now */
+    *phNewTimer = NULL;
+
+    /* Check if no queue was given */
+    if (!TimerQueue)
+    {
+        /* Create the queue if it didn't already exist */
+        if (!(BasepDefaultTimerQueue) && !(BasepCreateDefaultTimerQueue()))
+        {
+            return FALSE;
+        }
+
+        /* Use the default queue */
+        TimerQueue = BasepDefaultTimerQueue;
+    }
+
+    /* Create the timer. Note that no parameter checking is done here */
+    Status = RtlCreateTimer(TimerQueue,
+                            phNewTimer,
+                            Callback,
+                            Parameter,
+                            DueTime,
+                            Period,
+                            Flags);
+    if (!NT_SUCCESS(Status))
+    {
+        BaseSetLastNTError(Status);
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+/*
+ * @implemented
+ */
+BOOL
+WINAPI
+DeleteTimerQueue(IN HANDLE TimerQueue)
+{
+    /* We don't allow the user to delete the default timer queue */
+    if (!TimerQueue)
+    {
+        SetLastError(ERROR_INVALID_HANDLE);
+        return FALSE;
+    }
+
+    /* Delete the timer queue */
+    RtlDeleteTimerQueueEx(TimerQueue, 0);
+    return TRUE;
+}
+
+/*
+ * @implemented
+ */
+BOOL
+WINAPI
+DeleteTimerQueueEx(IN HANDLE TimerQueue,
+                   IN HANDLE CompletionEvent)
+{
+    NTSTATUS Status;
+
+    /* We don't allow the user to delete the default timer queue */
+    if (!TimerQueue)
+    {
+        SetLastError(ERROR_INVALID_HANDLE);
+        return FALSE;
+    }
+
+    /* Delete the timer queue */
+    Status = RtlDeleteTimerQueueEx(TimerQueue, CompletionEvent);
+    if (((CompletionEvent != INVALID_HANDLE_VALUE) && (Status == STATUS_PENDING))
||
+        !(NT_SUCCESS(Status)))
+    {
+        /* In case CompletionEvent == NULL, RtlDeleteTimerQueueEx() returns before
+           all callback routines returned. We set the last error code to STATUS_PENDING
+           and return FALSE. In case CompletionEvent == INVALID_HANDLE_VALUE we only
+           can get here if another error occured. In case CompletionEvent is something
+           else, we get here and fail, even though it isn't really an error (if
Status == STATUS_PENDING).
+           We also handle all other failures the same way. */
+        BaseSetLastNTError(Status);
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+/*
+ * @implemented
+ */
+BOOL
+WINAPI
+DeleteTimerQueueTimer(IN HANDLE TimerQueue,
+                      IN HANDLE Timer,
+                      IN HANDLE CompletionEvent)
+{
+    NTSTATUS Status;
+
+    if (!TimerQueue)
+    {
+        /* Use the default timer queue */
+        TimerQueue = BasepDefaultTimerQueue;
+        if (!TimerQueue)
+        {
+            /* A timer is being cancelled before one was created... fail */
+            SetLastError(ERROR_INVALID_PARAMETER);
+            return FALSE;
+        }
+    }
+
+    /* Delete the timer */
+    Status = RtlDeleteTimer(TimerQueue, Timer, CompletionEvent);
+    if (((CompletionEvent != INVALID_HANDLE_VALUE) && (Status == STATUS_PENDING))
||
+        !(NT_SUCCESS(Status)))
+    {
+        /* In case CompletionEvent == NULL, RtlDeleteTimerQueueEx() returns before
+           all callback routines returned. We set the last error code to STATUS_PENDING
+           and return FALSE. In case CompletionEvent == INVALID_HANDLE_VALUE we only
+           can get here if another error occured. In case CompletionEvent is something
+           else, we get here and fail, even though it isn't really an error (if
Status == STATUS_PENDING).
+           We also handle all other failures the same way. */
+        BaseSetLastNTError(Status);
+        return FALSE;
+    }
+
+    return TRUE;
 }
 
 /*
@@ -310,50 +278,44 @@
  */
 HANDLE
 WINAPI
-SetTimerQueueTimer(HANDLE TimerQueue,
-                   WAITORTIMERCALLBACK Callback,
-                   PVOID Parameter,
-                   DWORD DueTime,
-                   DWORD Period,
-                   BOOL PreferIo)
-{
-  /* Since this function is not documented in PSDK and apparently does nothing
-     but create a timer, we just do the same as CreateTimerQueueTimer(). Unfortunately
-     I don't really know what PreferIo is supposed to be, it propably just affects
the
-     Flags parameter of CreateTimerQueueTimer(). Looking at the PSDK documentation of
-     CreateTimerQueueTimer() there's only one flag (WT_EXECUTEINIOTHREAD) that
causes
-     the callback function queued to an I/O worker thread. I guess it uses this flag
-     if PreferIo == TRUE, otherwise let's just use WT_EXECUTEDEFAULT. We should
-     test this though, this is only guess work and I'm too lazy to do further
-     investigation. */
-
-  HANDLE Timer;
-  NTSTATUS Status;
-
-  if(TimerQueue == NULL)
-  {
-    /* the default timer queue is requested, try to create it if it hasn't been
already */
-    if(!(TimerQueue = DefaultTimerQueue))
-    {
-      if(!IntCreateDefaultTimerQueue())
-      {
-        /* IntCreateDefaultTimerQueue() set the last error code already, just fail */
-        return FALSE;
-      }
-      TimerQueue = DefaultTimerQueue;
-    }
-  }
-
-  /* create the timer */
-  Status = RtlCreateTimer(TimerQueue, &Timer, Callback, Parameter, DueTime,
-                          Period, (PreferIo ? WT_EXECUTEINIOTHREAD :
WT_EXECUTEDEFAULT));
-  if(!NT_SUCCESS(Status))
-  {
-    BaseSetLastNTError(Status);
-    return NULL;
-  }
-
-  return Timer;
+SetTimerQueueTimer(IN HANDLE TimerQueue,
+                   IN WAITORTIMERCALLBACK Callback,
+                   IN PVOID Parameter,
+                   IN DWORD DueTime,
+                   IN DWORD Period,
+                   IN BOOL PreferIo)
+{
+    HANDLE Timer;
+    NTSTATUS Status;
+
+    /* Check if no queue was given */
+    if (!TimerQueue)
+    {
+        /* Create the queue if it didn't already exist */
+        if (!(BasepDefaultTimerQueue) && !(BasepCreateDefaultTimerQueue()))
+        {
+            return FALSE;
+        }
+
+        /* Use the default queue */
+        TimerQueue = BasepDefaultTimerQueue;
+    }
+
+    /* Create the timer */
+    Status = RtlCreateTimer(TimerQueue,
+                            &Timer,
+                            Callback,
+                            Parameter,
+                            DueTime,
+                            Period,
+                            PreferIo ? WT_EXECUTEINIOTHREAD : WT_EXECUTEDEFAULT);
+    if (!NT_SUCCESS(Status))
+    {
+        BaseSetLastNTError(Status);
+        return NULL;
+    }
+
+    return Timer;
 }
 
 /* EOF */