[PATCH v3 01/24] Feature test macros overhaul: sys/features.h
Thu Mar 17 16:09:00 GMT 2016
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
> 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.
More information about the Newlib