From: Matt Joyce <matthew.joyce@embedded-brains.de> The _GLOBAL_REENT structure maintains global information across threads, including a global list of open file objects. This patch set aims to decouple the global file object list from _GLOBAL_REENT, reducing dependency on the structure. This is accomplished by: 1) Simplifying stdio and __sglue initializations 2) Adding a dedicated stdio atexit handler 3) Adding a global __sglue object and removing __sglue from struct _reent for certain configurations This effort will facilitate the future addition of a thread-local storage configuration option as an alternative to using struct _reent. 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 atexit 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 | 21 ++++-- 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 | 15 +++-- newlib/libc/stdio/local.h | 6 +- newlib/libc/stdio/refill.c | 4 +- newlib/libc/stdlib/exit.c | 6 +- winsup/cygwin/signal.cc | 5 +- winsup/cygwin/syscalls.cc | 5 +- 11 files changed, 110 insertions(+), 81 deletions(-) -- 2.31.1
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
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
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
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
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
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
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
From: Matt Joyce <matthew.joyce@embedded-brains.de> Add a dedicated stdio atexit handler to avoid using _GLOBAL_REENT in exit(). --- newlib/libc/stdio/findfp.c | 11 ++++++++++- newlib/libc/stdio/local.h | 1 + newlib/libc/stdlib/exit.c | 6 ++++-- winsup/cygwin/signal.cc | 5 +++-- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c index 5b0402ac1..022a298df 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_atexit) (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_atexit (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_atexit == NULL) { #ifdef _REENT_GLOBAL_STDIO_STREAMS _GLOBAL_REENT->__sglue._niobs = 3; _GLOBAL_REENT->__sglue._iobs = &__sf[0]; #endif __sinit (_GLOBAL_REENT); + __stdio_atexit = stdio_atexit; } for (g = &_GLOBAL_REENT->__sglue;; g = g->_next) diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h index 9c6f63fdb..c892f8378 100644 --- a/newlib/libc/stdio/local.h +++ b/newlib/libc/stdio/local.h @@ -180,6 +180,7 @@ extern _fpos_t __sseek (struct _reent *, void *, _fpos_t, int); extern int __sclose (struct _reent *, void *); extern int __stextmode (int); extern void __sinit (struct _reent *); +extern void (*__stdio_atexit) (void); 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 *)); diff --git a/newlib/libc/stdlib/exit.c b/newlib/libc/stdlib/exit.c index 3e618914e..741cf9cbe 100644 --- a/newlib/libc/stdlib/exit.c +++ b/newlib/libc/stdlib/exit.c @@ -44,6 +44,7 @@ Supporting OS subroutines required: <<_exit>>. #include <unistd.h> /* for _exit() declaration */ #include <reent.h> #include "atexit.h" +#include "../stdio/local.h" /* * Exit, flushing stdio buffers if necessary. @@ -59,7 +60,8 @@ exit (int code) #endif __call_exitprocs (code, NULL); - if (_GLOBAL_REENT->__cleanup) - (*_GLOBAL_REENT->__cleanup) (_GLOBAL_REENT); + if (__stdio_atexit != NULL) + (*__stdio_atexit) (); + _exit (code); } diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc index 9b6c2509d..b7f624cc3 100644 --- a/winsup/cygwin/signal.cc +++ b/winsup/cygwin/signal.cc @@ -22,6 +22,7 @@ details. */ #include "cygheap.h" #include "cygwait.h" #include "posix_timer.h" +#include "../../newlib/libc/stdio/local.h" #define _SA_NORESTART 0x8000 @@ -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_atexit != NULL) + (*__stdio_atexit) (); do_exit (SIGABRT); /* signal handler didn't exit. Goodbye. */ } -- 2.31.1
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/stdio/fcloseall.c | 2 +- newlib/libc/stdio/fflush.c | 2 +- newlib/libc/stdio/findfp.c | 14 ++++++++++---- newlib/libc/stdio/fwalk.c | 15 ++++++++------- newlib/libc/stdio/local.h | 3 ++- newlib/libc/stdio/refill.c | 4 ++-- winsup/cygwin/syscalls.cc | 5 +++-- 7 files changed, 27 insertions(+), 18 deletions(-) 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 022a298df..221052bd4 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_atexit (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..0fdad9d31 100644 --- a/newlib/libc/stdio/fwalk.c +++ b/newlib/libc/stdio/fwalk.c @@ -28,12 +28,11 @@ static char sccsid[] = "%W% (Berkeley) %G%"; #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 +42,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 c892f8378..0c1432d67 100644 --- a/newlib/libc/stdio/local.h +++ b/newlib/libc/stdio/local.h @@ -183,7 +183,8 @@ extern void __sinit (struct _reent *); extern void (*__stdio_atexit) (void); 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 _fwalk_sglue (struct _reent *, int (*)(struct _reent *, FILE *), + struct _glue *); 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..5bf6d8fc7 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -3058,7 +3058,8 @@ _cygwin_istext_for_stdio (int fd) } /* internal newlib function */ -extern "C" int _fwalk_reent (struct _reent *ptr, int (*function) (struct _reent *, FILE *)); +extern "C" int _fwalk_sglue (struct _reent *ptr, + int (*function) (struct _reent *, FILE *), struct _sglue *); static int setmode_helper (struct _reent *ptr __unused, FILE *f) @@ -3137,7 +3138,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
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 4b0d68610..708ec16e2 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 221052bd4..453103e77 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_atexit (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_atexit == NULL) { -#ifdef _REENT_GLOBAL_STDIO_STREAMS - _GLOBAL_REENT->__sglue._niobs = 3; - _GLOBAL_REENT->__sglue._iobs = &__sf[0]; -#endif __sinit (_GLOBAL_REENT); __stdio_atexit = stdio_atexit; } - 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 5bf6d8fc7..caeb5b095 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -3138,7 +3138,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
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 708ec16e2..a738ce3a6 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 453103e77..e1ffa12b9 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
On May 10 10:09, Matthew Joyce wrote:
> From: Matt Joyce <matthew.joyce@embedded-brains.de>
>
> Add a dedicated stdio atexit handler to avoid using _GLOBAL_REENT in exit().
The idea is ok, I would just prefer to use another name. The reason is
that an unsuspecting user could assume that this function actually gets
added to the atexit functions. What about stdio_exit_handler()?
Corinna
On May 10 10:09, Matthew Joyce wrote:
> 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/stdio/fcloseall.c | 2 +-
> newlib/libc/stdio/fflush.c | 2 +-
> newlib/libc/stdio/findfp.c | 14 ++++++++++----
> newlib/libc/stdio/fwalk.c | 15 ++++++++-------
> newlib/libc/stdio/local.h | 3 ++-
> newlib/libc/stdio/refill.c | 4 ++--
> winsup/cygwin/syscalls.cc | 5 +++--
> 7 files changed, 27 insertions(+), 18 deletions(-)
> [...]
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 1cecaa017..5bf6d8fc7 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -3058,7 +3058,8 @@ _cygwin_istext_for_stdio (int fd)
> }
>
> /* internal newlib function */
> -extern "C" int _fwalk_reent (struct _reent *ptr, int (*function) (struct _reent *, FILE *));
> +extern "C" int _fwalk_sglue (struct _reent *ptr,
> + int (*function) (struct _reent *, FILE *), struct _sglue *);
^^^^^^
_glue?
Corinna
On May 11 17:23, Corinna Vinschen wrote:
> On May 10 10:09, Matthew Joyce wrote:
> > From: Matt Joyce <matthew.joyce@embedded-brains.de>
> >
> > Add a dedicated stdio atexit handler to avoid using _GLOBAL_REENT in exit().
>
> The idea is ok, I would just prefer to use another name. The reason is
> that an unsuspecting user could assume that this function actually gets
> added to the atexit functions. What about stdio_exit_handler()?
Btw., this produces a build problem in Cygwin:
winsup/cygwin/signal.cc:179:1: error: conflicting declaration of C function ‘unsigned int usleep(useconds_t)’
179 | usleep (useconds_t useconds)
| ^~~~~~
In file included from libc/include/unistd.h:4,
from winsup/cygwin/../../newlib/libc/stdio/local.h:29,
from winsup/cygwin/signal.cc:25:
libc/include/sys/unistd.h:240:9: note: previous declaration ‘int usleep(useconds_t)’
240 | int usleep (useconds_t __useconds);
| ^~~~~~
Corinna
On 11/05/2022 18:54, Corinna Vinschen wrote: > On May 11 17:23, Corinna Vinschen wrote: >> On May 10 10:09, Matthew Joyce wrote: >>> From: Matt Joyce<matthew.joyce@embedded-brains.de> >>> >>> Add a dedicated stdio atexit handler to avoid using _GLOBAL_REENT in exit(). >> The idea is ok, I would just prefer to use another name. The reason is >> that an unsuspecting user could assume that this function actually gets >> added to the atexit functions. What about stdio_exit_handler()? > Btw., this produces a build problem in Cygwin: > > winsup/cygwin/signal.cc:179:1: error: conflicting declaration of C function ‘unsigned int usleep(useconds_t)’ > 179 | usleep (useconds_t useconds) > | ^~~~~~ > In file included from libc/include/unistd.h:4, > from winsup/cygwin/../../newlib/libc/stdio/local.h:29, > from winsup/cygwin/signal.cc:25: > libc/include/sys/unistd.h:240:9: note: previous declaration ‘int usleep(useconds_t)’ > 240 | int usleep (useconds_t __useconds); > | ^~~~~~ Should this be fixed in signal.cc? usleep() should return int. -- 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/
On 11/05/2022 17:23, Corinna Vinschen wrote: > On May 10 10:09, Matthew Joyce wrote: >> From: Matt Joyce<matthew.joyce@embedded-brains.de> >> >> Add a dedicated stdio atexit handler to avoid using _GLOBAL_REENT in exit(). > The idea is ok, I would just prefer to use another name. The reason is > that an unsuspecting user could assume that this function actually gets > added to the atexit functions. What about stdio_exit_handler()? Ok, sounds good. -- 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/
On 11/05/2022 17:56, Corinna Vinschen wrote: > On May 10 10:09, Matthew Joyce wrote: >> 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/stdio/fcloseall.c | 2 +- >> newlib/libc/stdio/fflush.c | 2 +- >> newlib/libc/stdio/findfp.c | 14 ++++++++++---- >> newlib/libc/stdio/fwalk.c | 15 ++++++++------- >> newlib/libc/stdio/local.h | 3 ++- >> newlib/libc/stdio/refill.c | 4 ++-- >> winsup/cygwin/syscalls.cc | 5 +++-- >> 7 files changed, 27 insertions(+), 18 deletions(-) >> [...] >> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc >> index 1cecaa017..5bf6d8fc7 100644 >> --- a/winsup/cygwin/syscalls.cc >> +++ b/winsup/cygwin/syscalls.cc >> @@ -3058,7 +3058,8 @@ _cygwin_istext_for_stdio (int fd) >> } >> >> /* internal newlib function */ >> -extern "C" int _fwalk_reent (struct _reent *ptr, int (*function) (struct _reent *, FILE *)); >> +extern "C" int _fwalk_sglue (struct _reent *ptr, >> + int (*function) (struct _reent *, FILE *), struct _sglue *); > ^^^^^^ > _glue? In patch 8, Matthew added this: diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc index 9b6c2509d..b7f624cc3 100644 --- a/winsup/cygwin/signal.cc +++ b/winsup/cygwin/signal.cc @@ -22,6 +22,7 @@ details. */ #include "cygheap.h" #include "cygwait.h" #include "posix_timer.h" +#include "../../newlib/libc/stdio/local.h" This seems to work in the cygwin build. Should we remove the local declaration and instead include the local.h header file? -- 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/
On May 11 19:55, Sebastian Huber wrote:
> On 11/05/2022 17:56, Corinna Vinschen wrote:
> > On May 10 10:09, Matthew Joyce wrote:
> > > 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/stdio/fcloseall.c | 2 +-
> > > newlib/libc/stdio/fflush.c | 2 +-
> > > newlib/libc/stdio/findfp.c | 14 ++++++++++----
> > > newlib/libc/stdio/fwalk.c | 15 ++++++++-------
> > > newlib/libc/stdio/local.h | 3 ++-
> > > newlib/libc/stdio/refill.c | 4 ++--
> > > winsup/cygwin/syscalls.cc | 5 +++--
> > > 7 files changed, 27 insertions(+), 18 deletions(-)
> > > [...]
> > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> > > index 1cecaa017..5bf6d8fc7 100644
> > > --- a/winsup/cygwin/syscalls.cc
> > > +++ b/winsup/cygwin/syscalls.cc
> > > @@ -3058,7 +3058,8 @@ _cygwin_istext_for_stdio (int fd)
> > > }
> > > /* internal newlib function */
> > > -extern "C" int _fwalk_reent (struct _reent *ptr, int (*function) (struct _reent *, FILE *));
> > > +extern "C" int _fwalk_sglue (struct _reent *ptr,
> > > + int (*function) (struct _reent *, FILE *), struct _sglue *);
> > ^^^^^^
> > _glue?
>
> In patch 8, Matthew added this:
>
> diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
> index 9b6c2509d..b7f624cc3 100644
> --- a/winsup/cygwin/signal.cc
> +++ b/winsup/cygwin/signal.cc
> @@ -22,6 +22,7 @@ details. */
> #include "cygheap.h"
> #include "cygwait.h"
> #include "posix_timer.h"
> +#include "../../newlib/libc/stdio/local.h"
>
> This seems to work in the cygwin build. Should we remove the local
> declaration and instead include the local.h header file?
Actually, no. In fact, we should avoid direct access to private
newlib headers from Cygwin entirely.
As for the above error, it's just a typo and struct _glue is
defined in sys/reeent anyway.
As for definitions of __stdio_atexit (resp. stdio_exit_handler) or
_fwalk_sglue... all definitions within newlib accessible from Cygwin
should be in a system header, for instance sys/reeent or sys/stdio.h.
Thanks,
Corinna