threads / patch / 59165

patchwin32: check for NULL when creating thread

Subject: [PATCH] win32: check for NULL when creating thread

## tl;dr

7 messages between Jan 31, 2023 and Feb 1, 2023. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Rose via GitGitGadget· Jan 31, 2023, 14:49 UTC · lore
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 when 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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1445/AtariDreams/hThread-v1
Pull-Request: https://github.com/git/git/pull/1445
 compat/winansi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/winansi.c +1 −1
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

-- 
gitgitgadget
Rose via GitGitGadget· Jan 31, 2023, 14:53 UTC · re: Rose via GitGitGadget · lore

[PATCH v2] win32: check for NULL after creating thread

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-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1445/AtariDreams/hThread-v2
Pull-Request: https://github.com/git/git/pull/1445
Range-diff vs v1:
 1:  e75d15e42f4 ! 1:  c956cafdec9 win32: check for NULL when creating thread
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    win32: check for NULL when creating thread
     +    win32: check for NULL after creating thread
      
          Check for NULL handles, not "INVALID_HANDLE,"
          as CreateThread guarantees a valid handle in most cases.
 compat/winansi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/winansi.c +1 −1
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

-- 
gitgitgadget
Rose via GitGitGadget· Feb 1, 2023, 14:40 UTC · re: Rose via GitGitGadget · lore

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

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(-)
Show changes to compat/winansi.c +1 −1
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

-- 
gitgitgadget
Johannes Sixt· Feb 1, 2023, 21:51 UTC · re: Rose via GitGitGadget · lore

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

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
Rose via GitGitGadget· Feb 1, 2023, 22:20 UTC · re: Rose via GitGitGadget · lore

[PATCH v4] compat/winansi: check for errors of CreateThread() correctly

From: Seija Kijin <doremylover123@gmail.com>

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-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1445/AtariDreams/hThread-v4
Pull-Request: https://github.com/git/git/pull/1445
Range-diff vs v3:
 1:  1cbc43e0d82 ! 1:  6c4188977e8 win32: check for NULL after creating thread
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    win32: check for NULL after creating thread
     -
     -    Check for NULL handles, not "INVALID_HANDLE,"
     -    as CreateThread guarantees a valid handle in most cases.
     +    compat/winansi: check for errors of CreateThread() correctly
      
          The return value for failed thread creation is NULL,
          not INVALID_HANDLE_VALUE, unlike other Windows API functions.
 compat/winansi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/winansi.c +1 −1
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

-- 
gitgitgadget
Junio C Hamano· Feb 1, 2023, 22:37 UTC · re: Rose via GitGitGadget · lore

Re: [PATCH v4] compat/winansi: check for errors of CreateThread() correctly

"Rose via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 7 quoted lines
> From: Seija Kijin <doremylover123@gmail.com>
>
> 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>
> ---
Thanks.  Will queue with the Ack by j6t given earlier.
Johannes Sixt· Feb 1, 2023, 22:39 UTC · re: Rose via GitGitGadget · lore

Re: [PATCH v4] compat/winansi: check for errors of CreateThread() correctly

Am 01.02.23 um 23:20 schrieb Rose via GitGitGadget:
Show 56 quoted lines
> From: Seija Kijin <doremylover123@gmail.com>
> 
> 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-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1445/AtariDreams/hThread-v4
> Pull-Request: https://github.com/git/git/pull/1445
> 
> Range-diff vs v3:
> 
>  1:  1cbc43e0d82 ! 1:  6c4188977e8 win32: check for NULL after creating thread
>      @@ Metadata
>       Author: Seija Kijin <doremylover123@gmail.com>
>       
>        ## Commit message ##
>      -    win32: check for NULL after creating thread
>      -
>      -    Check for NULL handles, not "INVALID_HANDLE,"
>      -    as CreateThread guarantees a valid handle in most cases.
>      +    compat/winansi: check for errors of CreateThread() correctly
>       
>           The return value for failed thread creation is NULL,
>           not INVALID_HANDLE_VALUE, unlike other Windows API functions.
> 
> 
>  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
This iteration looks good, thank you!
Acked-by: Johannes Sixt <j6t@kdbg.org>
-- Hannes

← back to recent threads