[PATCH v2 1/1] Make __sdidinit unused
Sebastian Huber
sebastian.huber@embedded-brains.de
Fri Feb 18 16:21:21 GMT 2022
Hello Corinna,
On 18/02/2022 13:41, Corinna Vinschen wrote:
> Hi Matt,
>
> On Feb 18 10:45, 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.
>
> The patch looks right now. But.
>
> What exactly are you going to do in __sfp? The reason I'm asking is
> that it's absolutely not clear yet which purpose this change serves.
> Looking at this patch, I only see that, rather than using an available
> flag, a pointer is now set to an invalid value -1, which looks more
> dangerous than what we did before.
>
> Care to explain or, even better, send the patch you have in mind?
this is related to our attempt to use individual thread-local storage
objects instead of the monolithic struct _reent:
https://sourceware.org/pipermail/newlib/2022/018855.html
It turned out that this is not possible while _GLOBAL_REENT exists.
Getting rid of struct _reent members with similar functionality helps to
refactor the code.
Instead of using -1 as a magic value a safer option would be to assign a
dummy function which does nothing:
diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
index 6ce1202ef..c8352adf9 100644
--- a/winsup/cygwin/cygtls.cc
+++ b/winsup/cygwin/cygtls.cc
@@ -60,9 +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.__cleanup = (void (*) (struct _reent *))
- (_GLOBAL_REENT->__cleanup
- ? (void *) -1 : NULL);
+ if (_GLOBAL_REENT->__cleanup)
+ local_clib.__cleanup = _cygtls::cleanup_early;
local_clib.__sglue._niobs = 3;
local_clib.__sglue._iobs = &_GLOBAL_REENT->__sf[0];
}
@@ -150,6 +149,12 @@ _cygtls::remove (DWORD wait)
}
}
+void
+_cygtls::cleanup_early (struct _reent *)
+{
+ /* Do nothing */
+}
+
#ifdef __x86_64__
void san::leave ()
{
diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
index a2e3676fc..c2c4141bf 100644
--- a/winsup/cygwin/cygtls.h
+++ b/winsup/cygwin/cygtls.h
@@ -272,6 +272,7 @@ public:
will_wait_for_signal = false;
}
void handle_SIGCONT ();
+ static void cleanup_early(struct _reent *);
private:
void __reg3 call2 (DWORD (*) (void *, void *), void *, void *);
void remove_pending_sigs ();
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 7f26c6304..e757c47ec 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -829,8 +829,8 @@ main_thread_sinit ()
wrong spot. The input or output buffer will be NULLed and nothing is
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 (*) (struct _reent *)) -1;
+ To fix this issue we set __cleanup to _cygtls::cleanup_early here. */
+ _REENT->__cleanup = _cygtls::cleanup_early;
}
/* 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 21b2dbe46..b7da4d0c7 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 (*) (struct _reent *)) -1)
+ if (_my_tls.local_clib.__cleanup == _cygtls::cleanup_early)
_my_tls.local_clib.__cleanup = NULL;
_reclaim_reent (_REENT);
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
More information about the Newlib
mailing list