what's up with _COMPILING_NEWLIB

Message ID YYcb752EA71AhfF3@vapier
State New
Headers show
Series
  • what's up with _COMPILING_NEWLIB
Related show

Commit Message

Mike Frysinger Nov. 7, 2021, 12:21 a.m.
i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol
that indicates the code currently being compiled is newlib itself so that the
header can change behavior for that environment specifically.  is that what
it's meant for ?

if so, why does it seem to be inconsistently defined ?  newlib/configure.host
will add it for a few random targets, as does the mips-specific
newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/
files.  it feels like the patch below is what we should have.

if that's not what this is for, is there a define that has this meaning ?
in the glibc & gnulib world, the plain _LIBC define indicates this.
-mike

Comments

Corinna Vinschen Nov. 8, 2021, 10:05 a.m. | #1
On Nov  6 20:21, Mike Frysinger wrote:
> i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol

> that indicates the code currently being compiled is newlib itself so that the

> header can change behavior for that environment specifically.  is that what

> it's meant for ?

> 

> if so, why does it seem to be inconsistently defined ?  newlib/configure.host

> will add it for a few random targets, as does the mips-specific

> newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/

> files.  it feels like the patch below is what we should have.

> 

> if that's not what this is for, is there a define that has this meaning ?

> in the glibc & gnulib world, the plain _LIBC define indicates this.


_COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need
it during build.  Your patch looks good to me, did you test it for some
targets?


Thanks,
Corinna
Mike Frysinger Nov. 8, 2021, 11:46 a.m. | #2
On 08 Nov 2021 11:05, Corinna Vinschen wrote:
> On Nov  6 20:21, Mike Frysinger wrote:

> > i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol

> > that indicates the code currently being compiled is newlib itself so that the

> > header can change behavior for that environment specifically.  is that what

> > it's meant for ?

> > 

> > if so, why does it seem to be inconsistently defined ?  newlib/configure.host

> > will add it for a few random targets, as does the mips-specific

> > newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/

> > files.  it feels like the patch below is what we should have.

> > 

> > if that's not what this is for, is there a define that has this meaning ?

> > in the glibc & gnulib world, the plain _LIBC define indicates this.

> 

> _COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need

> it during build.  Your patch looks good to me, did you test it for some

> targets?


yes, i tested it for bfin-elf and with a change that needed it in ctype.h.
the ctype.h change didn't work until i updated the build this way.
-mike
Corinna Vinschen Nov. 8, 2021, 3:05 p.m. | #3
On Nov  8 06:46, Mike Frysinger wrote:
> On 08 Nov 2021 11:05, Corinna Vinschen wrote:

> > On Nov  6 20:21, Mike Frysinger wrote:

> > > i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol

> > > that indicates the code currently being compiled is newlib itself so that the

> > > header can change behavior for that environment specifically.  is that what

> > > it's meant for ?

> > > 

> > > if so, why does it seem to be inconsistently defined ?  newlib/configure.host

> > > will add it for a few random targets, as does the mips-specific

> > > newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/

> > > files.  it feels like the patch below is what we should have.

> > > 

> > > if that's not what this is for, is there a define that has this meaning ?

> > > in the glibc & gnulib world, the plain _LIBC define indicates this.

> > 

> > _COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need

> > it during build.  Your patch looks good to me, did you test it for some

> > targets?

> 

> yes, i tested it for bfin-elf and with a change that needed it in ctype.h.

> the ctype.h change didn't work until i updated the build this way.


uhm... why does a change in a header file depend on the build system?
That sounds weird.

I tested building on Cygwin, which looks good.

So, here's a question: The patch is ok, just a change to the commit
message would be nice.  However, would you like to take the opportunity
to change _COMPILING_NEWLIB to _LIBC throughout?  That's something we
should have done long ago, I guess.


Thanks,
Corinna
Mike Frysinger Nov. 8, 2021, 6:14 p.m. | #4
On 08 Nov 2021 16:05, Corinna Vinschen wrote:
> On Nov  8 06:46, Mike Frysinger wrote:

> > On 08 Nov 2021 11:05, Corinna Vinschen wrote:

> > > On Nov  6 20:21, Mike Frysinger wrote:

> > > > i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol

> > > > that indicates the code currently being compiled is newlib itself so that the

> > > > header can change behavior for that environment specifically.  is that what

> > > > it's meant for ?

> > > > 

> > > > if so, why does it seem to be inconsistently defined ?  newlib/configure.host

> > > > will add it for a few random targets, as does the mips-specific

> > > > newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/

> > > > files.  it feels like the patch below is what we should have.

> > > > 

> > > > if that's not what this is for, is there a define that has this meaning ?

> > > > in the glibc & gnulib world, the plain _LIBC define indicates this.

> > > 

> > > _COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need

> > > it during build.  Your patch looks good to me, did you test it for some

> > > targets?

> > 

> > yes, i tested it for bfin-elf and with a change that needed it in ctype.h.

> > the ctype.h change didn't work until i updated the build this way.

> 

> uhm... why does a change in a header file depend on the build system?

> That sounds weird.


i'm making a fix to ctype.h that would utilize _COMPILING_NEWLIB as part of
its fix.  i was holding off posting that until the question of this symbol
could be sorted out.

> I tested building on Cygwin, which looks good.

> 

> So, here's a question: The patch is ok, just a change to the commit

> message would be nice.  However, would you like to take the opportunity

> to change _COMPILING_NEWLIB to _LIBC throughout?  That's something we

> should have done long ago, I guess.


let me do it as a series.  _LIBC is already used in newlib, so i'm afraid
it might not be as clean as just renaming the define.  but i'll give it a
shot and see what blows up.
-mike

Patch

--- a/newlib/configure.host
+++ b/newlib/configure.host
@@ -54,7 +54,7 @@ 
 #   have_init_fini	have init/fini ("yes" or "no", set to "yes" by default)
 #   noinclude		list of include files to not install
 
-newlib_cflags=
+newlib_cflags="-D_COMPILING_NEWLIB"
 libm_machine_dir=
 machine_dir=
 shared_machine_dir=
@@ -467,15 +467,11 @@  case "${host}" in
 	sys_dir=a29khif
 	signal_dir=
 	;;
-  aarch64*-*-*)
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
-	;;
   amdgcn*)
 	sys_dir=amdgcn
 	have_crt0="no"
 	;;
   arm*-*-*)
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
 	sys_dir=arm
 	if [ "x${newlib_may_supply_syscalls}" = "xno" ] ; then
 	  have_crt0="no"
@@ -652,11 +648,11 @@  case "${host}" in
 	default_newlib_io_long_double="yes"
 	default_newlib_io_pos_args="yes"
 	CC="${CC} -I${cygwin_srcdir}/include"
-	newlib_cflags="${newlib_cflags} -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED"
+	newlib_cflags="${newlib_cflags} -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED"
 	syscall_dir=syscalls
 	;;
   *-*-phoenix*)
-	newlib_cflags="${newlib_cflags} -DMISSING_SYSCALL_NAMES -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_NANOSLEEP"
+	newlib_cflags="${newlib_cflags} -DMISSING_SYSCALL_NAMES -DHAVE_BLKSIZE -DHAVE_NANOSLEEP"
 	default_newlib_io_long_long="yes"
 	syscall_dir=
 	;;
@@ -671,7 +667,6 @@  case "${host}" in
 	default_newlib_io_long_long="yes"
 	default_newlib_io_c99_formats="yes"
 	newlib_cflags="${newlib_cflags} -ffunction-sections -fdata-sections "
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
 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"
--- a/newlib/libc/machine/mips/Makefile.am
+++ b/newlib/libc/machine/mips/Makefile.am
@@ -9,8 +9,6 @@  AM_CCASFLAGS = $(INCLUDES)
 noinst_LIBRARIES = lib.a
 
 lib_a_SOURCES = setjmp.S strlen.c strcmp.S strncpy.c memset.S memcpy.S
-lib_a_CCASFLAGS=$(AM_CCASFLAGS) -D_COMPILING_NEWLIB
-lib_a_CFLAGS=$(AM_CFLAGS) -D_COMPILING_NEWLIB
 
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
--- a/newlib/libc/machine/mips/Makefile.in
+++ b/newlib/libc/machine/mips/Makefile.in
@@ -198,8 +198,6 @@  INCLUDES = $(NEWLIB_CFLAGS) $(CROSS_CFLAGS) $(TARGET_CFLAGS)
 AM_CCASFLAGS = $(INCLUDES)
 noinst_LIBRARIES = lib.a
 lib_a_SOURCES = setjmp.S strlen.c strcmp.S strncpy.c memset.S memcpy.S
-lib_a_CCASFLAGS = $(AM_CCASFLAGS) -D_COMPILING_NEWLIB
-lib_a_CFLAGS = $(AM_CFLAGS) -D_COMPILING_NEWLIB
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
 all: all-am