[PATCH 2/4] msvcrt: Use trampoline for _beginthreadex().

[Piotr Caban]

Hi Arek,

On 5/3/21 3:44 PM, Arkadiusz Hiler wrote:
> +/*********************************************************************
> + *		_beginthreadex_trampoline
> + */
> +static DWORD CALLBACK _beginthreadex_trampoline(LPVOID arg)
> +{
> +    unsigned int retval;
> +    _beginthread_trampoline_t local_trampoline;
> +    thread_data_t *data = msvcrt_get_thread_data();
> +
> +    memcpy(&local_trampoline,arg,sizeof(local_trampoline));
Please add spaces between arguments.

>   /*********************************************************************
>    *		_beginthreadex (MSVCRT.@)
>    */
> @@ -157,12 +177,30 @@ uintptr_t CDECL _beginthreadex(
>     unsigned int initflag,   /* [in] Initial state of new thread (0 for running or CREATE_SUSPEND for suspended) */
>     unsigned int *thrdaddr)  /* [out] Points to a 32-bit variable that receives the thread identifier */
>   {
> +  _beginthread_trampoline_t* trampoline;
> +  HANDLE thread;
> +
>     TRACE("(%p, %d, %p, %p, %d, %p)\n", security, stack_size, start_address, arglist, initflag, thrdaddr);
>   
> -  /* FIXME */
> -  return (uintptr_t)CreateThread(security, stack_size,
> -				     start_address, arglist,
> -				     initflag, thrdaddr);
> +  trampoline = malloc(sizeof(*trampoline));
> +  if(!trampoline) {
> +      *_errno() = EAGAIN;
> +      return -1;
malloc already sets errno. Resetting it to EAGAIN looks questionable. 
The function should also return 0 on error.

> +  trampoline->thread = INVALID_HANDLE_VALUE;
> +  trampoline->start_address_ex = start_address;
> +  trampoline->arglist = arglist;
> +
> +  thread = CreateThread(security, stack_size, _beginthreadex_trampoline,
> +          trampoline, initflag, thrdaddr);
> +  if(!thread) {
> +      free(trampoline);
> +      *_errno() = EAGAIN;
> +      return -1;
It should probably do something like:
msvcrt_set_errno(GetLastError());
return 0;

It would be nice to do similar changes in _beginthread() as well.

Thanks,
Piotr