diff mbox series

[v3] win32: check for NULL after creating thread

Message ID pull.1445.v3.git.git.1675262454817.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 592bcab61b86dbbac6fd2366039b4837b306ed66
Headers show
Series [v3] win32: check for NULL after creating thread | expand

Commit Message

Seija Kijin Feb. 1, 2023, 2:40 p.m. UTC 
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.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: check for NULL after creating thread
    
    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.
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1445%2FAtariDreams%2FhThread-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1445/AtariDreams/hThread-v3
Pull-Request: https://github.com/git/git/pull/1445

Range-diff vs v2:

 1:  c956cafdec9 ! 1:  1cbc43e0d82 win32: check for NULL after creating thread
     @@ Commit message
          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.
     +    not INVALID_HANDLE_VALUE, unlike other Windows API functions.
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      


 compat/winansi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473

Comments

Johannes Sixt Feb. 1, 2023, 9:51 p.m. UTC | #1 
Am 01.02.23 um 15:40 schrieb Rose via GitGitGadget:
> 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").

>  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
diff mbox series

Patch

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