Re: [PATCH v3 01/24] Feature test macros overhaul: sys/features.h

[Craig Howland <howland at LGSInnovations dot com>]

On 03/17/2016 04:44 AM, Yaakov Selkowitz wrote:
> On 2016-03-16 11:36, Craig Howland wrote:
> >       This all makes things more consistent (which is sorely needed),
> > but it seems to be lacking one major aspect:  documentation in the
> > source.  The email points to FEATURE_TEST_MACROS(7), but the newlib
> > developers and users are in the dark as to how to properly use this new
> > method--the man page is not mentioned anywhere in a file, for example.
>
> feature_test_macros(7) only covers the public side of this, and while it is a 
> nice explanation of the various standards, it is not strictly necessary in 
> order to know how to use it.  If you want more extensive documentation, then 
> the proper public macro(s) should be mentioned in the SYNOPSIS of each 
> function, as is done in throughout the Linux man-pages.
I agree, the usage should be in the synopsis sections.  That would cover what 
I've been calling the user aspect of documenting this. It is something that is 
presently missing; not a fault with this patch but one that has been brought to 
light by it.  (And while it's not fair to expect it, you're probably the one in 
the best position to add this information, since you had to have used it to 
generate all the new gating information.  Do you have summary notes on this that 
you could include in an email to be a starting point for someone else to use if 
you don't have time to be able to do it yourself?)
> > The prior method at least partly used user-level defines that were
> > self-explanatory, and this actually takes a step backwards from that.
>
> No, this is a step forward both in terms of consistency and correctness.
I am not saying that this is not a step forward in terms of consistency and 
correctness.  I was trying to say that in a narrow sense it has taken a step 
backwards in terms of a user trying to figure out what they need to do to get a 
particular gated function to be visible.  (Sorry for not saying it well enough 
initially.) Before a user could possibly (some times, but not all cases because 
it is inconsistent) just look at the main header file to figure out what they 
need to do, but now they definitely also need to go back to features.h and parse 
the VISIBLE tree.  See below for a further elaboration on this.
> > For example, __STRICT_ANSI is a compiler predefine that can be looked
> > up, and _POSIX_C_SOURCE can be found in POSIX. However, the user is now
> > confronted with the *_VISIBLE defines, but just what these mean is not
> > clear.
>
> Users are *not* "confronted" with __*_VISIBLE, as those are private (internal) 
> macros, nor is that naming scheme new.
Sorry, I was not clear enough.  I did not state an assumption I had made.  I 
think that since the general Newlib documentation does not tell users how things 
work in terms of function declaration visibility, that they then will go hunting 
in the header files to see how they are gated--this is what I do.  (I also 
assume that users are not going to go other than Newlib as their first course of 
action, as after all, while Newlib is a C library it is a different 
implementation than any other.  So while you might get some insight from looking 
at a GLIBC or FreeBSD man page, they are not Newlib. So the user would first 
look in the Newlib header files.  You might also check others to gain insight, 
but the Newlib source is its own definition.)  So then the *VISIBLE macros are 
what the user will see, as the user searches the headers due to lack of other 
documentation.  And, yes, the *VISIBLE macros are new from the point of view of 
now being the primary gates rather than the mix of the old way.  Yes, the new 
*VISIBLE gates are more consistent than what was there before, but they are new 
and unfamiliar to a Newlib-only person looking at the headers, and thus we're 
back to the need for documentation to aid in the transition.
> > (Some are more so than others, as ISOC99_SOURCE is pretty
> > obvious, but what does ATFILE_VISIBLE mean?)
>
> You're mixing public and private (internal) macros here.
Yes, I am, but that's what a user will see hunting through the headers trying to 
understand what they need to define.
> > In a similar manner, if a developer adds a function, what is the proper
> > way to label it with *VISIBLE?
>
> The same thing I did to get this far: look at the Linux man-pages, and use the 
> __*_VISIBLE macros which correspond to the public macros indicated therein.
My point is that you did work to understand how the public macros map to the 
*VISIBLE macros.  I think it was non-trivial and of value.  Let's get that 
written down so that every developer does not need to re-do what you have done.  
It not only saves time for them, but lessens the chance that they derive a 
different answer than you did.
One specific example is _DEFAULT_SOURCE.  It's said to deprecate _BSD_SOURCE and 
_SVID_SOURCE, but why?  And what does it really mean?  The description given for 
it is "POSIX-1.2008 with BSD and SVr4 extensions".  But if you look at POSIX, 
there is no such thing.  Where should one look to know what it is?  Is it a 
Newlib invention?  Or are we copying it from somewhere else that we need to 
refer to to try and stay in sync?  (We know from your overall reason for the 
patch that it is from elsewhere, but the source doesn't say, so N years from now 
someone looking would not know.)
> >       The comments at the start of features.h give a reasonable summary
> > for an informed user (so there is something on the user level), but
> > developer guidance on using VISIBLE is essentially non-existent.
>
> Note that's not a regression.
I agree, it is not a regression.  But I submit that adding some developer-aiding 
information is important so that all the hard work you've done on this does not 
degenerate going forward.  This is a very messy topic, and getting some more of 
your hard-earned wisdom on this added as some extra comments will help to make 
it somewhat less of a problem going ahead.
Craig