Re: Add global stdio streams support for reent small.

[Douglas <info at ourairquality dot org>]

Hi Jeff,

Updated the comments. The logic for those does not appear to be
consistent in the source code - should it use negative logic on the
#else or repeat the #if logic?  An attempt was made to follow the local
conventions in each file, and if you have a preference for negating the
logic on the #else then just let me know?

Re: findfp.c and the lock initialization. The original logic was:

!defined(__SINGLE_THREAD__) && !defined(_REENT_SMALL)

so the lock was not initialized if:

__SINGLE_THREAD__ || _REENT_SMALL

Then using _REENT_SMALL and the _REENT_GLOBAL_STDIO_STREAMS this lock
needs to be initialized here, so an exception for that was added:

!defined(__SINGLE_THREAD__) && !(defined(_REENT_SMALL) &&
!defined(_REENT_GLOBAL_STDIO_STREAMS))

and the lock is not initialized if:

__SINGLE_THREAD__ || (_REENT_SMALL && !_REENT_GLOBAL_STDIO_STREAMS))

Currently _REENT_GLOBAL_STDIO_STREAMS does not work with _REENT_SMALL so
this change does not appear to break that case - this patch is to fix
the case in which these are both used together.

If I have missed a logic case there could you please point to the
combination of options that it does break so I can see that clearly, thanks.

Douglas

On 03/02/2018 07:22 AM, Jeff Johnston wrote:
> Hi Douglas,
> 
> A few comments.  First of all, please update comments on preprocessor #if,
> #else statements to
> include your changes (e.g. you have added a #else, what does this #else
> mean...e.g. #else /* _REENT_SMALL || _REENT_GLOBAL_STDIO_STREAMS */
> This will aid future maintainers to not have to look all the way back to
> the original #if).  This also applies to
> comments for #else statements that are altered by your changes.
> 
> That said, I have one code comment.
> 
> In findfp.c std, you added "&& !defined(__REENT_GLOBAL_STDIO_STREAMS)" to
> check if the lock for the file should be initialized.  I don't understand
> why you added this.  AFAICT, this will affect current behaviour.
> 
> Regarding slots in the reent structs, yes, they are sometimes there for
> fill, for backwards compatibility.  Otherwise, everything needs to be
> recompiled/relinked.
> 
> -- Jeff J.
> 
> On Wed, Feb 28, 2018 at 7:58 AM, Douglas <info@ourairquality.org> wrote:
> 
>> The global stdio streams option did not appear to work with reent small
>> option, but these two options are both useful on a low memory system.
>>
>> This seems to simplify the reent small code in this area when using the
>> global stdio streams option. The fake streams and some of the checking
>> is not longer necessary.
>>
>> btw the __sf slot in the reent small struct _reent appears to be unused.
>> Could that be removed, or is it fill for backwards compatibility?
>>
> 

>From 314c32471918c65cc2561761bc69fee46e8b2385 Mon Sep 17 00:00:00 2001
From: Our Air Quality <info@ourairquality.org>
Date: Wed, 28 Feb 2018 23:31:48 +1100
Subject: [PATCH] Add global stdio streams support for reent small.

---
 newlib/libc/include/sys/reent.h | 45 ++++++++++++++++++++++++++++++++++++++---
 newlib/libc/stdio/findfp.c      | 16 ++++++++++-----
 newlib/libc/stdio/local.h       |  6 +++---
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 1ef226194..6e55e1c1f 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -144,7 +144,7 @@ struct __sbuf {
  * _ub._base!=NULL) and _up and _ur save the current values of _p and _r.
  */
 
-#ifdef _REENT_SMALL
+#if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS)
 /*
  * struct __sFILE_fake is the start of a struct __sFILE, with only the
  * minimal fields allocated.  In __sinit() we really allocate the 3
@@ -174,9 +174,9 @@ extern void   __sinit (struct _reent *);
 	__sinit (ptr);				\
     }						\
   while (0)
-#else
+#else /* _REENT_SMALL && !_REENT_GLOBAL_STDIO_STREAMS */
 # define _REENT_SMALL_CHECK_INIT(ptr) /* nothing */
-#endif
+#endif /* _REENT_SMALL && !_REENT_GLOBAL_STDIO_STREAMS */
 
 struct __sFILE {
   unsigned char *_p;	/* current position in (some) buffer */
@@ -418,6 +418,43 @@ struct _reent
   char *_signal_buf;                    /* strsignal */
 };
 
+#ifdef _REENT_GLOBAL_STDIO_STREAMS
+extern __FILE __sf[3];
+
+# define _REENT_INIT(var) \
+  { 0, \
+    &__sf[0], \
+    &__sf[1], \
+    &__sf[2], \
+    0,   \
+    _NULL, \
+    0, \
+    0, \
+    _NULL, \
+    _NULL, \
+    _NULL, \
+    0, \
+    0, \
+    _NULL, \
+    _NULL, \
+    _NULL, \
+    _NULL, \
+    _NULL, \
+    _REENT_INIT_ATEXIT \
+    {_NULL, 0, _NULL}, \
+    _NULL, \
+    _NULL, \
+    _NULL \
+  }
+
+#define _REENT_INIT_PTR_ZEROED(var) \
+  { (var)->_stdin = &__sf[0]; \
+    (var)->_stdout = &__sf[1]; \
+    (var)->_stderr = &__sf[2]; \
+  }
+
+#else /* _REENT_GLOBAL_STDIO_STREAMS */
+
 extern const struct __sFILE_fake __sf_fake_stdin;
 extern const struct __sFILE_fake __sf_fake_stdout;
 extern const struct __sFILE_fake __sf_fake_stderr;
@@ -454,6 +491,8 @@ extern const struct __sFILE_fake __sf_fake_stderr;
     (var)->_stderr = (__FILE *)&__sf_fake_stderr; \
   }
 
+#endif /* _REENT_GLOBAL_STDIO_STREAMS */
+
 /* Only add assert() calls if we are specified to debug.  */
 #ifdef _REENT_CHECK_DEBUG
 #include <assert.h>
diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index cf924536f..7119c051b 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -26,7 +26,7 @@
 #include <sys/lock.h>
 #include "local.h"
 
-#ifdef _REENT_SMALL
+#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};
 const struct __sFILE_fake __sf_fake_stdout =
@@ -73,7 +73,7 @@ std (FILE *ptr,
 #else /* _STDIO_CLOSE_STD_STREAMS */
   ptr->_close = NULL;
 #endif /* _STDIO_CLOSE_STD_STREAMS */
-#if !defined(__SINGLE_THREAD__) && !defined(_REENT_SMALL)
+#if !defined(__SINGLE_THREAD__) && !(defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS))
   __lock_init_recursive (ptr->_lock);
   /*
    * #else
@@ -260,7 +260,7 @@ __sinit (struct _reent *s)
 # ifndef _REENT_GLOBAL_STDIO_STREAMS
   s->__sglue._niobs = 3;
   s->__sglue._iobs = &s->__sf[0];
-# endif
+# endif /* _REENT_GLOBAL_STDIO_STREAMS */
 #else
   s->__sglue._niobs = 0;
   s->__sglue._iobs = NULL;
@@ -269,9 +269,15 @@ __sinit (struct _reent *s)
      __sinit if it's 0. */
   if (s == _GLOBAL_REENT)
     s->__sdidinit = 1;
+# ifndef _REENT_GLOBAL_STDIO_STREAMS
   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
 
 #ifdef _REENT_GLOBAL_STDIO_STREAMS
@@ -282,11 +288,11 @@ __sinit (struct _reent *s)
     stdout_init (&__sf[1]);
     stderr_init (&__sf[2]);
   }
-#else
+#else /* _REENT_GLOBAL_STDIO_STREAMS */
   stdin_init (s->_stdin);
   stdout_init (s->_stdout);
   stderr_init (s->_stderr);
-#endif
+#endif /* _REENT_GLOBAL_STDIO_STREAMS */
 
   s->__sdidinit = 1;
 
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index 5f56792de..53694aa1c 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -197,7 +197,7 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *,
 
 /* Called by the main entry point fns to ensure stdio has been initialized.  */
 
-#ifdef _REENT_SMALL
+#if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS)
 #define CHECK_INIT(ptr, fp) \
   do								\
     {								\
@@ -212,7 +212,7 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *,
 	(fp) = _stderr_r(_check_init_ptr);			\
     }								\
   while (0)
-#else /* !_REENT_SMALL   */
+#else /* !_REENT_SMALL || _REENT_GLOBAL_STDIO_STREAMS */
 #define CHECK_INIT(ptr, fp) \
   do								\
     {								\
@@ -221,7 +221,7 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *,
 	__sinit (_check_init_ptr);				\
     }								\
   while (0)
-#endif /* !_REENT_SMALL  */
+#endif /* !_REENT_SMALL || _REENT_GLOBAL_STDIO_STREAMS */
 
 #define CHECK_STD_INIT(ptr) \
   do								\
-- 
2.14.3