Re: [PATCH v3] win32: check for NULL after creating thread

From: Johannes Sixt <j6t@kdbg.org>
Date: Feb 1, 2023, 21:51 UTC
Message-ID: <f3de2ca3-167e-365c-8124-d6ba9bba920d@kdbg.org>
In-Reply-To: <pull.1445.v3.git.git.1675262454817.gitgitgadget@gmail.com>

Am 01.02.23 um 15:40 schrieb Rose via GitGitGadget:

Show 7 quoted lines

> From: Seija Kijin <doremylover123@gmail.com>
> 
> Check for NULL handles, not "INVALID_HANDLE,"
> as CreateThread guarantees a valid handle in most cases.
> 
> The return value for failed thread creation is NULL,
> not INVALID_HANDLE_VALUE, unlike other Windows API functions.

Nice catch!

The subject line sounds as if an error check was missing, but that is not true. I'd phrase it

	compat/winansi: check for errors of CreateThread() correctly

Then drop the first sentence of the message body as it is very handwavy: talking about "most cases" is not helpful if the few other cases are not enumerated. And the subsequent sentence is to the point and very helpful (substitute "CreateThread" for "thread creation").

Show 18 quoted lines

>  compat/winansi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 3abe8dd5a27..f83610f684d 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -644,7 +644,7 @@ void winansi_init(void)
>  
>  	/* start console spool thread on the pipe's read end */
>  	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
> -	if (hthread == INVALID_HANDLE_VALUE)
> +	if (!hthread)
>  		die_lasterr("CreateThread(console_thread) failed");
>  
>  	/* schedule cleanup routine */
> 
> base-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473

Acked-by: Johannes Sixt <j6t@kdbg.org>

-- Hannes