public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
@ 2022-05-12 12:11 Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 01/11] Remove duplicate stdio initializations Matthew Joyce
                   ` (11 more replies)
  0 siblings, 12 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Hello Corinna,

Per your comments, please see version 2.

1) stdio_atexit is now called stdio_exit_handler
2) declarations for stdio_exit_handler() and _fwalk_sglue() are moved
   to sys/reent.h

Thanks!

Matt

Matt Joyce (9):
  Remove duplicate stdio initializations
  Remove duplicate sglue initializations
  Add two __sglue initialization macros
  Remove __sinit_locks / __sinit_recursive_mutex
  Move __sglue initializations to __sfp()
  Add CLEANUP_FILE define
  Add stdio_exit_handler()
  Add global __sglue object for all configurations
  Remove __sglue member for one configuration

Sebastian Huber (2):
  Declare global __sf[] only once
  stdio: Replace _fwalk_reent() with _fwalk_sglue()

 newlib/libc/include/sys/reent.h |  26 ++++++--
 newlib/libc/reent/reent.c       |   6 +-
 newlib/libc/stdio/fcloseall.c   |   9 ++-
 newlib/libc/stdio/fflush.c      |   2 +-
 newlib/libc/stdio/findfp.c      | 112 +++++++++++++++++---------------
 newlib/libc/stdio/fwalk.c       |  16 ++---
 newlib/libc/stdio/local.h       |   3 -
 newlib/libc/stdio/refill.c      |   4 +-
 newlib/libc/stdlib/exit.c       |   5 +-
 winsup/cygwin/signal.cc         |   5 +-
 winsup/cygwin/syscalls.cc       |   6 +-
 11 files changed, 110 insertions(+), 84 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 01/11] Remove duplicate stdio initializations
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 02/11] Remove duplicate sglue initializations Matthew Joyce
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Removed duplicate stdio initializations from __sinit(). These
are already initialized in the _REENT_INIT macro in sys/reent.h.
This simplification enables the reduction of _GLOBAL_REENT
dependency in a follow-on patch.
---
 newlib/libc/stdio/findfp.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 1370b63b8..ab3c6a55d 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -259,10 +259,6 @@ __sinit (struct _reent *s)
   s->_stdin = __sfp(s);
   s->_stdout = __sfp(s);
   s->_stderr = __sfp(s);
-# else /* _REENT_GLOBAL_STDIO_STREAMS */
-  s->_stdin = &__sf[0];
-  s->_stdout = &__sf[1];
-  s->_stderr = &__sf[2];
 # endif /* _REENT_GLOBAL_STDIO_STREAMS */
 #endif
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 02/11] Remove duplicate sglue initializations
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 01/11] Remove duplicate stdio initializations Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 03/11] Declare global __sf[] only once Matthew Joyce
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Removed duplicate sglue initializations from __sinit(). These
are already initialized in the _REENT_INIT macro in sys/reent.h.
This simplification enables the reduction of _GLOBAL_REENT
dependency in a follow-on patch.
---
 newlib/libc/stdio/findfp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index ab3c6a55d..8327b1992 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -246,15 +246,12 @@ __sinit (struct _reent *s)
   /* make sure we clean up on exit */
   s->__cleanup = cleanup_stdio;	/* conservative */
 
-  s->__sglue._next = NULL;
 #ifndef _REENT_SMALL
 # ifndef _REENT_GLOBAL_STDIO_STREAMS
   s->__sglue._niobs = 3;
   s->__sglue._iobs = &s->__sf[0];
 # endif /* _REENT_GLOBAL_STDIO_STREAMS */
 #else
-  s->__sglue._niobs = 0;
-  s->__sglue._iobs = NULL;
 # ifndef _REENT_GLOBAL_STDIO_STREAMS
   s->_stdin = __sfp(s);
   s->_stdout = __sfp(s);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 03/11] Declare global __sf[] only once
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 01/11] Remove duplicate stdio initializations Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 02/11] Remove duplicate sglue initializations Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 04/11] Add two __sglue initialization macros Matthew Joyce
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Sebastian Huber <sebastian.huber@embedded-brains.de>

Reduced number of global __sf[] declarations from two to one,
simplifying initializations in sys/reent.h.
---
 newlib/libc/include/sys/reent.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 4bc78a447..e0b0ccffb 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -293,6 +293,10 @@ typedef struct __sFILE   __FILE;
 #endif /* __LARGE64_FILES */
 #endif /* !__CUSTOM_FILE_IO__ */
 
+#ifdef _REENT_GLOBAL_STDIO_STREAMS
+extern __FILE __sf[3];
+#endif
+
 struct _glue
 {
   struct _glue *_next;
@@ -426,7 +430,6 @@ struct _reent
 };
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
-extern __FILE __sf[3];
 
 # define _REENT_INIT(var) \
   { 0, \
@@ -698,7 +701,6 @@ struct _reent
 };
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
-extern __FILE __sf[3];
 #define _REENT_STDIO_STREAM(var, index) &__sf[index]
 #else
 #define _REENT_STDIO_STREAM(var, index) &(var)->__sf[index]
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 04/11] Add two __sglue initialization macros
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (2 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 03/11] Declare global __sf[] only once Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex Matthew Joyce
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Added _REENT_INIT_SGLUE and _REENT_INIT_SGLUE_ZEROED macros
to initialize __sglue member of struct _reent. This allows
further simplification of __sinit() and facilitates the removal
of __sglue as a member of struct _reent for certain configurations
in a follow-on patch.
---
 newlib/libc/include/sys/reent.h | 9 ++++++++-
 newlib/libc/stdio/findfp.c      | 7 +------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index e0b0ccffb..4b0d68610 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -702,8 +702,14 @@ struct _reent
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
 #define _REENT_STDIO_STREAM(var, index) &__sf[index]
+#define _REENT_INIT_SGLUE(_ptr) { _NULL, 0, _NULL }
+#define _REENT_INIT_SGLUE_ZEROED(_ptr) /* nothing to set */
 #else
 #define _REENT_STDIO_STREAM(var, index) &(var)->__sf[index]
+#define _REENT_INIT_SGLUE(_ptr) { _NULL, 3, &(_ptr)->__sf[0] }
+#define _REENT_INIT_SGLUE_ZEROED(_ptr) \
+  (_ptr)->__sglue._niobs = 3; \
+  (_ptr)->__sglue._iobs = &(_ptr)->__sf[0];
 #endif
 
 #define _REENT_INIT(var) \
@@ -751,7 +757,7 @@ struct _reent
     }, \
     _REENT_INIT_ATEXIT \
     _NULL, \
-    {_NULL, 0, _NULL} \
+    _REENT_INIT_SGLUE(&(var)) \
   }
 
 #define _REENT_INIT_PTR_ZEROED(var) \
@@ -766,6 +772,7 @@ struct _reent
     (var)->_new._reent._r48._mult[1] = _RAND48_MULT_1; \
     (var)->_new._reent._r48._mult[2] = _RAND48_MULT_2; \
     (var)->_new._reent._r48._add = _RAND48_ADD; \
+    _REENT_INIT_SGLUE_ZEROED(var) \
   }
 
 #define _REENT_CHECK_RAND48(ptr)	/* nothing */
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 8327b1992..66867e664 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -246,12 +246,7 @@ __sinit (struct _reent *s)
   /* make sure we clean up on exit */
   s->__cleanup = cleanup_stdio;	/* conservative */
 
-#ifndef _REENT_SMALL
-# ifndef _REENT_GLOBAL_STDIO_STREAMS
-  s->__sglue._niobs = 3;
-  s->__sglue._iobs = &s->__sf[0];
-# endif /* _REENT_GLOBAL_STDIO_STREAMS */
-#else
+#ifdef _REENT_SMALL
 # ifndef _REENT_GLOBAL_STDIO_STREAMS
   s->_stdin = __sfp(s);
   s->_stdout = __sfp(s);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (3 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 04/11] Add two __sglue initialization macros Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 13:25   ` Torbjorn SVENSSON
  2022-05-12 12:11 ` [PATCH v2 06/11] Move __sglue initializations to __sfp() Matthew Joyce
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Removed __sinit_lock_acquire() and __sinit_lock_release(). Replace these
with __sfp_lock_acquire() and __sfp_lock_release(), respectively. This
eliminates a potential deadlock issue between __sinit() and __sfp().
Removed now unused __sinit_recursive_mutex.
---
 newlib/libc/stdio/findfp.c | 19 +++----------------
 newlib/libc/stdio/local.h  |  2 --
 2 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 66867e664..afbdad9b1 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -235,11 +235,11 @@ cleanup_stdio (struct _reent *ptr)
 void
 __sinit (struct _reent *s)
 {
-  __sinit_lock_acquire ();
+  __sfp_lock_acquire ();
 
   if (s->__cleanup)
     {
-      __sinit_lock_release ();
+      __sfp_lock_release ();
       return;
     }
 
@@ -268,13 +268,12 @@ __sinit (struct _reent *s)
   stderr_init (s->_stderr);
 #endif /* _REENT_GLOBAL_STDIO_STREAMS */
 
-  __sinit_lock_release ();
+  __sfp_lock_release ();
 }
 
 #ifndef __SINGLE_THREAD__
 
 __LOCK_INIT_RECURSIVE(static, __sfp_recursive_mutex);
-__LOCK_INIT_RECURSIVE(static, __sinit_recursive_mutex);
 
 void
 __sfp_lock_acquire (void)
@@ -288,18 +287,6 @@ __sfp_lock_release (void)
   __lock_release_recursive (__sfp_recursive_mutex);
 }
 
-void
-__sinit_lock_acquire (void)
-{
-  __lock_acquire_recursive (__sinit_recursive_mutex);
-}
-
-void
-__sinit_lock_release (void)
-{
-  __lock_release_recursive (__sinit_recursive_mutex);
-}
-
 /* Walkable file locking routine.  */
 static int
 __fp_lock (struct _reent * ptr __unused, FILE * fp)
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index 30c534dcd..9c6f63fdb 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -287,8 +287,6 @@ char *_llicvt (char *, long long, char);
 #else
 void __sfp_lock_acquire (void);
 void __sfp_lock_release (void);
-void __sinit_lock_acquire (void);
-void __sinit_lock_release (void);
 #endif
 
 /* Types used in positional argument support in vfprinf/vfwprintf.
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 06/11] Move __sglue initializations to __sfp()
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (4 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 07/11] Add CLEANUP_FILE define Matthew Joyce
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Moved last remaining  __sglue initializations from __sinit() to
__sfp(). The move better encapsulates access to __sglue and
facilitates its decoupling from struct _reent in a follow-on patch.
---
 newlib/libc/stdio/findfp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index afbdad9b1..7434c343c 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -151,8 +151,14 @@ __sfp (struct _reent *d)
 
   _newlib_sfp_lock_start ();
 
-  if (_GLOBAL_REENT->__cleanup == NULL)
+  if (_GLOBAL_REENT->__cleanup == NULL) {
+#ifdef _REENT_GLOBAL_STDIO_STREAMS
+    _GLOBAL_REENT->__sglue._niobs = 3;
+    _GLOBAL_REENT->__sglue._iobs = &__sf[0];
+#endif
     __sinit (_GLOBAL_REENT);
+  }
+
   for (g = &_GLOBAL_REENT->__sglue;; g = g->_next)
     {
       for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
@@ -256,8 +262,6 @@ __sinit (struct _reent *s)
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
   if (__sf[0]._cookie == NULL) {
-    _GLOBAL_REENT->__sglue._niobs = 3;
-    _GLOBAL_REENT->__sglue._iobs = &__sf[0];
     stdin_init (&__sf[0]);
     stdout_init (&__sf[1]);
     stderr_init (&__sf[2]);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 07/11] Add CLEANUP_FILE define
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (5 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 06/11] Move __sglue initializations to __sfp() Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 08/11] Add stdio_exit_handler() Matthew Joyce
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Define the configuration-dependent constant CLEANUP_FILE for use in
cleanup_stdio(). This will reduce duplicate code during the addition
of a dedicated stdio atexit handler in a follow-on patch.
---
 newlib/libc/stdio/findfp.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 7434c343c..5b0402ac1 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -39,6 +39,21 @@ const struct __sFILE_fake __sf_fake_stderr =
 __FILE __sf[3];
 #endif
 
+#ifdef _STDIO_BSD_SEMANTICS
+  /* BSD and Glibc systems only flush streams which have been written to
+     at exit time.  Calling flush rather than close for speed, as on
+     the aforementioned systems. */
+#define CLEANUP_FILE __sflushw_r
+#else
+  /* Otherwise close files and flush read streams, too.
+     Note we call flush directly if "--enable-lite-exit" is in effect.  */
+#ifdef _LITE_EXIT
+#define CLEANUP_FILE _fflush_r
+#else
+#define CLEANUP_FILE _fclose_r
+#endif
+#endif
+
 #if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED))
 _NOINLINE_STATIC void
 #else
@@ -208,30 +223,15 @@ found:
 static void
 cleanup_stdio (struct _reent *ptr)
 {
-  int (*cleanup_func) (struct _reent *, FILE *);
-#ifdef _STDIO_BSD_SEMANTICS
-  /* BSD and Glibc systems only flush streams which have been written to
-     at exit time.  Calling flush rather than close for speed, as on
-     the aforementioned systems. */
-  cleanup_func = __sflushw_r;
-#else
-  /* Otherwise close files and flush read streams, too.
-     Note we call flush directly if "--enable-lite-exit" is in effect.  */
-#ifdef _LITE_EXIT
-  cleanup_func = _fflush_r;
-#else
-  cleanup_func = _fclose_r;
-#endif
-#endif
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
   if (ptr->_stdin != &__sf[0])
-    (*cleanup_func) (ptr, ptr->_stdin);
+    CLEANUP_FILE (ptr, ptr->_stdin);
   if (ptr->_stdout != &__sf[1])
-    (*cleanup_func) (ptr, ptr->_stdout);
+    CLEANUP_FILE (ptr, ptr->_stdout);
   if (ptr->_stderr != &__sf[2])
-    (*cleanup_func) (ptr, ptr->_stderr);
+    CLEANUP_FILE (ptr, ptr->_stderr);
 #endif
-  (void) _fwalk_reent (ptr, cleanup_func);
+  (void) _fwalk_reent (ptr, CLEANUP_FILE);
 }
 
 /*
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 08/11] Add stdio_exit_handler()
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (6 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 07/11] Add CLEANUP_FILE define Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 19:11   ` Corinna Vinschen
  2022-05-17  8:36   ` Takashi Yano
  2022-05-12 12:11 ` [PATCH v2 09/11] stdio: Replace _fwalk_reent() with _fwalk_sglue() Matthew Joyce
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Added a dedicated stdio exit handler to avoid using _GLOBAL_REENT
in exit().
---
 newlib/libc/include/sys/reent.h |  2 ++
 newlib/libc/stdio/findfp.c      | 11 ++++++++++-
 newlib/libc/stdlib/exit.c       |  5 +++--
 winsup/cygwin/signal.cc         |  5 +++--
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 4b0d68610..067294dc7 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -837,6 +837,8 @@ extern struct _reent *_impure_ptr __ATTRIBUTE_IMPURE_PTR__;
 
 extern struct _reent _impure_data __ATTRIBUTE_IMPURE_DATA__;
 
+extern void (*__stdio_exit_handler) (void);
+
 void _reclaim_reent (struct _reent *);
 
 /* #define _REENT_ONLY define this to get only reentrant routines */
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 5b0402ac1..719886e27 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -26,6 +26,8 @@
 #include <sys/lock.h>
 #include "local.h"
 
+void (*__stdio_exit_handler) (void);
+
 #if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS)
 const struct __sFILE_fake __sf_fake_stdin =
     {_NULL, 0, 0, 0, 0, {_NULL, 0}, 0, _NULL};
@@ -153,6 +155,12 @@ sfmoreglue (struct _reent *d, int n)
   return &g->glue;
 }
 
+static void
+stdio_exit_handler (void)
+{
+  (void) _fwalk_reent (_GLOBAL_REENT, CLEANUP_FILE);
+}
+
 /*
  * Find a free FILE for fopen et al.
  */
@@ -166,12 +174,13 @@ __sfp (struct _reent *d)
 
   _newlib_sfp_lock_start ();
 
-  if (_GLOBAL_REENT->__cleanup == NULL) {
+  if (__stdio_exit_handler == NULL) {
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
     _GLOBAL_REENT->__sglue._niobs = 3;
     _GLOBAL_REENT->__sglue._iobs = &__sf[0];
 #endif
     __sinit (_GLOBAL_REENT);
+    __stdio_exit_handler = stdio_exit_handler;
   }
 
   for (g = &_GLOBAL_REENT->__sglue;; g = g->_next)
diff --git a/newlib/libc/stdlib/exit.c b/newlib/libc/stdlib/exit.c
index 3e618914e..9b7bd518b 100644
--- a/newlib/libc/stdlib/exit.c
+++ b/newlib/libc/stdlib/exit.c
@@ -59,7 +59,8 @@ exit (int code)
 #endif
     __call_exitprocs (code, NULL);
 
-  if (_GLOBAL_REENT->__cleanup)
-    (*_GLOBAL_REENT->__cleanup) (_GLOBAL_REENT);
+  if (__stdio_exit_handler != NULL)
+    (*__stdio_exit_handler) ();
+
   _exit (code);
 }
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index 9b6c2509d..f5f2efdc7 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -13,6 +13,7 @@ details. */
 #include <stdlib.h>
 #include <sys/cygwin.h>
 #include <sys/signalfd.h>
+#include <sys/reent.h> /* needed for __stdio_exit_handler declaration */
 #include "pinfo.h"
 #include "sigproc.h"
 #include "cygtls.h"
@@ -408,8 +409,8 @@ abort (void)
   _my_tls.call_signal_handler (); /* Call any signal handler */
 
   /* Flush all streams as per SUSv2.  */
-  if (_GLOBAL_REENT->__cleanup)
-    _GLOBAL_REENT->__cleanup (_GLOBAL_REENT);
+  if (__stdio_exit_handler != NULL)
+    (*__stdio_exit_handler) ();
   do_exit (SIGABRT);	/* signal handler didn't exit.  Goodbye. */
 }
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 09/11] stdio: Replace _fwalk_reent() with _fwalk_sglue()
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (7 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 08/11] Add stdio_exit_handler() Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 10/11] Add global __sglue object for all configurations Matthew Joyce
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Sebastian Huber <sebastian.huber@embedded-brains.de>

Replaced _fwalk_reent() with _fwalk_sglue(). The change adds an
extra __sglue object as a parameter, which will allow the passing
of a global __sglue object separate from the __sglue member of
struct _reent. The global __sglue object will be added in a
follow-on patch.
---
 newlib/libc/include/sys/reent.h |  3 +++
 newlib/libc/stdio/fcloseall.c   |  2 +-
 newlib/libc/stdio/fflush.c      |  2 +-
 newlib/libc/stdio/findfp.c      | 14 ++++++++++----
 newlib/libc/stdio/fwalk.c       | 16 ++++++++--------
 newlib/libc/stdio/local.h       |  1 -
 newlib/libc/stdio/refill.c      |  4 ++--
 winsup/cygwin/syscalls.cc       |  6 ++----
 8 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 067294dc7..6e420d0c8 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -841,6 +841,9 @@ extern void (*__stdio_exit_handler) (void);
 
 void _reclaim_reent (struct _reent *);
 
+extern int _fwalk_sglue (struct _reent *, int (*)(struct _reent *, __FILE *),
+			                   struct _glue *);
+
 /* #define _REENT_ONLY define this to get only reentrant routines */
 
 #if defined(__DYNAMIC_REENT__) && !defined(__SINGLE_THREAD__)
diff --git a/newlib/libc/stdio/fcloseall.c b/newlib/libc/stdio/fcloseall.c
index 014c451cd..4d4e3554b 100644
--- a/newlib/libc/stdio/fcloseall.c
+++ b/newlib/libc/stdio/fcloseall.c
@@ -59,7 +59,7 @@ Required OS subroutines: <<close>>, <<fstat>>, <<isatty>>, <<lseek>>,
 int
 _fcloseall_r (struct _reent *ptr)
 {
-  return _fwalk_reent (ptr, _fclose_r);
+  return _fwalk_sglue (ptr, _fclose_r, &ptr->__sglue);
 }
 
 #ifndef _REENT_ONLY
diff --git a/newlib/libc/stdio/fflush.c b/newlib/libc/stdio/fflush.c
index 2b5f13bff..6e946da7b 100644
--- a/newlib/libc/stdio/fflush.c
+++ b/newlib/libc/stdio/fflush.c
@@ -286,7 +286,7 @@ int
 fflush (register FILE * fp)
 {
   if (fp == NULL)
-    return _fwalk_reent (_GLOBAL_REENT, _fflush_r);
+    return _fwalk_sglue (_GLOBAL_REENT, _fflush_r, &_GLOBAL_REENT->__sglue);
 
   return _fflush_r (_REENT, fp);
 }
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 719886e27..1627032a3 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -158,7 +158,7 @@ sfmoreglue (struct _reent *d, int n)
 static void
 stdio_exit_handler (void)
 {
-  (void) _fwalk_reent (_GLOBAL_REENT, CLEANUP_FILE);
+  (void) _fwalk_sglue (_GLOBAL_REENT, CLEANUP_FILE, &_GLOBAL_REENT->__sglue);
 }
 
 /*
@@ -240,7 +240,7 @@ cleanup_stdio (struct _reent *ptr)
   if (ptr->_stderr != &__sf[2])
     CLEANUP_FILE (ptr, ptr->_stderr);
 #endif
-  (void) _fwalk_reent (ptr, CLEANUP_FILE);
+  (void) _fwalk_sglue (ptr, CLEANUP_FILE, &ptr->__sglue);
 }
 
 /*
@@ -323,15 +323,21 @@ __fp_unlock (struct _reent * ptr __unused, FILE * fp)
 void
 __fp_lock_all (void)
 {
+  struct _reent *ptr;
+
   __sfp_lock_acquire ();
 
-  (void) _fwalk_reent (_REENT, __fp_lock);
+  ptr = _REENT;
+  (void) _fwalk_sglue (ptr, __fp_lock, &ptr->__sglue);
 }
 
 void
 __fp_unlock_all (void)
 {
-  (void) _fwalk_reent (_REENT, __fp_unlock);
+  struct _reent *ptr;
+
+  ptr = _REENT;
+  (void) _fwalk_sglue (ptr, __fp_unlock, &ptr->__sglue);
 
   __sfp_lock_release ();
 }
diff --git a/newlib/libc/stdio/fwalk.c b/newlib/libc/stdio/fwalk.c
index 2cefcc40e..b1284c6ea 100644
--- a/newlib/libc/stdio/fwalk.c
+++ b/newlib/libc/stdio/fwalk.c
@@ -25,15 +25,13 @@ static char sccsid[] = "%W% (Berkeley) %G%";
 #include <stdio.h>
 #include <stdlib.h>
 #include <errno.h>
-#include "local.h"
 
 int
-_fwalk_reent (struct _reent *ptr,
-       register int (*reent_function) (struct _reent *, FILE *))
+_fwalk_sglue (struct _reent *ptr, int (*func) (struct _reent *, FILE *),
+    struct _glue *g)
 {
-  register FILE *fp;
-  register int n, ret = 0;
-  register struct _glue *g;
+  FILE *fp;
+  int n, ret = 0;
 
   /*
    * It should be safe to walk the list without locking it;
@@ -43,10 +41,12 @@ _fwalk_reent (struct _reent *ptr,
    * Avoid locking this list while walking it or else you will
    * introduce a potential deadlock in [at least] refill.c.
    */
-  for (g = &ptr->__sglue; g != NULL; g = g->_next)
+  do {
     for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
       if (fp->_flags != 0 && fp->_flags != 1 && fp->_file != -1)
-	ret |= (*reent_function) (ptr, fp);
+	ret |= (*func) (ptr, fp);
+    g = g->_next;
+  } while (g != NULL);
 
   return ret;
 }
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index 9c6f63fdb..e245fdb4e 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -182,7 +182,6 @@ extern int    __stextmode (int);
 extern void   __sinit (struct _reent *);
 extern void   __smakebuf_r (struct _reent *, FILE *);
 extern int    __swhatbuf_r (struct _reent *, FILE *, size_t *, int *);
-extern int    _fwalk_reent (struct _reent *, int (*)(struct _reent *, FILE *));
 extern int __submore (struct _reent *, FILE *);
 
 #ifdef __LARGE64_FILES
diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
index 31665bcd9..4084d7250 100644
--- a/newlib/libc/stdio/refill.c
+++ b/newlib/libc/stdio/refill.c
@@ -102,10 +102,10 @@ __srefill_r (struct _reent * ptr,
    */
   if (fp->_flags & (__SLBF | __SNBF))
     {
-      /* Ignore this file in _fwalk_reent to avoid potential deadlock. */
+      /* Ignore this file in _fwalk_sglue to avoid potential deadlock. */
       short orig_flags = fp->_flags;
       fp->_flags = 1;
-      (void) _fwalk_reent (_GLOBAL_REENT, lflush);
+      (void) _fwalk_sglue (_GLOBAL_REENT, lflush, &_GLOBAL_REENT->__sglue);
       fp->_flags = orig_flags;
 
       /* Now flush this file without locking it. */
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 1cecaa017..fb994ac41 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -63,6 +63,7 @@ details. */
 #include "sync.h"
 #include "child_info.h"
 #include <cygwin/fs.h>  /* needed for RENAME_NOREPLACE */
+#include <sys/reent.h>  /* needed for _fwalk_sglue() declaration */
 
 #undef _close
 #undef _lseek
@@ -3057,9 +3058,6 @@ _cygwin_istext_for_stdio (int fd)
   return 1;
 }
 
-/* internal newlib function */
-extern "C" int _fwalk_reent (struct _reent *ptr, int (*function) (struct _reent *, FILE *));
-
 static int
 setmode_helper (struct _reent *ptr __unused, FILE *f)
 {
@@ -3137,7 +3135,7 @@ cygwin_setmode (int fd, int mode)
 	_my_tls.locals.setmode_mode = O_TEXT;
       else
 	_my_tls.locals.setmode_mode = O_BINARY;
-      _fwalk_reent (_GLOBAL_REENT, setmode_helper);
+      _fwalk_sglue (_GLOBAL_REENT, setmode_helper, &_GLOBAL_REENT->__sglue);
     }
   return res;
 }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 10/11] Add global __sglue object for all configurations
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (8 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 09/11] stdio: Replace _fwalk_reent() with _fwalk_sglue() Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 12:11 ` [PATCH v2 11/11] Remove __sglue member for one configuration Matthew Joyce
  2022-05-12 19:14 ` [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Corinna Vinschen
  11 siblings, 0 replies; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Added a new global __sglue object for all configurations.
Decouples the global file object list from the _GLOBAL_REENT
structure by using this new object instead of the __sglue member
of _GLOBAL_REENT in __sfp() and _fwalk_sglue().
---
 newlib/libc/include/sys/reent.h |  2 ++
 newlib/libc/stdio/fcloseall.c   |  2 +-
 newlib/libc/stdio/fflush.c      |  2 +-
 newlib/libc/stdio/findfp.c      | 15 +++++++++------
 newlib/libc/stdio/refill.c      |  2 +-
 winsup/cygwin/syscalls.cc       |  2 +-
 6 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 6e420d0c8..2c0598a5d 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -304,6 +304,8 @@ struct _glue
   __FILE *_iobs;
 };
 
+extern struct _glue __sglue;
+
 /*
  * rand48 family support
  *
diff --git a/newlib/libc/stdio/fcloseall.c b/newlib/libc/stdio/fcloseall.c
index 4d4e3554b..f14c28d34 100644
--- a/newlib/libc/stdio/fcloseall.c
+++ b/newlib/libc/stdio/fcloseall.c
@@ -67,7 +67,7 @@ _fcloseall_r (struct _reent *ptr)
 int
 fcloseall (void)
 {
-  return _fcloseall_r (_GLOBAL_REENT);
+  return _fwalk_sglue (_GLOBAL_REENT, _fclose_r, &__sglue);
 }
 
 #endif
diff --git a/newlib/libc/stdio/fflush.c b/newlib/libc/stdio/fflush.c
index 6e946da7b..bbec4a19b 100644
--- a/newlib/libc/stdio/fflush.c
+++ b/newlib/libc/stdio/fflush.c
@@ -286,7 +286,7 @@ int
 fflush (register FILE * fp)
 {
   if (fp == NULL)
-    return _fwalk_sglue (_GLOBAL_REENT, _fflush_r, &_GLOBAL_REENT->__sglue);
+    return _fwalk_sglue (_GLOBAL_REENT, _fflush_r, &__sglue);
 
   return _fflush_r (_REENT, fp);
 }
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 1627032a3..bf471369c 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -39,6 +39,13 @@ const struct __sFILE_fake __sf_fake_stderr =
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
 __FILE __sf[3];
+struct _glue __sglue = {NULL, 3, &__sf[0]};
+#else
+#ifndef _REENT_SMALL
+struct _glue __sglue = {NULL, 3, &_GLOBAL_REENT->__sf[0]};
+#else
+struct _glue __sglue = {NULL, 0, NULL};
+#endif
 #endif
 
 #ifdef _STDIO_BSD_SEMANTICS
@@ -158,7 +165,7 @@ sfmoreglue (struct _reent *d, int n)
 static void
 stdio_exit_handler (void)
 {
-  (void) _fwalk_sglue (_GLOBAL_REENT, CLEANUP_FILE, &_GLOBAL_REENT->__sglue);
+  (void) _fwalk_sglue (_GLOBAL_REENT, CLEANUP_FILE, &__sglue);
 }
 
 /*
@@ -175,15 +182,11 @@ __sfp (struct _reent *d)
   _newlib_sfp_lock_start ();
 
   if (__stdio_exit_handler == NULL) {
-#ifdef _REENT_GLOBAL_STDIO_STREAMS
-    _GLOBAL_REENT->__sglue._niobs = 3;
-    _GLOBAL_REENT->__sglue._iobs = &__sf[0];
-#endif
     __sinit (_GLOBAL_REENT);
     __stdio_exit_handler = stdio_exit_handler;
   }
 
-  for (g = &_GLOBAL_REENT->__sglue;; g = g->_next)
+  for (g = &__sglue;; g = g->_next)
     {
       for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
 	if (fp->_flags == 0)
diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
index 4084d7250..8516d8576 100644
--- a/newlib/libc/stdio/refill.c
+++ b/newlib/libc/stdio/refill.c
@@ -105,7 +105,7 @@ __srefill_r (struct _reent * ptr,
       /* Ignore this file in _fwalk_sglue to avoid potential deadlock. */
       short orig_flags = fp->_flags;
       fp->_flags = 1;
-      (void) _fwalk_sglue (_GLOBAL_REENT, lflush, &_GLOBAL_REENT->__sglue);
+      (void) _fwalk_sglue (_GLOBAL_REENT, lflush, &__sglue);
       fp->_flags = orig_flags;
 
       /* Now flush this file without locking it. */
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index fb994ac41..bb9df76cf 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -3135,7 +3135,7 @@ cygwin_setmode (int fd, int mode)
 	_my_tls.locals.setmode_mode = O_TEXT;
       else
 	_my_tls.locals.setmode_mode = O_BINARY;
-      _fwalk_sglue (_GLOBAL_REENT, setmode_helper, &_GLOBAL_REENT->__sglue);
+      _fwalk_sglue (_GLOBAL_REENT, setmode_helper, &__sglue);
     }
   return res;
 }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 11/11] Remove __sglue member for one configuration
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (9 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 10/11] Add global __sglue object for all configurations Matthew Joyce
@ 2022-05-12 12:11 ` Matthew Joyce
  2022-05-12 19:09   ` Corinna Vinschen
  2022-05-12 19:14 ` [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Corinna Vinschen
  11 siblings, 1 reply; 27+ messages in thread
From: Matthew Joyce @ 2022-05-12 12:11 UTC (permalink / raw)
  To: newlib

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Removed __sglue member of struct reent when
_REENT_GLOBAL_STDIO_STREAMS is defined.
---
 newlib/libc/include/sys/reent.h | 8 ++++----
 newlib/libc/reent/reent.c       | 6 +++++-
 newlib/libc/stdio/fcloseall.c   | 5 +++++
 newlib/libc/stdio/findfp.c      | 9 ++++++++-
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 2c0598a5d..5cdc67f68 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -696,19 +696,19 @@ struct _reent
   /* These are here last so that __FILE can grow without changing the offsets
      of the above members (on the off chance that future binary compatibility
      would be broken otherwise).  */
-  struct _glue __sglue;		/* root of glue chain */
 # ifndef _REENT_GLOBAL_STDIO_STREAMS
+  struct _glue __sglue;		/* root of glue chain */
   __FILE __sf[3];  		/* first three file descriptors */
 # endif
 };
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
 #define _REENT_STDIO_STREAM(var, index) &__sf[index]
-#define _REENT_INIT_SGLUE(_ptr) { _NULL, 0, _NULL }
+#define _REENT_INIT_SGLUE(_ptr) /* nothing to initialize */
 #define _REENT_INIT_SGLUE_ZEROED(_ptr) /* nothing to set */
 #else
 #define _REENT_STDIO_STREAM(var, index) &(var)->__sf[index]
-#define _REENT_INIT_SGLUE(_ptr) { _NULL, 3, &(_ptr)->__sf[0] }
+#define _REENT_INIT_SGLUE(_ptr) , { _NULL, 3, &(_ptr)->__sf[0] }
 #define _REENT_INIT_SGLUE_ZEROED(_ptr) \
   (_ptr)->__sglue._niobs = 3; \
   (_ptr)->__sglue._iobs = &(_ptr)->__sf[0];
@@ -758,7 +758,7 @@ struct _reent
       } \
     }, \
     _REENT_INIT_ATEXIT \
-    _NULL, \
+    _NULL \
     _REENT_INIT_SGLUE(&(var)) \
   }
 
diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
index 5a04fcf62..70f1c5f45 100644
--- a/newlib/libc/reent/reent.c
+++ b/newlib/libc/reent/reent.c
@@ -27,9 +27,10 @@ int errno;
 
 #endif
 
+#ifndef _REENT_GLOBAL_STDIO_STREAMS
 /* Interim cleanup code */
 
-void
+static void
 cleanup_glue (struct _reent *ptr,
      struct _glue *glue)
 {
@@ -39,6 +40,7 @@ cleanup_glue (struct _reent *ptr,
 
   _free_r (ptr, glue);
 }
+#endif
 
 void
 _reclaim_reent (struct _reent *ptr)
@@ -124,8 +126,10 @@ _reclaim_reent (struct _reent *ptr)
 	     before the program exits, and who wants to wait for that? */
 	  ptr->__cleanup (ptr);
 
+#ifndef _REENT_GLOBAL_STDIO_STREAMS
 	  if (ptr->__sglue._next)
 	    cleanup_glue (ptr, ptr->__sglue._next);
+#endif
 	}
 
       /* Malloc memory not reclaimed; no good way to return memory anyway. */
diff --git a/newlib/libc/stdio/fcloseall.c b/newlib/libc/stdio/fcloseall.c
index f14c28d34..642dc7d94 100644
--- a/newlib/libc/stdio/fcloseall.c
+++ b/newlib/libc/stdio/fcloseall.c
@@ -59,7 +59,12 @@ Required OS subroutines: <<close>>, <<fstat>>, <<isatty>>, <<lseek>>,
 int
 _fcloseall_r (struct _reent *ptr)
 {
+#ifdef _REENT_GLOBAL_STDIO_STREAMS
+  /* There are no thread-specific FILE objects */
+  return 0;
+#else
   return _fwalk_sglue (ptr, _fclose_r, &ptr->__sglue);
+#endif
 }
 
 #ifndef _REENT_ONLY
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index bf471369c..cad50d4e1 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -242,8 +242,9 @@ cleanup_stdio (struct _reent *ptr)
     CLEANUP_FILE (ptr, ptr->_stdout);
   if (ptr->_stderr != &__sf[2])
     CLEANUP_FILE (ptr, ptr->_stderr);
-#endif
+#else
   (void) _fwalk_sglue (ptr, CLEANUP_FILE, &ptr->__sglue);
+#endif
 }
 
 /*
@@ -326,21 +327,27 @@ __fp_unlock (struct _reent * ptr __unused, FILE * fp)
 void
 __fp_lock_all (void)
 {
+#ifndef _REENT_GLOBAL_STDIO_STREAMS
   struct _reent *ptr;
+#endif
 
   __sfp_lock_acquire ();
 
+#ifndef _REENT_GLOBAL_STDIO_STREAMS
   ptr = _REENT;
   (void) _fwalk_sglue (ptr, __fp_lock, &ptr->__sglue);
+#endif
 }
 
 void
 __fp_unlock_all (void)
 {
+#ifndef _REENT_GLOBAL_STDIO_STREAMS
   struct _reent *ptr;
 
   ptr = _REENT;
   (void) _fwalk_sglue (ptr, __fp_unlock, &ptr->__sglue);
+#endif
 
   __sfp_lock_release ();
 }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex
  2022-05-12 12:11 ` [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex Matthew Joyce
@ 2022-05-12 13:25   ` Torbjorn SVENSSON
  2022-05-12 13:51     ` Sebastian Huber
  0 siblings, 1 reply; 27+ messages in thread
From: Torbjorn SVENSSON @ 2022-05-12 13:25 UTC (permalink / raw)
  To: Matthew Joyce, newlib

Hello,

I'm not against this change, but I have some questions related to the change...

What about the retargetable locking scheme?
Should the mutex __sinit_recursive_mutex be removed from newlib/libc/misc/lock.c too?
Without removing the mutex, I suppose there would be a mutex that is never used in the system.
Removing the mutex would pose an API break as I assume that the functions defined in newlib/libc/include/sys/lock.h (and implemented in newlib/libc/misc/lock.c) are to be considered API as it's the functions that the application needs to provide in order to ensure newlib is thread safe.

Does this change force newlib to do a major release?

Kind regards,
Torbjörn

-----Original Message-----
From: Newlib <newlib-bounces+torbjorn.svensson=st.com@sourceware.org> On Behalf Of Matthew Joyce
Sent: den 12 maj 2022 14:12
To: newlib@sourceware.org
Subject: [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex

From: Matt Joyce <matthew.joyce@embedded-brains.de>

Removed __sinit_lock_acquire() and __sinit_lock_release(). Replace these
with __sfp_lock_acquire() and __sfp_lock_release(), respectively. This
eliminates a potential deadlock issue between __sinit() and __sfp().
Removed now unused __sinit_recursive_mutex.
---
 newlib/libc/stdio/findfp.c | 19 +++----------------
 newlib/libc/stdio/local.h  |  2 --
 2 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 66867e664..afbdad9b1 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -235,11 +235,11 @@ cleanup_stdio (struct _reent *ptr)
 void
 __sinit (struct _reent *s)
 {
-  __sinit_lock_acquire ();
+  __sfp_lock_acquire ();
 
   if (s->__cleanup)
     {
-      __sinit_lock_release ();
+      __sfp_lock_release ();
       return;
     }
 
@@ -268,13 +268,12 @@ __sinit (struct _reent *s)
   stderr_init (s->_stderr);
 #endif /* _REENT_GLOBAL_STDIO_STREAMS */
 
-  __sinit_lock_release ();
+  __sfp_lock_release ();
 }
 
 #ifndef __SINGLE_THREAD__
 
 __LOCK_INIT_RECURSIVE(static, __sfp_recursive_mutex);
-__LOCK_INIT_RECURSIVE(static, __sinit_recursive_mutex);
 
 void
 __sfp_lock_acquire (void)
@@ -288,18 +287,6 @@ __sfp_lock_release (void)
   __lock_release_recursive (__sfp_recursive_mutex);
 }
 
-void
-__sinit_lock_acquire (void)
-{
-  __lock_acquire_recursive (__sinit_recursive_mutex);
-}
-
-void
-__sinit_lock_release (void)
-{
-  __lock_release_recursive (__sinit_recursive_mutex);
-}
-
 /* Walkable file locking routine.  */
 static int
 __fp_lock (struct _reent * ptr __unused, FILE * fp)
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index 30c534dcd..9c6f63fdb 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -287,8 +287,6 @@ char *_llicvt (char *, long long, char);
 #else
 void __sfp_lock_acquire (void);
 void __sfp_lock_release (void);
-void __sinit_lock_acquire (void);
-void __sinit_lock_release (void);
 #endif
 
 /* Types used in positional argument support in vfprinf/vfwprintf.
-- 
2.31.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex
  2022-05-12 13:25   ` Torbjorn SVENSSON
@ 2022-05-12 13:51     ` Sebastian Huber
  0 siblings, 0 replies; 27+ messages in thread
From: Sebastian Huber @ 2022-05-12 13:51 UTC (permalink / raw)
  To: Torbjorn SVENSSON, Matthew Joyce, newlib

On 12/05/2022 15:25, Torbjorn SVENSSON via Newlib wrote:
> What about the retargetable locking scheme?
> Should the mutex __sinit_recursive_mutex be removed from newlib/libc/misc/lock.c too?

Yes, we should this dummy mutex.

> Without removing the mutex, I suppose there would be a mutex that is never used in the system.
> Removing the mutex would pose an API break as I assume that the functions defined in newlib/libc/include/sys/lock.h (and implemented in newlib/libc/misc/lock.c) are to be considered API as it's the functions that the application needs to provide in order to ensure newlib is thread safe.

It shouldn't hurt if a lock implementer provides an unused mutex.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 11/11] Remove __sglue member for one configuration
  2022-05-12 12:11 ` [PATCH v2 11/11] Remove __sglue member for one configuration Matthew Joyce
@ 2022-05-12 19:09   ` Corinna Vinschen
  0 siblings, 0 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-12 19:09 UTC (permalink / raw)
  To: newlib

On May 12 14:11, Matthew Joyce wrote:
> From: Matt Joyce <matthew.joyce@embedded-brains.de>
> 
> Removed __sglue member of struct reent when
> _REENT_GLOBAL_STDIO_STREAMS is defined.
> ---
>  newlib/libc/include/sys/reent.h | 8 ++++----
>  newlib/libc/reent/reent.c       | 6 +++++-
>  newlib/libc/stdio/fcloseall.c   | 5 +++++
>  newlib/libc/stdio/findfp.c      | 9 ++++++++-
>  4 files changed, 22 insertions(+), 6 deletions(-)

This patch requires another change in Cygwin.

For some reason, entirely lost in the mist of time, Cygwin exports the
cleanup_glue function.  It already did so when winsup and newlib sources
have been imported into the sourceware CVS repo, back in 2000.

At this point I'd usually say that we can't change that because backward
compatibility and yada yada, but it just doesn't make any sense to export
cleanup_glue, and never did.  At all.

So, instead of reverting cleanup_glue to non-static, let's pull this
through and let's drop it from the list of exported functions.

That means, this patch needs a tiny extension and the commit message
should mention that it's removing cleanup_glue from the list of exported
symbols.

Here's the diff of the required change, please add it to this patch:

diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 63fa09af2553..751ab3754421 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -316,7 +316,6 @@ chroot SIGFE
 cimag NOSIGFE
 cimagf NOSIGFE
 cimagl NOSIGFE
-cleanup_glue NOSIGFE
 clearenv SIGFE
 clearerr SIGFE
 clearerr_unlocked SIGFE
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index ad7fab43f458..6f65a12994bc 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -516,12 +516,13 @@ details. */
   341: Export pthread_cond_clockwait, pthread_mutex_clocklock,
        pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock,
        sem_clockwait, sig2str, str2sig.
+  342: Remove cleanup_glue.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 341
+#define CYGWIN_VERSION_API_MINOR 342
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 08/11] Add stdio_exit_handler()
  2022-05-12 12:11 ` [PATCH v2 08/11] Add stdio_exit_handler() Matthew Joyce
@ 2022-05-12 19:11   ` Corinna Vinschen
  2022-05-17  8:36   ` Takashi Yano
  1 sibling, 0 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-12 19:11 UTC (permalink / raw)
  To: newlib

On May 12 14:11, Matthew Joyce wrote:
> From: Matt Joyce <matthew.joyce@embedded-brains.de>
> 
> Added a dedicated stdio exit handler to avoid using _GLOBAL_REENT
> in exit().
> ---
>  newlib/libc/include/sys/reent.h |  2 ++
>  newlib/libc/stdio/findfp.c      | 11 ++++++++++-
>  newlib/libc/stdlib/exit.c       |  5 +++--
>  winsup/cygwin/signal.cc         |  5 +++--
>  4 files changed, 18 insertions(+), 5 deletions(-)

Just FYI, the winsup/cygwin/signal.cc snippet doesn't apply cleanly
anymore due to Sebastian's patch.  It's just a minor conflict,
nothing to worry about.


Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
                   ` (10 preceding siblings ...)
  2022-05-12 12:11 ` [PATCH v2 11/11] Remove __sglue member for one configuration Matthew Joyce
@ 2022-05-12 19:14 ` Corinna Vinschen
  2022-05-12 19:44   ` Corinna Vinschen
  2022-05-13  8:03   ` Corinna Vinschen
  11 siblings, 2 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-12 19:14 UTC (permalink / raw)
  To: newlib

Hi Matt,

On May 12 14:11, Matthew Joyce wrote:
> From: Matt Joyce <matthew.joyce@embedded-brains.de>
> 
> Hello Corinna,
> 
> Per your comments, please see version 2.
> 
> 1) stdio_atexit is now called stdio_exit_handler
> 2) declarations for stdio_exit_handler() and _fwalk_sglue() are moved
>    to sys/reent.h

Please see my comments in terms of patch 11, which needs an additional
tweak.  With this change, the patchset is GTG.


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-12 19:14 ` [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Corinna Vinschen
@ 2022-05-12 19:44   ` Corinna Vinschen
  2022-05-13  5:44     ` Sebastian Huber
  2022-05-13 10:54     ` Sebastian Huber
  2022-05-13  8:03   ` Corinna Vinschen
  1 sibling, 2 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-12 19:44 UTC (permalink / raw)
  To: newlib

On May 12 21:14, Corinna Vinschen wrote:
> Hi Matt,
> 
> On May 12 14:11, Matthew Joyce wrote:
> > From: Matt Joyce <matthew.joyce@embedded-brains.de>
> > 
> > Hello Corinna,
> > 
> > Per your comments, please see version 2.
> > 
> > 1) stdio_atexit is now called stdio_exit_handler
> > 2) declarations for stdio_exit_handler() and _fwalk_sglue() are moved
> >    to sys/reent.h
> 
> Please see my comments in terms of patch 11, which needs an additional
> tweak.  With this change, the patchset is GTG.

Btw., it looks like Cygwin can switch to _REENT_GLOBAL_STDIO_STREAMS
with only minor changes.  I'll test this a bit more before pushing,
but it's quite neat so far...

diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
index c8352adf9f25..0fd5509d2306 100644
--- a/winsup/cygwin/cygtls.cc
+++ b/winsup/cygwin/cygtls.cc
@@ -55,16 +55,8 @@ _cygtls::init_thread (void *x, DWORD (*func) (void *, void *))
       _REENT_INIT_PTR (&local_clib);
       stackptr = stack;
       altstack.ss_flags = SS_DISABLE;
-      if (_GLOBAL_REENT)
-	{
-	  local_clib._stdin = _GLOBAL_REENT->_stdin;
-	  local_clib._stdout = _GLOBAL_REENT->_stdout;
-	  local_clib._stderr = _GLOBAL_REENT->_stderr;
-	  if (_GLOBAL_REENT->__cleanup)
-	    local_clib.__cleanup = _cygtls::cleanup_early;
-	  local_clib.__sglue._niobs = 3;
-	  local_clib.__sglue._iobs = &_GLOBAL_REENT->__sf[0];
-	}
+      if (_GLOBAL_REENT && _GLOBAL_REENT->__cleanup)
+	local_clib.__cleanup = _cygtls::cleanup_early;
     }
 
   thread_id = GetCurrentThreadId ();
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 5f460d8a5c78..6b816763e002 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -757,9 +757,7 @@ dll_crt0_0 ()
 
   lock_process::init ();
   _impure_ptr = _GLOBAL_REENT;
-  _impure_ptr->_stdin = &_impure_ptr->__sf[0];
-  _impure_ptr->_stdout = &_impure_ptr->__sf[1];
-  _impure_ptr->_stderr = &_impure_ptr->__sf[2];
+  _REENT_INIT_PTR_ZEROED (_GLOBAL_REENT);
   user_data->impure_ptr = _impure_ptr;
   user_data->impure_ptr_ptr = &_impure_ptr;
 
diff --git a/winsup/cygwin/include/cygwin/config.h b/winsup/cygwin/include/cygwin/config.h
index 71a216fbd16d..56fc326bd43e 100644
--- a/winsup/cygwin/include/cygwin/config.h
+++ b/winsup/cygwin/include/cygwin/config.h
@@ -66,6 +66,7 @@ extern inline struct _reent *__getreent (void)
 /* The following block of macros is required to build newlib correctly for
    Cygwin.  Changing them in applications has no or not the desired effect.
    Just leave them alone. */
+#define _REENT_GLOBAL_STDIO_STREAMS 1
 #define _READ_WRITE_RETURN_TYPE _ssize_t
 #define _READ_WRITE_BUFSIZE_TYPE size_t
 #define __LARGE64_FILES 1
diff --git a/winsup/cygwin/tlsoffsets.h b/winsup/cygwin/tlsoffsets.h
[...skipped, just autogenerated data...]
diff --git a/winsup/cygwin/tlsoffsets64.h b/winsup/cygwin/tlsoffsets64.h
[...skipped, just autogenerated data...]


Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-12 19:44   ` Corinna Vinschen
@ 2022-05-13  5:44     ` Sebastian Huber
  2022-05-13  7:53       ` Corinna Vinschen
  2022-05-13 10:54     ` Sebastian Huber
  1 sibling, 1 reply; 27+ messages in thread
From: Sebastian Huber @ 2022-05-13  5:44 UTC (permalink / raw)
  To: newlib

On 12/05/2022 21:44, Corinna Vinschen wrote:
> diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
> index c8352adf9f25..0fd5509d2306 100644
> --- a/winsup/cygwin/cygtls.cc
> +++ b/winsup/cygwin/cygtls.cc
> @@ -55,16 +55,8 @@ _cygtls::init_thread (void *x, DWORD (*func) (void *, void *))
>         _REENT_INIT_PTR (&local_clib);
>         stackptr = stack;
>         altstack.ss_flags = SS_DISABLE;
> -      if (_GLOBAL_REENT)
> -	{
> -	  local_clib._stdin = _GLOBAL_REENT->_stdin;
> -	  local_clib._stdout = _GLOBAL_REENT->_stdout;
> -	  local_clib._stderr = _GLOBAL_REENT->_stderr;
> -	  if (_GLOBAL_REENT->__cleanup)
> -	    local_clib.__cleanup = _cygtls::cleanup_early;
> -	  local_clib.__sglue._niobs = 3;
> -	  local_clib.__sglue._iobs = &_GLOBAL_REENT->__sf[0];
> -	}
> +      if (_GLOBAL_REENT && _GLOBAL_REENT->__cleanup)
> +	local_clib.__cleanup = _cygtls::cleanup_early;

We have:

#define _GLOBAL_REENT (&_impure_data)

So, the NULL pointer check is superfluous.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-13  5:44     ` Sebastian Huber
@ 2022-05-13  7:53       ` Corinna Vinschen
  0 siblings, 0 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-13  7:53 UTC (permalink / raw)
  To: newlib

On May 13 07:44, Sebastian Huber wrote:
> On 12/05/2022 21:44, Corinna Vinschen wrote:
> > diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
> > index c8352adf9f25..0fd5509d2306 100644
> > --- a/winsup/cygwin/cygtls.cc
> > +++ b/winsup/cygwin/cygtls.cc
> > @@ -55,16 +55,8 @@ _cygtls::init_thread (void *x, DWORD (*func) (void *, void *))
> >         _REENT_INIT_PTR (&local_clib);
> >         stackptr = stack;
> >         altstack.ss_flags = SS_DISABLE;
> > -      if (_GLOBAL_REENT)
> > -	{
> > -	  local_clib._stdin = _GLOBAL_REENT->_stdin;
> > -	  local_clib._stdout = _GLOBAL_REENT->_stdout;
> > -	  local_clib._stderr = _GLOBAL_REENT->_stderr;
> > -	  if (_GLOBAL_REENT->__cleanup)
> > -	    local_clib.__cleanup = _cygtls::cleanup_early;
> > -	  local_clib.__sglue._niobs = 3;
> > -	  local_clib.__sglue._iobs = &_GLOBAL_REENT->__sf[0];
> > -	}
> > +      if (_GLOBAL_REENT && _GLOBAL_REENT->__cleanup)
> > +	local_clib.__cleanup = _cygtls::cleanup_early;
> 
> We have:
> 
> #define _GLOBAL_REENT (&_impure_data)
> 
> So, the NULL pointer check is superfluous.

Oh, right, thanks!


Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-12 19:14 ` [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Corinna Vinschen
  2022-05-12 19:44   ` Corinna Vinschen
@ 2022-05-13  8:03   ` Corinna Vinschen
  2022-05-13 11:20     ` Sebastian Huber
  1 sibling, 1 reply; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-13  8:03 UTC (permalink / raw)
  To: Sebastian Huber, Matt Joyce; +Cc: newlib

Hi Sebastian, Hi Matt,

On May 12 21:14, Corinna Vinschen wrote:
> Hi Matt,
> 
> On May 12 14:11, Matthew Joyce wrote:
> > From: Matt Joyce <matthew.joyce@embedded-brains.de>
> > 
> > Hello Corinna,
> > 
> > Per your comments, please see version 2.
> > 
> > 1) stdio_atexit is now called stdio_exit_handler
> > 2) declarations for stdio_exit_handler() and _fwalk_sglue() are moved
> >    to sys/reent.h
> 
> Please see my comments in terms of patch 11, which needs an additional
> tweak.  With this change, the patchset is GTG.

I forgot to mention... you don't have to send a v3, just push the
patchset with this additional tweak (and talk about it in the commit
message).


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-12 19:44   ` Corinna Vinschen
  2022-05-13  5:44     ` Sebastian Huber
@ 2022-05-13 10:54     ` Sebastian Huber
  2022-05-13 11:20       ` Corinna Vinschen
  1 sibling, 1 reply; 27+ messages in thread
From: Sebastian Huber @ 2022-05-13 10:54 UTC (permalink / raw)
  To: newlib

On 12/05/2022 21:44, Corinna Vinschen wrote:
> diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
> index 5f460d8a5c78..6b816763e002 100644
> --- a/winsup/cygwin/dcrt0.cc
> +++ b/winsup/cygwin/dcrt0.cc
> @@ -757,9 +757,7 @@ dll_crt0_0 ()
>   
>     lock_process::init ();
>     _impure_ptr = _GLOBAL_REENT;
> -  _impure_ptr->_stdin = &_impure_ptr->__sf[0];
> -  _impure_ptr->_stdout = &_impure_ptr->__sf[1];
> -  _impure_ptr->_stderr = &_impure_ptr->__sf[2];
> +  _REENT_INIT_PTR_ZEROED (_GLOBAL_REENT);
>     user_data->impure_ptr = _impure_ptr;
>     user_data->impure_ptr_ptr = &_impure_ptr;
>   

The _GLOBAL_REENT == (&_impure_data) is statically initialized in impure.c:

struct _reent __ATTRIBUTE_IMPURE_DATA__ _impure_data = _REENT_INIT 
(_impure_data);

I guess you don't need the _REENT_INIT_PTR_ZEROED (_GLOBAL_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/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-13 10:54     ` Sebastian Huber
@ 2022-05-13 11:20       ` Corinna Vinschen
  0 siblings, 0 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-13 11:20 UTC (permalink / raw)
  To: newlib

On May 13 12:54, Sebastian Huber wrote:
> On 12/05/2022 21:44, Corinna Vinschen wrote:
> > diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
> > index 5f460d8a5c78..6b816763e002 100644
> > --- a/winsup/cygwin/dcrt0.cc
> > +++ b/winsup/cygwin/dcrt0.cc
> > @@ -757,9 +757,7 @@ dll_crt0_0 ()
> >     lock_process::init ();
> >     _impure_ptr = _GLOBAL_REENT;
> > -  _impure_ptr->_stdin = &_impure_ptr->__sf[0];
> > -  _impure_ptr->_stdout = &_impure_ptr->__sf[1];
> > -  _impure_ptr->_stderr = &_impure_ptr->__sf[2];
> > +  _REENT_INIT_PTR_ZEROED (_GLOBAL_REENT);
> >     user_data->impure_ptr = _impure_ptr;
> >     user_data->impure_ptr_ptr = &_impure_ptr;
> 
> The _GLOBAL_REENT == (&_impure_data) is statically initialized in impure.c:
> 
> struct _reent __ATTRIBUTE_IMPURE_DATA__ _impure_data = _REENT_INIT
> (_impure_data);
> 
> I guess you don't need the _REENT_INIT_PTR_ZEROED (_GLOBAL_REENT).

You're right, and more: Given that impure.c also sets _impure_ptr to
&_impure_data statically, setting _impure_ptr to _GLOBAL_REENT is
redundant as well.  Nice.


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-13  8:03   ` Corinna Vinschen
@ 2022-05-13 11:20     ` Sebastian Huber
  2022-05-13 11:30       ` Corinna Vinschen
  0 siblings, 1 reply; 27+ messages in thread
From: Sebastian Huber @ 2022-05-13 11:20 UTC (permalink / raw)
  To: Matt Joyce, newlib

On 13/05/2022 10:03, Corinna Vinschen wrote:
> Hi Sebastian, Hi Matt,
> 
> On May 12 21:14, Corinna Vinschen wrote:
>> Hi Matt,
>>
>> On May 12 14:11, Matthew Joyce wrote:
>>> From: Matt Joyce <matthew.joyce@embedded-brains.de>
>>>
>>> Hello Corinna,
>>>
>>> Per your comments, please see version 2.
>>>
>>> 1) stdio_atexit is now called stdio_exit_handler
>>> 2) declarations for stdio_exit_handler() and _fwalk_sglue() are moved
>>>     to sys/reent.h
>>
>> Please see my comments in terms of patch 11, which needs an additional
>> tweak.  With this change, the patchset is GTG.
> 
> I forgot to mention... you don't have to send a v3, just push the
> patchset with this additional tweak (and talk about it in the commit
> message).

Thanks for the review. I checked it in with hopefully the right 
additional tweaks.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT
  2022-05-13 11:20     ` Sebastian Huber
@ 2022-05-13 11:30       ` Corinna Vinschen
  0 siblings, 0 replies; 27+ messages in thread
From: Corinna Vinschen @ 2022-05-13 11:30 UTC (permalink / raw)
  To: newlib

On May 13 13:20, Sebastian Huber wrote:
> On 13/05/2022 10:03, Corinna Vinschen wrote:
> > Hi Sebastian, Hi Matt,
> > 
> > On May 12 21:14, Corinna Vinschen wrote:
> > > Hi Matt,
> > > 
> > > On May 12 14:11, Matthew Joyce wrote:
> > > > From: Matt Joyce <matthew.joyce@embedded-brains.de>
> > > > 
> > > > Hello Corinna,
> > > > 
> > > > Per your comments, please see version 2.
> > > > 
> > > > 1) stdio_atexit is now called stdio_exit_handler
> > > > 2) declarations for stdio_exit_handler() and _fwalk_sglue() are moved
> > > >     to sys/reent.h
> > > 
> > > Please see my comments in terms of patch 11, which needs an additional
> > > tweak.  With this change, the patchset is GTG.
> > 
> > I forgot to mention... you don't have to send a v3, just push the
> > patchset with this additional tweak (and talk about it in the commit
> > message).
> 
> Thanks for the review. I checked it in with hopefully the right additional
> tweaks.

LGTM.


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 08/11] Add stdio_exit_handler()
  2022-05-12 12:11 ` [PATCH v2 08/11] Add stdio_exit_handler() Matthew Joyce
  2022-05-12 19:11   ` Corinna Vinschen
@ 2022-05-17  8:36   ` Takashi Yano
  2022-05-17 10:13     ` Sebastian Huber
  1 sibling, 1 reply; 27+ messages in thread
From: Takashi Yano @ 2022-05-17  8:36 UTC (permalink / raw)
  To: newlib

On Thu, 12 May 2022 14:11:40 +0200
Matthew Joyce wrote:
> From: Matt Joyce <matthew.joyce@embedded-brains.de>
> 
> Added a dedicated stdio exit handler to avoid using _GLOBAL_REENT
> in exit().
> ---
>  newlib/libc/include/sys/reent.h |  2 ++
>  newlib/libc/stdio/findfp.c      | 11 ++++++++++-
>  newlib/libc/stdlib/exit.c       |  5 +++--
>  winsup/cygwin/signal.cc         |  5 +++--
>  4 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
> index 4b0d68610..067294dc7 100644
> --- a/newlib/libc/include/sys/reent.h
> +++ b/newlib/libc/include/sys/reent.h
> @@ -837,6 +837,8 @@ extern struct _reent *_impure_ptr __ATTRIBUTE_IMPURE_PTR__;
>  
>  extern struct _reent _impure_data __ATTRIBUTE_IMPURE_DATA__;
>  
> +extern void (*__stdio_exit_handler) (void);
> +
>  void _reclaim_reent (struct _reent *);
>  
>  /* #define _REENT_ONLY define this to get only reentrant routines */
> diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
> index 5b0402ac1..719886e27 100644
> --- a/newlib/libc/stdio/findfp.c
> +++ b/newlib/libc/stdio/findfp.c
> @@ -26,6 +26,8 @@
>  #include <sys/lock.h>
>  #include "local.h"
>  
> +void (*__stdio_exit_handler) (void);
> +
>  #if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS)
>  const struct __sFILE_fake __sf_fake_stdin =
>      {_NULL, 0, 0, 0, 0, {_NULL, 0}, 0, _NULL};
> @@ -153,6 +155,12 @@ sfmoreglue (struct _reent *d, int n)
>    return &g->glue;
>  }
>  
> +static void
> +stdio_exit_handler (void)
> +{
> +  (void) _fwalk_reent (_GLOBAL_REENT, CLEANUP_FILE);
> +}
> +
>  /*
>   * Find a free FILE for fopen et al.
>   */
> @@ -166,12 +174,13 @@ __sfp (struct _reent *d)
>  
>    _newlib_sfp_lock_start ();
>  
> -  if (_GLOBAL_REENT->__cleanup == NULL) {
> +  if (__stdio_exit_handler == NULL) {
>  #ifdef _REENT_GLOBAL_STDIO_STREAMS
>      _GLOBAL_REENT->__sglue._niobs = 3;
>      _GLOBAL_REENT->__sglue._iobs = &__sf[0];
>  #endif
>      __sinit (_GLOBAL_REENT);
> +    __stdio_exit_handler = stdio_exit_handler;
>    }
>  
>    for (g = &_GLOBAL_REENT->__sglue;; g = g->_next)
> diff --git a/newlib/libc/stdlib/exit.c b/newlib/libc/stdlib/exit.c
> index 3e618914e..9b7bd518b 100644
> --- a/newlib/libc/stdlib/exit.c
> +++ b/newlib/libc/stdlib/exit.c
> @@ -59,7 +59,8 @@ exit (int code)
>  #endif
>      __call_exitprocs (code, NULL);
>  
> -  if (_GLOBAL_REENT->__cleanup)
> -    (*_GLOBAL_REENT->__cleanup) (_GLOBAL_REENT);
> +  if (__stdio_exit_handler != NULL)
> +    (*__stdio_exit_handler) ();
> +
>    _exit (code);
>  }
> diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
> index 9b6c2509d..f5f2efdc7 100644
> --- a/winsup/cygwin/signal.cc
> +++ b/winsup/cygwin/signal.cc
> @@ -13,6 +13,7 @@ details. */
>  #include <stdlib.h>
>  #include <sys/cygwin.h>
>  #include <sys/signalfd.h>
> +#include <sys/reent.h> /* needed for __stdio_exit_handler declaration */
>  #include "pinfo.h"
>  #include "sigproc.h"
>  #include "cygtls.h"
> @@ -408,8 +409,8 @@ abort (void)
>    _my_tls.call_signal_handler (); /* Call any signal handler */
>  
>    /* Flush all streams as per SUSv2.  */
> -  if (_GLOBAL_REENT->__cleanup)
> -    _GLOBAL_REENT->__cleanup (_GLOBAL_REENT);
> +  if (__stdio_exit_handler != NULL)
> +    (*__stdio_exit_handler) ();
>    do_exit (SIGABRT);	/* signal handler didn't exit.  Goodbye. */
>  }
>  
> -- 
> 2.31.1
> 

After this commit, "ps | cat" outputs nothing in cygwin. However,
just "ps" and "ls | cat" works. "stdbuf -o 0 ps | cat" also works.

I am not sure this is the right thing, however, I found the following
patch solves the issue. It seems that initializing __stdio_exit_handler
is missing.

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 19952d4e0..e759b5402 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -63,6 +63,8 @@ struct _glue __sglue = {NULL, 3, &_GLOBAL_REENT->__sf[0]};
 #endif
 #endif
 
+static void stdio_exit_handler (void);
+
 #if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED))
 _NOINLINE_STATIC void
 #else
@@ -109,6 +111,11 @@ std (FILE *ptr,
   if (__stextmode (ptr->_file))
     ptr->_flags |= __SCLE;
 #endif
+
+  if (__stdio_exit_handler == NULL) {
+    __sinit (_GLOBAL_REENT);
+    __stdio_exit_handler = stdio_exit_handler;
+  }
 }
 
 static inline void

Could you please have a look?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 08/11] Add stdio_exit_handler()
  2022-05-17  8:36   ` Takashi Yano
@ 2022-05-17 10:13     ` Sebastian Huber
  0 siblings, 0 replies; 27+ messages in thread
From: Sebastian Huber @ 2022-05-17 10:13 UTC (permalink / raw)
  To: Takashi Yano, newlib

On 17/05/2022 10:36, Takashi Yano wrote:
[...]
> After this commit, "ps | cat" outputs nothing in cygwin. However,
> just "ps" and "ls | cat" works. "stdbuf -o 0 ps | cat" also works.
> 
> I am not sure this is the right thing, however, I found the following
> patch solves the issue. It seems that initializing __stdio_exit_handler
> is missing.
> 
> diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
> index 19952d4e0..e759b5402 100644
> --- a/newlib/libc/stdio/findfp.c
> +++ b/newlib/libc/stdio/findfp.c
> @@ -63,6 +63,8 @@ struct _glue __sglue = {NULL, 3, &_GLOBAL_REENT->__sf[0]};
>   #endif
>   #endif
>   
> +static void stdio_exit_handler (void);
> +
>   #if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED))
>   _NOINLINE_STATIC void
>   #else
> @@ -109,6 +111,11 @@ std (FILE *ptr,
>     if (__stextmode (ptr->_file))
>       ptr->_flags |= __SCLE;
>   #endif
> +
> +  if (__stdio_exit_handler == NULL) {
> +    __sinit (_GLOBAL_REENT);
> +    __stdio_exit_handler = stdio_exit_handler;
> +  }
>   }
>   
>   static inline void
> 
> Could you please have a look?

Could you please check if this patch fixes the issue:

https://sourceware.org/pipermail/newlib/2022/019713.html

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-05-17 10:13 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12 12:11 [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 01/11] Remove duplicate stdio initializations Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 02/11] Remove duplicate sglue initializations Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 03/11] Declare global __sf[] only once Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 04/11] Add two __sglue initialization macros Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 05/11] Remove __sinit_locks / __sinit_recursive_mutex Matthew Joyce
2022-05-12 13:25   ` Torbjorn SVENSSON
2022-05-12 13:51     ` Sebastian Huber
2022-05-12 12:11 ` [PATCH v2 06/11] Move __sglue initializations to __sfp() Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 07/11] Add CLEANUP_FILE define Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 08/11] Add stdio_exit_handler() Matthew Joyce
2022-05-12 19:11   ` Corinna Vinschen
2022-05-17  8:36   ` Takashi Yano
2022-05-17 10:13     ` Sebastian Huber
2022-05-12 12:11 ` [PATCH v2 09/11] stdio: Replace _fwalk_reent() with _fwalk_sglue() Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 10/11] Add global __sglue object for all configurations Matthew Joyce
2022-05-12 12:11 ` [PATCH v2 11/11] Remove __sglue member for one configuration Matthew Joyce
2022-05-12 19:09   ` Corinna Vinschen
2022-05-12 19:14 ` [PATCH v2 00/11] Decouple global file object list from _GLOBAL_REENT Corinna Vinschen
2022-05-12 19:44   ` Corinna Vinschen
2022-05-13  5:44     ` Sebastian Huber
2022-05-13  7:53       ` Corinna Vinschen
2022-05-13 10:54     ` Sebastian Huber
2022-05-13 11:20       ` Corinna Vinschen
2022-05-13  8:03   ` Corinna Vinschen
2022-05-13 11:20     ` Sebastian Huber
2022-05-13 11:30       ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).