[PATCH] Mark win32's pthread_exit() as NORETURN

[PATCH] Mark win32's pthread_exit() as NORETURN

[Johannes Schindelin]

The pthread_exit() function is not expected to return. Ever.

Pointed out by Jeff King.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/win32/pthread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index 20b35a2..148db60 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
 #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
 extern pthread_t pthread_self(void);
 
-static inline int pthread_exit(void *ret)
+static inline int NORETURN pthread_exit(void *ret)
 {
 	ExitThread((DWORD)(intptr_t)ret);
 }
-- 
2.7.2.windows.1.5.g64acc33

Re: [PATCH] Mark win32's pthread_exit() as NORETURN

[Jeff King]

On Tue, Mar 01, 2016 at 02:53:04PM +0100, Johannes Schindelin wrote:

> The pthread_exit() function is not expected to return. Ever.
> 
> Pointed out by Jeff King.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/win32/pthread.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
> index 20b35a2..148db60 100644
> --- a/compat/win32/pthread.h
> +++ b/compat/win32/pthread.h
> @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
>  #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
>  extern pthread_t pthread_self(void);
>  
> -static inline int pthread_exit(void *ret)
> +static inline int NORETURN pthread_exit(void *ret)
>  {
>  	ExitThread((DWORD)(intptr_t)ret);
>  }

Looks obviously correct to me (I'll assume Windows isn't so crazy as to
let ExitThread ever return :) ).

-Peff
> -- 
> 2.7.2.windows.1.5.g64acc33

Re: [PATCH] Mark win32's pthread_exit() as NORETURN

[Johannes Schindelin]

Hi Peff,

On Tue, 1 Mar 2016, Jeff King wrote:

> On Tue, Mar 01, 2016 at 02:53:04PM +0100, Johannes Schindelin wrote:
> 
> > The pthread_exit() function is not expected to return. Ever.
> > 
> > Pointed out by Jeff King.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  compat/win32/pthread.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
> > index 20b35a2..148db60 100644
> > --- a/compat/win32/pthread.h
> > +++ b/compat/win32/pthread.h
> > @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
> >  #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
> >  extern pthread_t pthread_self(void);
> >  
> > -static inline int pthread_exit(void *ret)
> > +static inline int NORETURN pthread_exit(void *ret)
> >  {
> >  	ExitThread((DWORD)(intptr_t)ret);
> >  }
> 
> Looks obviously correct to me (I'll assume Windows isn't so crazy as to
> let ExitThread ever return :) ).

Indeed, you are correct in your implicit assumption that I should have
clarified that in my commit message. I will amend the commit message.

Ciao,
Dscho

[PATCH v2] Mark win32's pthread_exit() as NORETURN

[Johannes Schindelin]

The pthread_exit() function is not expected to return. Ever. On Windows,
we call ExitThread() whose documentation claims: "This function does not
return a value.":

	https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659

Pointed out by Jeff King.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Relative to v1, only the commit message changed (to clarify that
	ExitThread() indeed never returns).

 compat/win32/pthread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index 20b35a2..148db60 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
 #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
 extern pthread_t pthread_self(void);
 
-static inline int pthread_exit(void *ret)
+static inline int NORETURN pthread_exit(void *ret)
 {
 	ExitThread((DWORD)(intptr_t)ret);
 }
-- 
2.7.2.windows.1.5.g64acc33

Re: [PATCH v2] Mark win32's pthread_exit() as NORETURN

[stefan.naewe]

Am 01.03.2016 um 15:13 schrieb Johannes Schindelin:
> The pthread_exit() function is not expected to return. Ever. On Windows,
> we call ExitThread() whose documentation claims: "This function does not
> return a value.":

Does this really mean that ExitThread() does not return ?

Just wondering...

>         https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659
> 
> Pointed out by Jeff King.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
>         Relative to v1, only the commit message changed (to clarify that
>         ExitThread() indeed never returns).
> 
>  compat/win32/pthread.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
> index 20b35a2..148db60 100644
> --- a/compat/win32/pthread.h
> +++ b/compat/win32/pthread.h
> @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
>  #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
>  extern pthread_t pthread_self(void);
> 
> -static inline int pthread_exit(void *ret)
> +static inline int NORETURN pthread_exit(void *ret)
>  {
>         ExitThread((DWORD)(intptr_t)ret);
>  }
> --


Stefan
-- 
----------------------------------------------------------------
/dev/random says: We're lost, but we're making good time.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF

Re: [PATCH v2] Mark win32's pthread_exit() as NORETURN

[Junio C Hamano]

<stefan.naewe@atlas-elektronik.com> writes:

> Am 01.03.2016 um 15:13 schrieb Johannes Schindelin:
>> The pthread_exit() function is not expected to return. Ever. On Windows,
>> we call ExitThread() whose documentation claims: "This function does not
>> return a value.":
>
> Does this really mean that ExitThread() does not return ?
>
> Just wondering...

;-).

That made me re-read the patch and notice another thing... Dscho, I
think you would need s/int/void/, too, no?

>
>>         https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659
>> 
>> Pointed out by Jeff King.
>> 
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>> 
>>         Relative to v1, only the commit message changed (to clarify that
>>         ExitThread() indeed never returns).
>> 
>>  compat/win32/pthread.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
>> index 20b35a2..148db60 100644
>> --- a/compat/win32/pthread.h
>> +++ b/compat/win32/pthread.h
>> @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
>>  #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
>>  extern pthread_t pthread_self(void);
>> 
>> -static inline int pthread_exit(void *ret)
>> +static inline int NORETURN pthread_exit(void *ret)
>>  {
>>         ExitThread((DWORD)(intptr_t)ret);
>>  }
>> --
>
>
> Stefan

Re: [PATCH v2] Mark win32's pthread_exit() as NORETURN

[Johannes Schindelin]

Hi,

On Tue, 1 Mar 2016, stefan.naewe@atlas-elektronik.com wrote:

> Am 01.03.2016 um 15:13 schrieb Johannes Schindelin:
> > The pthread_exit() function is not expected to return. Ever. On Windows,
> > we call ExitThread() whose documentation claims: "This function does not
> > return a value.":
> 
> Does this really mean that ExitThread() does not return ?

I fixed the commit message in v3.

Thanks,
Dscho

Re: [PATCH v2] Mark win32's pthread_exit() as NORETURN

[Johannes Sixt]

Am 01.03.2016 um 15:13 schrieb Johannes Schindelin:
> The pthread_exit() function is not expected to return. Ever. On Windows,
> we call ExitThread() whose documentation claims: "This function does not
> return a value.":
>
> 	https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659

This is misleading: MSDN marks all functions declared void as "does not 
return a value," for example, look at EnterCriticalSection:

https://msdn.microsoft.com/en-us/library/windows/desktop/ms682608

For this reason, I actually prefer your version 1 patch without the 
explanation.

>
> Pointed out by Jeff King.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	Relative to v1, only the commit message changed (to clarify that
> 	ExitThread() indeed never returns).
>
>   compat/win32/pthread.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
> index 20b35a2..148db60 100644
> --- a/compat/win32/pthread.h
> +++ b/compat/win32/pthread.h
> @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
>   #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
>   extern pthread_t pthread_self(void);
>
> -static inline int pthread_exit(void *ret)
> +static inline int NORETURN pthread_exit(void *ret)

I would have written it as

#ifdef __GNUC__
__attribute__((__noreturn__))
#endif
static inline int pthread_exit(void *ret) ...

but I can live with your version as long as it compiles.

Your solution is pragmatic: NORETURN is defined in git-compat-util.h, 
and by using it here, we depend on that pthread.h is included 
sufficiently late that the macro is available at this point. The 
instance in compat/nedmalloc/malloc.c.h is bracketed with #ifndef WIN32 
so that it is not compiled on Windows, all other instances are after 
git-compat-util.h or cache.h or in headers that are to be included only 
after git-compat-util.h or cache.h per convention. Looks like we are safe.

>   {
>   	ExitThread((DWORD)(intptr_t)ret);
>   }
>

Re: [PATCH v2] Mark win32's pthread_exit() as NORETURN

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 01.03.2016 um 15:13 schrieb Johannes Schindelin:
>> The pthread_exit() function is not expected to return. Ever. On Windows,
>> we call ExitThread() whose documentation claims: "This function does not
>> return a value.":
>>
>> 	https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659
>
> This is misleading: MSDN marks all functions declared void as "does
> not return a value," for example, look at EnterCriticalSection:
>
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms682608
>
> For this reason, I actually prefer your version 1 patch without the
> explanation.

;-)

>> -static inline int pthread_exit(void *ret)
>> +static inline int NORETURN pthread_exit(void *ret)
>
> I would have written it as
>
> #ifdef __GNUC__
> __attribute__((__noreturn__))
> #endif
> static inline int pthread_exit(void *ret) ...
>
> but I can live with your version as long as it compiles.

Either way, let's make sure that the final version returns "void",
cf.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html

Thanks.

Re: [PATCH v2] Mark win32's pthread_exit() as NORETURN

[Johannes Schindelin]

Hi,

On Tue, 1 Mar 2016, Junio C Hamano wrote:

> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > Am 01.03.2016 um 15:13 schrieb Johannes Schindelin:
> >> The pthread_exit() function is not expected to return. Ever. On Windows,
> >> we call ExitThread() whose documentation claims: "This function does not
> >> return a value.":
> >>
> >> 	https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659
> >
> > This is misleading: MSDN marks all functions declared void as "does
> > not return a value," for example, look at EnterCriticalSection:
> >
> > https://msdn.microsoft.com/en-us/library/windows/desktop/ms682608
> >
> > For this reason, I actually prefer your version 1 patch without the
> > explanation.
> 
> ;-)

Well, I really like to have that link so I can find it very easily myself
by simply inspecting the Git log. And the explanation itself was easily
fixed.

> 
> >> -static inline int pthread_exit(void *ret)
> >> +static inline int NORETURN pthread_exit(void *ret)
> >
> > I would have written it as
> >
> > #ifdef __GNUC__
> > __attribute__((__noreturn__))
> > #endif
> > static inline int pthread_exit(void *ret) ...
> >
> > but I can live with your version as long as it compiles.
> 
> Either way, let's make sure that the final version returns "void",
> cf.
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html

Fixed, too, in the upcoming v3.

Ciao,
Dscho

[PATCH v3] Mark win32's pthread_exit() as NORETURN

[Johannes Schindelin]

The pthread_exit() function is not expected to return. Ever. On Windows,
we call ExitThread() whose documentation claims: "Ends the calling
thread", i.e. there is no condition in which this function simply
returns: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659

While at it, fix the return type to be void, as per
http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html

Pointed out by Jeff King, helped by Stefan Naewe, Junio Hamano &
Johannes Sixt.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/win32/pthread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Interdiff vs v2:

 diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
 index 148db60..b6ed9e7 100644
 --- a/compat/win32/pthread.h
 +++ b/compat/win32/pthread.h
 @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
  #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
  extern pthread_t pthread_self(void);
  
 -static inline int NORETURN pthread_exit(void *ret)
 +static inline void NORETURN pthread_exit(void *ret)
  {
  	ExitThread((DWORD)(intptr_t)ret);
  }


diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index 20b35a2..b6ed9e7 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
 #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
 extern pthread_t pthread_self(void);
 
-static inline int pthread_exit(void *ret)
+static inline void NORETURN pthread_exit(void *ret)
 {
 	ExitThread((DWORD)(intptr_t)ret);
 }
-- 
2.7.2.windows.1.5.g64acc33

Re: [PATCH v3] Mark win32's pthread_exit() as NORETURN

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The pthread_exit() function is not expected to return. Ever. On Windows,
> we call ExitThread() whose documentation claims: "Ends the calling
> thread", i.e. there is no condition in which this function simply
> returns: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682659
>
> While at it, fix the return type to be void, as per
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_exit.html
>
> Pointed out by Jeff King, helped by Stefan Naewe, Junio Hamano &
> Johannes Sixt.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

Thanks, will queue.

>  compat/win32/pthread.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> Interdiff vs v2:
>
>  diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
>  index 148db60..b6ed9e7 100644
>  --- a/compat/win32/pthread.h
>  +++ b/compat/win32/pthread.h
>  @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
>   #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
>   extern pthread_t pthread_self(void);
>   
>  -static inline int NORETURN pthread_exit(void *ret)
>  +static inline void NORETURN pthread_exit(void *ret)
>   {
>   	ExitThread((DWORD)(intptr_t)ret);
>   }
>
>
> diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
> index 20b35a2..b6ed9e7 100644
> --- a/compat/win32/pthread.h
> +++ b/compat/win32/pthread.h
> @@ -78,7 +78,7 @@ extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
>  #define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
>  extern pthread_t pthread_self(void);
>  
> -static inline int pthread_exit(void *ret)
> +static inline void NORETURN pthread_exit(void *ret)
>  {
>  	ExitThread((DWORD)(intptr_t)ret);
>  }