[PATCH v3 2/5] Add stdin_init(), stdout_init() and stderr_init()

[PATCH v3 2/5] Add stdin_init(), stdout_init() and stderr_init()

[Sebastian Huber]

This simplifies further changes in this area.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/libc/stdio/findfp.c | 50 ++++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index ecc65d6d3..b40aa9240 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -84,6 +84,36 @@ _DEFUN(std, (ptr, flags, file),
 #endif
 }
 
+static inline void
+stdin_init(FILE *ptr)
+{
+  std (ptr,  __SRD, 0);
+}
+
+static inline void
+stdout_init(FILE *ptr)
+{
+  /* On platforms that have true file system I/O, we can verify
+     whether stdout is an interactive terminal or not, as part of
+     __smakebuf on first use of the stream.  For all other platforms,
+     we will default to line buffered mode here.  Technically, POSIX
+     requires both stdin and stdout to be line-buffered, but tradition
+     leaves stdin alone on systems without fcntl.  */
+#ifdef HAVE_FCNTL
+  std (ptr, __SWR, 1);
+#else
+  std (ptr, __SWR | __SLBF, 1);
+#endif
+}
+
+static inline void
+stderr_init(FILE *ptr)
+{
+  /* POSIX requires stderr to be opened for reading and writing, even
+     when the underlying fd 2 is write-only.  */
+  std (ptr, __SRW | __SNBF, 2);
+}
+
 struct glue_with_file {
   struct _glue glue;
   FILE file;
@@ -235,23 +265,9 @@ _DEFUN(__sinit, (s),
   s->_stderr = __sfp(s);
 #endif
 
-  std (s->_stdin,  __SRD, 0);
-
-  /* On platforms that have true file system I/O, we can verify
-     whether stdout is an interactive terminal or not, as part of
-     __smakebuf on first use of the stream.  For all other platforms,
-     we will default to line buffered mode here.  Technically, POSIX
-     requires both stdin and stdout to be line-buffered, but tradition
-     leaves stdin alone on systems without fcntl.  */
-#ifdef HAVE_FCNTL
-  std (s->_stdout, __SWR, 1);
-#else
-  std (s->_stdout, __SWR | __SLBF, 1);
-#endif
-
-  /* POSIX requires stderr to be opened for reading and writing, even
-     when the underlying fd 2 is write-only.  */
-  std (s->_stderr, __SRW | __SNBF, 2);
+  stdin_init (s->_stdin);
+  stdout_init (s->_stdout);
+  stderr_init (s->_stderr);
 
   s->__sdidinit = 1;
 
-- 
2.12.3

[PATCH v3 1/5] Remove superfluous parameter from std()

[Sebastian Huber]

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/libc/stdio/findfp.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 83d3dc558..ecc65d6d3 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -40,11 +40,10 @@ _NOINLINE_STATIC _VOID
 #else
 static _VOID
 #endif
-_DEFUN(std, (ptr, flags, file, data),
+_DEFUN(std, (ptr, flags, file),
             FILE *ptr _AND
             int flags _AND
-            int file  _AND
-            struct _reent *data)
+            int file)
 {
   ptr->_p = 0;
   ptr->_r = 0;
@@ -236,7 +235,7 @@ _DEFUN(__sinit, (s),
   s->_stderr = __sfp(s);
 #endif
 
-  std (s->_stdin,  __SRD, 0, s);
+  std (s->_stdin,  __SRD, 0);
 
   /* On platforms that have true file system I/O, we can verify
      whether stdout is an interactive terminal or not, as part of
@@ -245,14 +244,14 @@ _DEFUN(__sinit, (s),
      requires both stdin and stdout to be line-buffered, but tradition
      leaves stdin alone on systems without fcntl.  */
 #ifdef HAVE_FCNTL
-  std (s->_stdout, __SWR, 1, s);
+  std (s->_stdout, __SWR, 1);
 #else
-  std (s->_stdout, __SWR | __SLBF, 1, s);
+  std (s->_stdout, __SWR | __SLBF, 1);
 #endif
 
   /* POSIX requires stderr to be opened for reading and writing, even
      when the underlying fd 2 is write-only.  */
-  std (s->_stderr, __SRW | __SNBF, 2, s);
+  std (s->_stderr, __SRW | __SNBF, 2);
 
   s->__sdidinit = 1;
 
-- 
2.12.3

[PATCH v3 4/5] Enable _REENT_GLOBAL_STDIO_STREAMS for RTEMS

[Sebastian Huber]

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/libc/include/sys/config.h | 1 +
 newlib/libc/stdio/local.h        | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/config.h b/newlib/libc/include/sys/config.h
index 555239f8b..ae8caff7b 100644
--- a/newlib/libc/include/sys/config.h
+++ b/newlib/libc/include/sys/config.h
@@ -238,6 +238,7 @@
 #define _READ_WRITE_RETURN_TYPE _ssize_t
 #define __DYNAMIC_REENT__
 #define _REENT_GLOBAL_ATEXIT
+#define _REENT_GLOBAL_STDIO_STREAMS
 #endif
 
 #ifndef __EXPORT
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index 5f6995501..511e5e35f 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -38,7 +38,7 @@
    case _STDIO_CLOSE_PER_REENT_STD_STREAMS is defined these file descriptors
    will be closed via close() provided the owner of the reent structure
    triggerd the on demand reent initilization, see CHECK_INIT(). */
-#if !defined(__rtems__) && !defined(__tirtos__)
+#if !defined(__tirtos__)
 #define _STDIO_CLOSE_PER_REENT_STD_STREAMS
 #endif
 
-- 
2.12.3

[PATCH v3 5/5] Add --enable-newlib-global-stdio-streams

[Sebastian Huber]

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/README                    |  6 ++++++
 newlib/configure                 | 24 ++++++++++++++++++++++--
 newlib/configure.in              | 13 +++++++++++++
 newlib/libc/include/sys/config.h |  6 ++++++
 newlib/newlib.hin                |  5 +++++
 5 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/newlib/README b/newlib/README
index ed8fb3540..78f4de846 100644
--- a/newlib/README
+++ b/newlib/README
@@ -297,6 +297,12 @@ One feature can be enabled by specifying `--enable-FEATURE=yes' or
      is not referenced.
      Disabled by default.
 
+`--enable-newlib-global-stdio-streams'
+     Enable to move the stdio stream FILE objects out of struct _reent and make
+     them global.  The stdio stream pointers of struct _reent are initialized
+     to point to the global stdio FILE stream objects.
+     Disabled by default.
+
 `--enable-newlib-reent-small'
      Enable small reentrant struct support.
      Disabled by default.
diff --git a/newlib/configure b/newlib/configure
index de28c25b3..b2f0b3340 100755
--- a/newlib/configure
+++ b/newlib/configure
@@ -795,6 +795,7 @@ enable_newlib_iconv_external_ccs
 enable_newlib_atexit_dynamic_alloc
 enable_newlib_global_atexit
 enable_newlib_reent_small
+enable_newlib_global_stdio_streams
 enable_newlib_fvwrite_in_streamio
 enable_newlib_fseek_optimization
 enable_newlib_wide_orient
@@ -1467,6 +1468,7 @@ Optional Features:
   --disable-newlib-atexit-dynamic-alloc    disable dynamic allocation of atexit entries
   --enable-newlib-global-atexit	enable atexit data structure as global
   --enable-newlib-reent-small   enable small reentrant struct support
+  --enable-newlib-global-stdio-streams   enable global stdio streams
   --disable-newlib-fvwrite-in-streamio    disable iov in streamio
   --disable-newlib-fseek-optimization    disable fseek optimization
   --disable-newlib-wide-orient    Turn off wide orientation in streamio
@@ -2387,6 +2389,17 @@ else
   newlib_reent_small=
 fi
 
+# Check whether --enable-newlib-global-stdio-streams was given.
+if test "${enable_newlib_global_stdio_streams+set}" = set; then :
+  enableval=$enable_newlib_global_stdio_streams; case "${enableval}" in
+  yes) newlib_global_stdio_streams=yes;;
+  no)  newlib_global_stdio_streams=no ;;
+  *)   as_fn_error $? "bad value ${enableval} for newlib-global-stdio-streams option" "$LINENO" 5 ;;
+ esac
+else
+  newlib_global_stdio_streams=
+fi
+
 # Check whether --enable-newlib-fvwrite-in-streamio was given.
 if test "${enable_newlib_fvwrite_in_streamio+set}" = set; then :
   enableval=$enable_newlib_fvwrite_in_streamio; if test "${newlib_fvwrite_in_streamio+set}" != set; then
@@ -11794,7 +11807,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11797 "configure"
+#line 11810 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11900,7 +11913,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11903 "configure"
+#line 11916 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12347,6 +12360,13 @@ _ACEOF
 
 fi
 
+if test "${newlib_global_stdio_streams}" = "yes"; then
+cat >>confdefs.h <<_ACEOF
+#define _WANT_REENT_GLOBAL_STDIO_STREAMS 1
+_ACEOF
+
+fi
+
 if test "${newlib_mb}" = "yes"; then
 cat >>confdefs.h <<_ACEOF
 #define _MB_CAPABLE 1
diff --git a/newlib/configure.in b/newlib/configure.in
index 354c07ca5..5b86ee800 100644
--- a/newlib/configure.in
+++ b/newlib/configure.in
@@ -136,6 +136,15 @@ AC_ARG_ENABLE(newlib-reent-small,
   no)  newlib_reent_small=no ;;
   *)   AC_MSG_ERROR(bad value ${enableval} for newlib-reent-small option) ;;
  esac], [newlib_reent_small=])dnl
+
+dnl Support --enable-newlib-global-stdio-streams
+AC_ARG_ENABLE(newlib-global-stdio-streams,
+[  --enable-newlib-global-stdio-streams   enable global stdio streams],
+[case "${enableval}" in
+  yes) newlib_global_stdio_streams=yes;;
+  no)  newlib_global_stdio_streams=no ;;
+  *)   AC_MSG_ERROR(bad value ${enableval} for newlib-global-stdio-streams option) ;;
+ esac], [newlib_global_stdio_streams=])dnl
  
 dnl Support --disable-newlib-fvwrite-in-streamio
 AC_ARG_ENABLE(newlib-fvwrite-in-streamio,
@@ -400,6 +409,10 @@ if test "${newlib_reent_small}" = "yes"; then
 AC_DEFINE_UNQUOTED(_WANT_REENT_SMALL)
 fi
 
+if test "${newlib_global_stdio_streams}" = "yes"; then
+AC_DEFINE_UNQUOTED(_WANT_REENT_GLOBAL_STDIO_STREAMS)
+fi
+
 if test "${newlib_mb}" = "yes"; then
 AC_DEFINE_UNQUOTED(_MB_CAPABLE)
 AC_DEFINE_UNQUOTED(_MB_LEN_MAX,8)
diff --git a/newlib/libc/include/sys/config.h b/newlib/libc/include/sys/config.h
index ae8caff7b..e45aa5417 100644
--- a/newlib/libc/include/sys/config.h
+++ b/newlib/libc/include/sys/config.h
@@ -276,6 +276,12 @@
 #endif
 #endif
 
+#ifdef _WANT_REENT_GLOBAL_STDIO_STREAMS
+#ifndef _REENT_GLOBAL_STDIO_STREAMS
+#define _REENT_GLOBAL_STDIO_STREAMS
+#endif
+#endif
+
 /* If _MB_EXTENDED_CHARSETS_ALL is set, we want all of the extended
    charsets.  The extended charsets add a few functions and a couple
    of tables of a few K each. */
diff --git a/newlib/newlib.hin b/newlib/newlib.hin
index 397bc9b96..45c683187 100644
--- a/newlib/newlib.hin
+++ b/newlib/newlib.hin
@@ -79,6 +79,11 @@
 /* Define if declare atexit data as global.  */
 #undef _REENT_GLOBAL_ATEXIT
 
+/* Define to move the stdio stream FILE objects out of struct _reent and make
+   them global.  The stdio stream pointers of struct _reent are initialized to
+   point to the global stdio FILE stream objects. */
+#undef _WANT_REENT_GLOBAL_STDIO_STREAMS
+
 /* Define if small footprint nano-formatted-IO implementation used.  */
 #undef _NANO_FORMATTED_IO
 
-- 
2.12.3

[PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Sebastian Huber]

In Newlib, the stdio streams are defined to thread-specific pointers
_reent::_stdin, _reent::_stdout and _reent::_stderr.  In case
_REENT_SMALL is not defined, then these pointers are initialized via
_REENT_INIT_PTR() or _REENT_INIT_PTR_ZEROED() to thread-specific FILE
objects provided via _reent::__sf[3].  There are two problems with this
(at least in case of RTEMS).

(1) The thread-specific FILE objects are closed by _reclaim_reent().
This leads to problems with language run-time libraries that provide
wrappers to the C/POSIX stdio streams (e.g.  C++ and Ada), since they
use the thread-specific FILE objects of the initialization thread.  In
case the initialization thread is deleted, then they use freed memory.

(2) Since thread-specific FILE objects are used with a common output
device via file descriptors 0, 1 and 2, the locking at FILE object level
cannot ensure atomicity of the output, e.g. a call to printf().

Introduce a new Newlib configuration option _REENT_GLOBAL_STDIO_STREAMS
to enable the use of global stdio FILE objects.

As a side-effect this reduces the size of struct _reent by more than
50%.

The _REENT_GLOBAL_STDIO_STREAMS should not be used without
_STDIO_CLOSE_PER_REENT_STD_STREAMS.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/libc/include/sys/reent.h | 21 +++++++++++++++------
 newlib/libc/stdio/findfp.c      | 24 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 8b67889ac..c045ca549 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -644,14 +644,23 @@ struct _reent
      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
   __FILE __sf[3];  		/* first three file descriptors */
+# endif
 };
 
+#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]
+#endif
+
 #define _REENT_INIT(var) \
   { 0, \
-    &(var).__sf[0], \
-    &(var).__sf[1], \
-    &(var).__sf[2], \
+    _REENT_STDIO_STREAM(&(var), 0), \
+    _REENT_STDIO_STREAM(&(var), 1), \
+    _REENT_STDIO_STREAM(&(var), 2), \
     0, \
     "", \
     0, \
@@ -696,9 +705,9 @@ struct _reent
   }
 
 #define _REENT_INIT_PTR_ZEROED(var) \
-  { (var)->_stdin = &(var)->__sf[0]; \
-    (var)->_stdout = &(var)->__sf[1]; \
-    (var)->_stderr = &(var)->__sf[2]; \
+  { (var)->_stdin = _REENT_STDIO_STREAM(var, 0); \
+    (var)->_stdout = _REENT_STDIO_STREAM(var, 1); \
+    (var)->_stderr = _REENT_STDIO_STREAM(var, 2); \
     (var)->_new._reent._rand_next = 1; \
     (var)->_new._reent._r48._seed[0] = _RAND48_SEED_0; \
     (var)->_new._reent._r48._seed[1] = _RAND48_SEED_1; \
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index b40aa9240..737bde102 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -35,6 +35,10 @@ const struct __sFILE_fake __sf_fake_stderr =
     {_NULL, 0, 0, 0, 0, {_NULL, 0}, 0, _NULL};
 #endif
 
+#ifdef _REENT_GLOBAL_STDIO_STREAMS
+__FILE __sf[3];
+#endif
+
 #if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED))
 _NOINLINE_STATIC _VOID
 #else
@@ -218,6 +222,14 @@ _DEFUN(_cleanup_r, (ptr),
   cleanup_func = _fclose_r;
 #endif
 #endif
+#ifdef _REENT_GLOBAL_STDIO_STREAMS
+  if (ptr->_stdin != &__sf[0])
+    (*cleanup_func) (ptr, ptr->_stdin);
+  if (ptr->_stdout != &__sf[1])
+    (*cleanup_func) (ptr, ptr->_stdout);
+  if (ptr->_stderr != &__sf[2])
+    (*cleanup_func) (ptr, ptr->_stderr);
+#endif
   _CAST_VOID _fwalk_reent (ptr, cleanup_func);
 }
 
@@ -250,8 +262,10 @@ _DEFUN(__sinit, (s),
 
   s->__sglue._next = NULL;
 #ifndef _REENT_SMALL
+# ifndef _REENT_GLOBAL_STDIO_STREAMS
   s->__sglue._niobs = 3;
   s->__sglue._iobs = &s->__sf[0];
+# endif
 #else
   s->__sglue._niobs = 0;
   s->__sglue._iobs = NULL;
@@ -265,9 +279,19 @@ _DEFUN(__sinit, (s),
   s->_stderr = __sfp(s);
 #endif
 
+#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]);
+  }
+#else
   stdin_init (s->_stdin);
   stdout_init (s->_stdout);
   stderr_init (s->_stderr);
+#endif
 
   s->__sdidinit = 1;
 
-- 
2.12.3

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]

On Jun 29 14:20, Sebastian Huber wrote:
> In Newlib, the stdio streams are defined to thread-specific pointers
> _reent::_stdin, _reent::_stdout and _reent::_stderr.  In case
> _REENT_SMALL is not defined, then these pointers are initialized via
> _REENT_INIT_PTR() or _REENT_INIT_PTR_ZEROED() to thread-specific FILE
> objects provided via _reent::__sf[3].  There are two problems with this
> (at least in case of RTEMS).
> 
> (1) The thread-specific FILE objects are closed by _reclaim_reent().
> This leads to problems with language run-time libraries that provide
> wrappers to the C/POSIX stdio streams (e.g.  C++ and Ada), since they
> use the thread-specific FILE objects of the initialization thread.  In
> case the initialization thread is deleted, then they use freed memory.
> 
> (2) Since thread-specific FILE objects are used with a common output
> device via file descriptors 0, 1 and 2, the locking at FILE object level
> cannot ensure atomicity of the output, e.g. a call to printf().
> 
> Introduce a new Newlib configuration option _REENT_GLOBAL_STDIO_STREAMS
> to enable the use of global stdio FILE objects.
> 
> As a side-effect this reduces the size of struct _reent by more than
> 50%.
> 
> The _REENT_GLOBAL_STDIO_STREAMS should not be used without
> _STDIO_CLOSE_PER_REENT_STD_STREAMS.

Patch series looks good, builds and works on Cygwin, so I think this
is good to go.  Please push.

And, JFYI, Cygwin will start to use it too after the next release :)


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Sebastian Huber]

On 29/06/17 20:31, Corinna Vinschen wrote:

> And, JFYI, Cygwin will start to use it too after the next release:)

There are some new problems with this change. We have no reference 
counting in the FILE objects, so a freopen(..., stdin), closes the 
global stdin FILE object (__sf[0]), etc. What works is a stdin = 
fopen(). I guess this could break existing applications.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On Jun 30 07:43, Sebastian Huber wrote:
> On 29/06/17 20:31, Corinna Vinschen wrote:
> 
> > And, JFYI, Cygwin will start to use it too after the next release:)
> 
> There are some new problems with this change. We have no reference counting
> in the FILE objects, so a freopen(..., stdin), closes the global stdin FILE
> object (__sf[0]), etc. What works is a stdin = fopen(). I guess this could
> break existing applications.

Erm... isn't that expected behaviour?  stdin/stdout/stderr are global
objects, after all.  They were never thread-local per POSIX.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Sebastian Huber]

----- Am 30. Jun 2017 um 10:13 schrieb Corinna Vinschen vinschen@redhat.com:

> On Jun 30 07:43, Sebastian Huber wrote:
>> On 29/06/17 20:31, Corinna Vinschen wrote:
>> 
>> > And, JFYI, Cygwin will start to use it too after the next release:)
>> 
>> There are some new problems with this change. We have no reference counting
>> in the FILE objects, so a freopen(..., stdin), closes the global stdin FILE
>> object (__sf[0]), etc. What works is a stdin = fopen(). I guess this could
>> break existing applications.
> 
> Erm... isn't that expected behaviour?  stdin/stdout/stderr are global
> objects, after all.  They were never thread-local per POSIX.

The stdin/stdout/stderr pointers are still thread-local with this option. Only the FILE objects itself are now global. If you do a stdin = fopen() you get a completely thread-local stdin. I have absolutely no idea why this stuff is thread-local in Newlib by default.

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

On Jun 30 10:58, Sebastian Huber wrote:
> 
> 
> ----- Am 30. Jun 2017 um 10:13 schrieb Corinna Vinschen vinschen@redhat.com:
> 
> > On Jun 30 07:43, Sebastian Huber wrote:
> >> On 29/06/17 20:31, Corinna Vinschen wrote:
> >> 
> >> > And, JFYI, Cygwin will start to use it too after the next release:)
> >> 
> >> There are some new problems with this change. We have no reference counting
> >> in the FILE objects, so a freopen(..., stdin), closes the global stdin FILE
> >> object (__sf[0]), etc. What works is a stdin = fopen(). I guess this could
> >> break existing applications.
> > 
> > Erm... isn't that expected behaviour?  stdin/stdout/stderr are global
> > objects, after all.  They were never thread-local per POSIX.
> 
> The stdin/stdout/stderr pointers are still thread-local with this
> option. Only the FILE objects itself are now global. If you do a stdin
> = fopen() you get a completely thread-local stdin. I have absolutely
> no idea why this stuff is thread-local in Newlib by default.

Me neither.  This predates my involvement in newlib.

I guess it sounded like a good idea at the time, to allow multiple,
independent applets on bare metal systems.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Freddie Chopin]

On Fri, 2017-06-30 at 10:58 +0200, Sebastian Huber wrote:
> 
> ----- Am 30. Jun 2017 um 10:13 schrieb Corinna Vinschen 
> vinschen@redhat.com:
> 
> > On Jun 30 07:43, Sebastian Huber wrote:
> > > On 29/06/17 20:31, Corinna Vinschen wrote:
> > > 
> > > > And, JFYI, Cygwin will start to use it too after the next
> > > > release:)
> > > 
> > > There are some new problems with this change. We have no
> > > reference counting
> > > in the FILE objects, so a freopen(..., stdin), closes the global
> > > stdin FILE
> > > object (__sf[0]), etc. What works is a stdin = fopen(). I guess
> > > this could
> > > break existing applications.
> > 
> > Erm... isn't that expected behaviour?  stdin/stdout/stderr are
> > global
> > objects, after all.  They were never thread-local per POSIX.
> 
> The stdin/stdout/stderr pointers are still thread-local with this
> option. Only the FILE objects itself are now global. If you do a
> stdin = fopen() you get a completely thread-local stdin. I have
> absolutely no idea why this stuff is thread-local in Newlib by
> default.

Would that be possible, to make stdin/stdout/stderr completely global
(remove them from _reent, use global objects) when
_REENT_GLOBAL_STDIO_STREAMS is enabled?

Regards,
FCh

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Sebastian Huber]

On 08/08/18 15:19, Freddie Chopin wrote:
> On Fri, 2017-06-30 at 10:58 +0200, Sebastian Huber wrote:
>> ----- Am 30. Jun 2017 um 10:13 schrieb Corinna Vinschen
>> vinschen@redhat.com:
>>
>>> On Jun 30 07:43, Sebastian Huber wrote:
>>>> On 29/06/17 20:31, Corinna Vinschen wrote:
>>>>
>>>>> And, JFYI, Cygwin will start to use it too after the next
>>>>> release:)
>>>> There are some new problems with this change. We have no
>>>> reference counting
>>>> in the FILE objects, so a freopen(..., stdin), closes the global
>>>> stdin FILE
>>>> object (__sf[0]), etc. What works is a stdin = fopen(). I guess
>>>> this could
>>>> break existing applications.
>>> Erm... isn't that expected behaviour?  stdin/stdout/stderr are
>>> global
>>> objects, after all.  They were never thread-local per POSIX.
>> The stdin/stdout/stderr pointers are still thread-local with this
>> option. Only the FILE objects itself are now global. If you do a
>> stdin = fopen() you get a completely thread-local stdin. I have
>> absolutely no idea why this stuff is thread-local in Newlib by
>> default.
> Would that be possible, to make stdin/stdout/stderr completely global
> (remove them from _reent, use global objects) when
> _REENT_GLOBAL_STDIO_STREAMS is enabled?

The thread-local IO streams are a Newlib feature that is used by our 
applications.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Freddie Chopin]

On Wed, 2018-08-08 at 16:15 +0200, Sebastian Huber wrote:
> The thread-local IO streams are a Newlib feature that is used by our 
> applications.

Don't get me wrong, but I would expect an option with "global" in the
name to make the streams really global, not "sort-of-global" (;
Otherwise it looks like a half-baked-hack to fix just some particular
problem instead of looking at the whole picture.

Regards,
FCh

Re: [PATCH v3 3/5] Introduce _REENT_GLOBAL_STDIO_STREAMS

[Sebastian Huber]

On 08/08/18 17:20, Freddie Chopin wrote:
> On Wed, 2018-08-08 at 16:15 +0200, Sebastian Huber wrote:
>> The thread-local IO streams are a Newlib feature that is used by our
>> applications.
> Don't get me wrong, but I would expect an option with "global" in the
> name to make the streams really global, not "sort-of-global" (;
> Otherwise it looks like a half-baked-hack to fix just some particular
> problem instead of looking at the whole picture.

There are two things involved here. The FILE objects and pointers to 
FILE objects. The stdio, etc. are pointers. These pointers are 
thread-local. The FILE objects are global and are used for the initial 
values of these thread-local pointers. If you want to get rid of the 
thread-local pointers, then you probably need a new configuration option.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.