On WinCE, createThreadInternal can be implemented directly with CreateThread
Created attachment 32790 [details] patch to ThreadingWin.cpp
Comment on attachment 32790 [details] patch to ThreadingWin.cpp > +#if !PLATFORM(WINCE) > #include <process.h> > +#endif For? > +#if HAVE(ERRNO_H) > +#include <errno.h> > +#else > +#define NO_ERRNO > +#endif I'm not sure. In a previous patch to RegisterFile we were using 'GetLastError' instead of errno for WINCE. Shouldn't we do the same here? Moreover, we should create a proper WTF abstraction for ERRNO it seems. > +#if PLATFORM(WINCE) > + // This should be safe on WINCE. On Windows desktop it is not. > + HANDLE threadHandle = CreateThread(0, 0, (LPTHREAD_START_ROUTINE)wtfThreadEntryPoint, invocation, 0, (LPDWORD)&threadIdentifier); Why is it safe for WINCE, but not for desktop and what does it gain us?
(In reply to comment #2) > (From update of attachment 32790 [details]) > > +#if !PLATFORM(WINCE) > > #include <process.h> > > +#endif > > For? process.h doesn't exist on WINCE > > > +#if HAVE(ERRNO_H) > > +#include <errno.h> > > +#else > > +#define NO_ERRNO > > +#endif > > I'm not sure. In a previous patch to RegisterFile we were using 'GetLastError' > instead of errno for WINCE. Shouldn't we do the same here? Moreover, we > should create a proper WTF abstraction for ERRNO it seems. I'm not sure replacing ERRNO with GetLastError is good in all cases. In this file, we can do that because the only case of using errno is after CreateThread() fails. > > > +#if PLATFORM(WINCE) > > + // This should be safe on WINCE. On Windows desktop it is not. > > + HANDLE threadHandle = CreateThread(0, 0, (LPTHREAD_START_ROUTINE)wtfThreadEntryPoint, invocation, 0, (LPDWORD)&threadIdentifier); > > Why is it safe for WINCE, but not for desktop and what does it gain us? No gain. we do this because with don't have _beginthreadex() on WINCE.
(In reply to comment #3) > I'm not sure replacing ERRNO with GetLastError is good in all cases. In this > file, we can do that because the only case of using errno is after > CreateThread() fails. We should do that then.
(In reply to comment #4) > (In reply to comment #3) > > I'm not sure replacing ERRNO with GetLastError is good in all cases. In this > > file, we can do that because the only case of using errno is after > > CreateThread() fails. > > We should do that then. Seems like a separate bug+patch.
Created attachment 32795 [details] Updated patch Updated the patch to call ::GetLastError on WINCE (and also leave the NO_ERRNO path in, just in case there's a different platform without errno). Also added a more detailed comment on why CreateThread is safe.
Comment on attachment 32795 [details] Updated patch I'm not sure that NO_ERRNO is needed anymore but I don't see the harm either. It will help in porting to other platforms I think.
Comment on attachment 32795 [details] Updated patch Looks good to me too.
Landed with r45931.