Re: [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(a)reactos.org [mailto:ros-dev- 
>  bounces(a)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(a)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(a)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(a)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(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 
Best regards,
Alex Ionescu