Re: [PATCH v2 2/2] newlib/.../getreent.c: Allow to be provided by host and do so for RTEMS

[Joel Sherrill <jrs007 at oarcorp dot com>]

On 10/13/2017 4:15 AM, Corinna Vinschen wrote:
> On Oct 12 20:31, Joel Sherrill wrote:
> > On 10/11/2017 8:54 AM, Joel Sherrill wrote:
> > > Sorry to top post but replying from my phone on a plane.
> > >
> > > Some of our tests are managing to get duplicate symbol errors when linking.
> > >
> > > I put the dummy symbol in to follow the pattern in the file I patterned this after. I would have to look at the source again to see what file that was
> > >
> > >
> > > On Oct 11, 2017, 9:49 AM, at 9:49 AM, Corinna Vinschen <vinschen@redhat.com> wrote:
> > > Hi Joel,
> > >
> > > On Oct 11 07:57, Joel Sherrill wrote:
> > >    RTEMS provides the option to have a global or per-thread reentrancy
> > >    as part of application configuration. As part of this, RTEMS provides
> > >    the implementation of __getreent() as appropriate. Allow the target
> > >    to determine if this method is present in libc.a.
> > >    ---
> > >     newlib/configure.host<http://configure.host>        | 2 +-
> > >     newlib/libc/reent/getreent.c | 8 ++++++++
> > >     2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > >    diff --git a/newlib/configure.host<http://configure.host> b/newlib/configure.host<http://configure.host>
> > >    index fb3362b..7f397dd 100644
> > >    --- a/newlib/configure.host<http://configure.host>
> > >    +++ b/newlib/configure.host<http://configure.host>
> > >    @@ -605,7 +605,7 @@ case "${host}" in
> > >           default_newlib_io_long_long="yes"
> > >           default_newlib_io_c99_formats="yes"
> > >           newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
> > >    -      newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED
> > > -DEXIT_PROVIDED -DSIGNAL_PROVIDED -DREENTRANT_SYSCALLS_PROVIDED
> > > -DHAVE_NANOSLEEP -DHAVE_BLKSIZE -DHAVE_FCNTL -DHAVE_ASSERT_FUNC"
> > >    +newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED
> > > -DEXIT_PROVIDED -DSIGNAL_PROVIDED -DGETREENT_PROVIDED
> > > -DREENTRANT_SYSCALLS_PROVIDED -DHAVE_NANOSLEEP -DHAVE_BLKSIZE
> > > -DHAVE_FCNTL -DHAVE_ASSERT_FUNC"
> > >             # turn off unsupported items in posix directory
> > >           newlib_cflags="${newlib_cflags} -D_NO_GETLOGIN -D_NO_GETPWENT
> > > -D_NO_GETUT -D_NO_GETPASS -D_NO_SIGSET -D_NO_WORDEXP -D_NO_POPEN
> > > -D_NO_POSIX_SPAWN"
> > >           ;;
> > >    diff --git a/newlib/libc/reent/getreent.c
> > > b/newlib/libc/reent/getreent.c
> > >    index 60ae6fb..124abce 100644
> > >    --- a/newlib/libc/reent/getreent.c
> > >    +++ b/newlib/libc/reent/getreent.c
> > >    @@ -1,5 +1,11 @@
> > >     /* default reentrant pointer when multithread enabled */
> > >
> > >    +#ifdef GETREENT_PROVIDED
> > >    +
> > >    +int _dummy_getreent;
> > >    +
> > >
> > > So, why do you need this?  If you implement your own __getreent,
> > > there's
> > > no reason for this _dummy_getreent to exist, no?
> > >
> > > Btw., Cygwin implements its own __getreent(), too, without having to
> > > change newlib/libc/reent/getreent.c at all...

That means there are two ways in which the symbol __getreent() can
be satisfied. One which is correct and one which isn't correct.
For RTEMS, we have tried to have only the one correct version
available across all the libraries.

For some reason, one example program managed to pull in the
incorrect one. This wouldn't happen if it wasn't there.

> > Is it OK to push these patches?
>
> I still need an explanation for _dummy_getreent.  It's kind of ugly.
> Why exaclty is it required?

I modeled this on other files in libc/ including reent/,
stdlib/ and time/. They have a "_dummy_XXX" variable. I vaguely
recall something about this avoids having an object
with no symbols in a library. Do "grep -r _dummy_ ." from
the top of libc and you will see more than 20 examples.

They appear to be for a variety of conditionals like
ABORT_PROVIDED, REENTRANT_SYSCALLS_PROVIDED, NO_EXEC,
SIGNAL_PROVIDED, etc.

--joel


> Corinna