Dmitry do you have a reply for this? I worry that you're simply copying Windows internals without thinking about it.
I agree with Thomas on this, if we can do something better then we should. There's no reason to do everything in exactly the same manner just to for the sake of copying. In fact, I would actually advise we do things slightly differently whenever possible. Ged. -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Thomas Bluemel Sent: 17 July 2009 22:53 To: ReactOS Development List Subject: Re: [ros-dev] [ros-diffs] [dchapyshev] 42012: - Samplify SwitchToThread and QueueUserWorkItem - Remove unneeded InternalWorkItemTrampoline function and QUEUE_USER_WORKITEM_CONTEXT structure - Other small changes Just because Wine and Windows do it that way doesn't make it any more correct or portable. RtlQueueWorkItem expects a function of this type: typedef VOID (NTAPI *WORKERCALLBACKFUNC)(IN PVOID Context); The supplied function is of this type: typedef ULONG (NTAPI *PTHREAD_START_ROUTINE)(PVOID Parameter); This doesn't crash on x86 and "works" because the return value is simply ignored, but it's definitely not portable and MAY crash on another architecture where it does make a difference. I don't know if it would matter on ARM/PPC/... but the old code at least implemented it correctly/portable. It's not really an issue but IMO a bad hack. Typecasting a function pointer to a function with a different signature is hardly ever good practice. - Thomas Dmitry Chapyshev wrote: > On Fri, 17 Jul 2009 23:03:18 +0400, Thomas Bluemel <[email protected]> > wrote: > >> This explains why we are using a trampoline function and not just >> typecast there... Is there a reason you changed this? >> >> - Thomas >> >> [email protected] wrote: >>> - /* NOTE: Don't use Function directly since the callback signature >>> - differs. This might cause problems on certain >>> platforms... */ >>> - Status = RtlQueueWorkItem(InternalWorkItemTrampoline, >>> - WorkItemContext, >> _______________________________________________ >> Ros-dev mailing list >> [email protected] >> http://www.reactos.org/mailman/listinfo/ros-dev > > > > Wine and Windows do so. Why we should do in another way? Other reasons are > necessary? > _______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev _______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
