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

[Johannes Schindelin]

Hi,

On Tue, 1 Mar 2016, stefan.na...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Schindelin]

Hi,

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

> Johannes Sixt  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Johannes Sixt  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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);
  }



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

 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 
>> ---
>> 
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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

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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html