Re: [dpdk-dev] [PATCH v3] eal/windows: fix invalid thread handle

[Thomas Monjalon <thomas@monjalon.net>]

02/06/2020 04:00, Tasnim Bashar:
> Casting thread ID to handle is not accurate way to get thread handle.
> Need to use OpenThread function to get thread handle from thread ID.
> 
> pthread_setaffinity_np and pthread_getaffinity_np functions
> for Windows are affected because of it.
> 
> Signed-off-by: Tasnim Bashar <tbashar@mellanox.com>
> ---
> v3: WA to remove warning(-Wmaybe-uninitialized)

The -Wmaybe-uninitialized warning was there before this patch.
Shouldn't it be a separate patch before this one?

>  static inline int
> -eal_get_thread_affinity_mask(pthread_t threadid, unsigned long *cpuset)
> +eal_get_thread_affinity_mask(pthread_t threadid, rte_cpuset_t *cpuset)
>  {
>  	/* Workaround for the lack of a GetThreadAffinityMask()
>  	 *API in Windows
>  	 */
> -		/* obtain previous mask by setting dummy mask */
> -	DWORD dwprevaffinitymask =
> -		SetThreadAffinityMask((HANDLE) threadid, 0x1);
> +	DWORD_PTR dwprevaffinitymask;

Please use underscores to separate parts in names.

> +	HANDLE thread_handle;
> +	DWORD_PTR ret;
> +
> +	thread_handle = OpenThread(THREAD_ALL_ACCESS, FALSE, threadid);
> +	if (thread_handle == NULL) {
> +		RTE_LOG_WIN32_ERR("OpenThread()");
> +		return -1;
> +	}
> +
> +	/* obtain previous mask by setting dummy mask */
> +	dwprevaffinitymask = SetThreadAffinityMask(thread_handle, 0x1);
> +	if (dwprevaffinitymask == 0) {
> +		RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
> +		CloseHandle(thread_handle);
> +		return -1;
> +	}
> +
>  	/* set it back! */
> -	SetThreadAffinityMask((HANDLE) threadid, dwprevaffinitymask);
> -	*cpuset = dwprevaffinitymask;
> +	ret = SetThreadAffinityMask(thread_handle, dwprevaffinitymask);
> +	if (ret == 0) {
> +		RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
> +		CloseHandle(thread_handle);
> +		return -1;
> +	}
> +	memset(cpuset, 0, sizeof(rte_cpuset_t));

Shouldn't we use RTE_CPU_ZERO instead of memset?

> +	*cpuset->_bits = dwprevaffinitymask;
> +	CloseHandle(thread_handle);
>  	return 0;
>  }