Re: [PATCH 1/1] Make __sdidinit unused

Hi Matt,

this doesn't build on Cygwin.

On Feb 17 14:05, Matthew Joyce wrote:
> Remove dependency on __sdidinit member of struct _reent to check
> object initialization. Like __sdidinit, the __cleanup member of
> struct _reent is initialized in the __sinit() function. Checking
> initialization against __cleanup serves the same purpose and will
> reduce overhead in the __sfp() function in a follow up patch.
> ---
> [...]
> diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
> index 1a2213d1f..239a9d7e1 100644
> --- a/winsup/cygwin/cygtls.cc
> +++ b/winsup/cygwin/cygtls.cc
> @@ -60,8 +60,8 @@ _cygtls::init_thread (void *x, DWORD (*func) (void *, void *))
>  	  local_clib._stdin = _GLOBAL_REENT->_stdin;
>  	  local_clib._stdout = _GLOBAL_REENT->_stdout;
>  	  local_clib._stderr = _GLOBAL_REENT->_stderr;
> -	  local_clib.__sdidinit = _GLOBAL_REENT->__sdidinit ? -1 : 0;
> -	  local_clib.__cleanup = _GLOBAL_REENT->__cleanup;
> +	  local_clib.__cleanup = _GLOBAL_REENT->__cleanup ?
> +	    (void *)(uintptr_t)-1 : NULL;

  CXX      cygtls.o
winsup/cygwin/cygtls.cc: In member function ‘void _cygtls::init_thread(void*, DWORD (*)(void*, void*))’:
winsup/cygwin/cygtls.cc:63:59: error: invalid conversion from ‘void*’ to ‘void (*)(_reent*)’ [-fpermissive]
   63 |           local_clib.__cleanup = _GLOBAL_REENT->__cleanup ?
      |                                                           ^
      |                                                           |
      |                                                           void*

> diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
> index f3d09c169..16ca5888c 100644
> --- a/winsup/cygwin/dcrt0.cc
> +++ b/winsup/cygwin/dcrt0.cc
> @@ -824,15 +824,13 @@ main_thread_sinit ()
> [...]
> -     to _REENT here again. */
> -  _REENT->__sdidinit = -1;
> -  _REENT->__cleanup = _GLOBAL_REENT->__cleanup;
> +     To fix this issue we set __cleanup to -1 here. */
> +  _REENT->__cleanup = (void *)(uintptr_t)-1;

winsup/cygwin/dcrt0.cc: In function ‘void main_thread_sinit()’:
winsup/cygwin/dcrt0.cc:833:23: error: invalid conversion from ‘void*’ to ‘void (*)(_reent*)’ [-fpermissive]
  833 |   _REENT->__cleanup = (void *)(uintptr_t)-1;
      |                       ^~~~~~~~~~~~~~~~~~~~~
      |                       |
      |                       void*

I suggest this change, which also includes a matching change to
thread.cc, to be type-safe:

diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
index 239a9d7e1f51..e80fbce5f3e8 100644
--- a/winsup/cygwin/cygtls.cc
+++ b/winsup/cygwin/cygtls.cc
@@ -60,8 +60,9 @@ _cygtls::init_thread (void *x, DWORD (*func) (void *, void *))
 	  local_clib._stdin = _GLOBAL_REENT->_stdin;
 	  local_clib._stdout = _GLOBAL_REENT->_stdout;
 	  local_clib._stderr = _GLOBAL_REENT->_stderr;
-	  local_clib.__cleanup = _GLOBAL_REENT->__cleanup ?
-	    (void *)(uintptr_t)-1 : NULL;
+	  local_clib.__cleanup = (void (*) (struct _reent *))
+				 (_GLOBAL_REENT->__cleanup
+				  ? (void *) -1 : NULL);
 	  local_clib.__sglue._niobs = 3;
 	  local_clib.__sglue._iobs = &_GLOBAL_REENT->__sf[0];
 	}
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 16ca5888c28a..7f26c6304030 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -830,7 +830,7 @@ main_thread_sinit ()
      read or written in the first stdio function call in the main thread.
 
      To fix this issue we set __cleanup to -1 here. */
-  _REENT->__cleanup = (void *)(uintptr_t)-1;
+  _REENT->__cleanup = (void (*) (struct _reent *)) -1;
 }
 
 /* Take over from libc's crt0.o and start the application. Note the
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 3c3a2f3b3a52..21b2dbe4686e 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -564,7 +564,7 @@ pthread::exit (void *value_ptr)
       mutex.unlock ();
     }
 
-  if (_my_tls.local_clib.__cleanup == (void *)(uintptr_t)-1)
+  if (_my_tls.local_clib.__cleanup == (void (*) (struct _reent *)) -1)
     _my_tls.local_clib.__cleanup = NULL;
   _reclaim_reent (_REENT);
 
With these changes, GTG.


Thx,
Corinna