Re: [PATCH 09/11] stdio: Replace _fwalk_reent() with _fwalk_sglue()

[Corinna Vinschen <vinschen@redhat.com>]

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