[ros-dev] [ros-diffs] [dchapyshev] 42012: - Samplify SwitchToThread and QueueUserWorkItem - Remove unneeded InternalWorkItemTrampoline function and QUEUE_USER_WORKITEM_CONTEXT structure - Other small changes
Ged
gedmurphy at gmail.com
Tue Jul 21 13:02:39 CEST 2009
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: ros-dev-bounces at reactos.org [mailto:ros-dev-bounces at reactos.org] 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 <thomas at reactsoft.com>
> wrote:
>
>> This explains why we are using a trampoline function and not just
>> typecast there... Is there a reason you changed this?
>>
>> - Thomas
>>
>> dchapyshev at svn.reactos.org 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
>> Ros-dev at reactos.org
>> 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
Ros-dev at reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
More information about the Ros-dev
mailing list