This is the mail archive of the mailing list for the newlib project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]