[ros-dev] [ros-diffs] [mjmartin] 47393: [win32k] - The timer is created usingUserCreateObject. It may be a good idea to save the handle in the timer object so that it can be deleted later. - Dereference the object before attempting to delete it.

[Ged Murphy]

There's no lock on the list access.

On 29 May 2010 07:51, <mjmartin at svn.reactos.org> wrote:

> Author: mjmartin
> Date: Sat May 29 08:51:03 2010
> New Revision: 47393
>
> URL: http://svn.reactos.org/svn/reactos?rev=47393&view=rev
> Log:
> [win32k]
> - The timer is created usingUserCreateObject. It may be a good idea to save
> the handle in the timer object so that it can be deleted later.
> - Dereference the object before attempting to delete it.
>
> Modified:
>    trunk/reactos/subsystems/win32/win32k/ntuser/timer.c
>
> Modified: trunk/reactos/subsystems/win32/win32k/ntuser/timer.c
> URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntuser/timer.c?rev=47393&r1=47392&r2=47393&view=diff
>
> ==============================================================================
> --- trunk/reactos/subsystems/win32/win32k/ntuser/timer.c [iso-8859-1]
> (original)
> +++ trunk/reactos/subsystems/win32/win32k/ntuser/timer.c [iso-8859-1] Sat
> May 29 08:51:03 2010
> @@ -50,13 +50,21 @@
>   if (!FirstpTmr)
>   {
>       FirstpTmr = UserCreateObject(gHandleTable, NULL, &Handle, otTimer,
> sizeof(TIMER));
> -      if (FirstpTmr) InitializeListHead(&FirstpTmr->ptmrList);
> +      if (FirstpTmr)
> +      {
> +         FirstpTmr->head.h = Handle;
> +         InitializeListHead(&FirstpTmr->ptmrList);
> +      }
>       Ret = FirstpTmr;
>   }
>   else
>   {
>       Ret = UserCreateObject(gHandleTable, NULL, &Handle, otTimer,
> sizeof(TIMER));
> -      if (Ret) InsertTailList(&FirstpTmr->ptmrList, &Ret->ptmrList);
> +      if (Ret)
> +      {
> +         Ret->head.h = Handle;
> +         InsertTailList(&FirstpTmr->ptmrList, &Ret->ptmrList);
> +      }
>   }
>   return Ret;
>  }
> @@ -66,14 +74,17 @@
>  FASTCALL
>  RemoveTimer(PTIMER pTmr)
>  {
> +  BOOL Ret = FALSE;
>   if (pTmr)
>   {
>      /* Set the flag, it will be removed when ready */
>      RemoveEntryList(&pTmr->ptmrList);
> -     UserDeleteObject( UserHMGetHandle(pTmr), otTimer);
> -     return TRUE;
> -  }
> -  return FALSE;
> +     UserDereferenceObject(pTmr);
> +     Ret = UserDeleteObject( UserHMGetHandle(pTmr), otTimer);
> +  }
> +  if (!Ret) DPRINT1("Warning unable to delete timer\n");
> +
> +  return Ret;
>  }
>
>  PTIMER
> @@ -528,9 +539,7 @@
>    {
>       if ((pTmr) && (pTmr->pti == pti) && (pTmr->pWnd == Window))
>       {
> -         RemoveEntryList(&pTmr->ptmrList);
> -         UserDeleteObject( UserHMGetHandle(pTmr), otTimer);
> -         TimersRemoved = TRUE;
> +         TimersRemoved = RemoveTimer(pTmr);
>       }
>       pLE = pTmr->ptmrList.Flink;
>       pTmr = CONTAINING_RECORD(pLE, TIMER, ptmrList);
> @@ -557,9 +566,7 @@
>    {
>       if ((pTmr) && (pTmr->pti == pti))
>       {
> -         RemoveEntryList(&pTmr->ptmrList);
> -         UserDeleteObject( UserHMGetHandle(pTmr), otTimer);
> -         TimersRemoved = TRUE;
> +         TimersRemoved = RemoveTimer(pTmr);
>       }
>       pLE = pTmr->ptmrList.Flink;
>       pTmr = CONTAINING_RECORD(pLE, TIMER, ptmrList);
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20100530/3507667a/attachment-0001.htm>