Re: [PATCH] Automated the generation of the __NEWLIB__, __NEWLIB_MINOR__ and __NEWLIB_PATCHLEVEL__ macros.

[Pieter du Preez <pdupreez at gmail dot com>]

On Thu, Jan 28, 2016 at 08:27:06AM -0500, Jeff Johnston wrote:
> I am in Belgium this week.  Most of the patch is fine, however, there is a problem caused
> because <sys/features.h> doesn't include <newlib.h> and existing code could be expecting
> <sys/features.h> to bring in the definition of the macros.

Thanks Jeff, you're right. I wrongly assumed that consumers will
_always_ include the generated newlib.h.

Even though newlib.h gets included by _ansi.h, which gets included
almost everywhere, it is _not_ included from all the files that
libc/include/sys/features.h is being included from. If a consumer of
newlib includes any of the following, libc/include/sys/features.h gets
included, but not newlib.h:

libc/include/complex.h
libc/include/fnmatch.h
libc/include/glob.h
libc/include/machine/_default_types.h
libc/include/machine/endian.h
libc/include/machine/_types.h
libc/include/regex.h
libc/include/stdint.h
libc/include/sys/cdefs.h
libc/include/sys/config.h
libc/include/sys/features.h
libc/include/sys/_intsup.h
libc/include/sys/param.h
libc/include/sys/queue.h
libc/include/sys/select.h
libc/include/sys/_stdint.h
libc/include/sys/timespec.h
libc/include/sys/tree.h

BTW, I used the following to get to the above list:
find libc/include/ -type f -name \*.h | awk '{print "echo " $1 " `gcc -Ilibc/include -E "$1" 2>/dev/null |grep '/' |grep features.h |wc -l`"}' |sh |grep -w 0 |sort >features.h_zero
find libc/include/ -type f -name \*.h | awk '{print "echo " $1 " `gcc -Ilibc/include -E "$1" 2>/dev/null |grep '/' |grep newlib.h |wc -l`"}' |sh |grep -w 0 |sort >newlib.h_zero
diff -u newlib.h_zero features.h_zero

I would think that the generated newlib.h is actually the correct
place for the __NEWLIB__, __NEWLIB_MINOR__ and __NEWLIB_PATCHLEVEL__
definitions, so this will stay and the definitions in
libc/include/sys/features.h will eventually go. Is my assumption
correct?

If we keep these definitions in newlib.h
_and_ libc/include/sys/features.h, they'll have to be guarded against
re-definition, as there are several cases where both headers get
included. So, something like this will have to be done: 

In newlib.h:
#ifndef _SYS_FEATURES_H
  #define __NEWLIB__ 2
  #define __NEWLIB_MINOR__ 2
  #define __NEWLIB_PATCHLEVEL__ 0
#endif

lib/include/sys/features.h:
#ifndef __NEWLIB_H__
  #define __NEWLIB__ 2
  #define __NEWLIB_MINOR__ 2
  #define __NEWLIB_PATCHLEVEL__ 0
#endif

I can easily adapt the patch, but of course there's at least 3 ways to do
this (note: all solutions require the re-definition guarding,
mentioned above):

1. Simply leave libc/include/sys/features.h as is, and live with the
burden of manually updating the version macros in it, every time the
version changes.
#ifndef __NEWLIB_H__
  #define __NEWLIB__ 2
  #define __NEWLIB_MINOR__ 2
  #define __NEWLIB_PATCHLEVEL__ 0
#endif

2. We generate a newlib_version.h at the same level as newlib.h
is, and include newlib_version.h in both newlib.h and
libc/include/sys/features.h.

3. Remove the existing 2 definitions from libc/include/sys/features.h
and include newlib.h in the 18 header files, mentioned at the start of
this email.

Although the 2nd and 3rd solution requires a bit more work, it frees the
maintainer(s) of newlib from the burden to update the version in 2
different places, every time the version changes.

Please let me know which one you'd prefer. Other suggestions are also
welcome.

BTW, I'm voting for solution 3.

--
Pieter du Preez