I don't know if this is the same thing or not, but I just happened to read
this the other day (while looking up QueueUserWorkItem, incidentally) and
this conversation reminded me of it:
http://msdn.microsoft.com/en-us/library/ms686736(VS.85).aspx
Do not declare this callback function with a void return type and cast the
function pointer to LPTHREAD_START_ROUTINE when creating the thread. Code
that does this is common, but it can crash on 64-bit Windows.
It certainly sounds as though somebody at Microsoft actually encountered
such a crash. I don't think it would be a problem on x64, but I know
next to nothing about Itanium, maybe it's an issue there?
On Tue, 21 Jul 2009, Alex Ionescu wrote:
That doesn't make any sense.
All architectures have an ABI that defines which registers are used for
return values, and so the compiler will NEVER (under ANY circumstances)
assume that the return register should somehow be "usable", especially for
an external function pointer!
Furthermore, this is typecasting from a ULONG to a VOID, isn't it? So the
"real" function will never return anything in the first place, making this a
non-issue.
I challenge you to provide a test case/example on any architecture where
this could possibly happen. It can't.
Best regards,
Alex Ionescu
On Tue, Jul 21, 2009 at 1:11 PM, Thomas Bluemel <thomas@reactsoft.com>
wrote:
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: ros-dev-bounces@reactos.org [mailto:ros-dev-
bounces@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@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@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@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@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
Best regards,
Alex Ionescu
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev