On an architecture where function return values e.g. modify a register that would otherwise be preserved? Or that have a different return method depending on whether data needs to be returned? It's like typecasting a function to one with a different calling method, except that it may work fine for *most* but not all architectures. It's still bad code...
Thomas Alex Ionescu wrote: > I have no idea why typecasting would make it "crash" on another > architecture... > > On 21-Jul-09, at 4:02 AM, Ged wrote: > >> 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:ros-dev- >> [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 > > Best regards, > Alex Ionescu > > _______________________________________________ > 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
