[ros-dev] [ros-diffs] [dchapyshev] 42012: - Samplify SwitchToThread and QueueUserWorkItem - Remove unneeded InternalWorkItemTrampoline function and QUEUE_USER_WORKITEM_CONTEXT structure - Other small changes

[Alex Ionescu]

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: 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
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev

Best regards,
Alex Ionescu