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

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

On Sat, 23 May 2020 00:25:56 -0700
Tasnim Bashar <tbashar@mellanox.com> wrote:

> 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>
> ---

Please use --in-reply-to=Message-ID-of-previous-version to send v2, etc.
If you're using Outlook, you can find message ID as follows:

https://support.office.com/en-us/article/view-internet-message-headers-in-outlook-cd039382-dc6e-4264-ac74-c048563d212c

For example, this is how this patch should have been sent:

git send-email --in-reply-to=20200522001112.48932-1-tbashar@mellanox.com ...

[snip]
> +
>  	/* set it back! */
> -	SetThreadAffinityMask((HANDLE) threadid, dwprevaffinitymask);
> +	ret = SetThreadAffinityMask(thread_handle, dwprevaffinitymask);
> +	if (ret == 0) {
> +		RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
> +		CloseHandle(thread_handle);
> +		return -1;
> +	}
>  	*cpuset = dwprevaffinitymask;

Getting a warning with MinGW-w64:

In file included from ../../../../../lib/librte_eal/include/rte_eal.h:15,
                 from ../../../../../lib/librte_eal/common/eal_common_options.c:24:
../../../../../lib/librte_eal/common/eal_common_options.c: In function 'eal_adjust_config':
../../../../../lib/librte_eal/windows/include/sched.h:63:40: warning: 'default_set' may be used uninitialized in this function [-Wmaybe-uninitialized]
   63 |   (dst)->_bits[_i] = (src1)->_bits[_i] & (src2)->_bits[_i]; \
      |                                        ^
../../../../../lib/librte_eal/common/eal_common_options.c:1624:15: note: 'default_set' was declared here
 1624 |  rte_cpuset_t default_set;
      |               ^~~~~~~~~~~

This is correct, because you only initialize first "long long" in
rte_cpuset_t and not the second. Since we're trying not to introduce new
warnings, the fix is as follows:

	memset(cpuset, 0, sizeof(rte_cpuset_t));

"sizeof(rte_cpuset_t)" requires <sched.h> and then it's probably worth using
"rte_cpuset_t *" for instead of "long long *" for the parameter.

-- 
Dmitry Kozlyuk