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

[Yaakov Selkowitz <yselkowitz at cygwin dot com>]

On 2016-03-17 11:09, Craig Howland wrote:
> 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.

Ack.  I'm not against improving the documentation, but as this is not a 
regression I don't believe it should hold back this patchset.

> (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?)

I have been relying primarily on the Linux man-pages, so whatever macros 
are documented therein should apply to newlib as well.  (If they don't, 
please let me know so I can see why.)  Besides that, anything 
newlib-specific is _DEFAULT_SOURCE (__MISC_VISIBLE internally).

> 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.

Note that glibc also uses internal macros to guard functions (albeit 
with a different nomenclature), so anyone attempting the same there 
would have the same issue.  The function SYNOPSIS is where to document this.

> (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.)

While I have built newlib for embedded architectures before, my primary 
interest is obviously Cygwin, where most software built thereon is not 
written specifically for it.  Therefore, having headers which behave 
similarly to glibc's will mean users won't *have* to look at the 
headers, because software will just build.

> 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.

Is that mapping not clear from how __*_VISIBLE are themselves defined? 
Could you be more specific about what you're looking for here?

> One specific example is _DEFAULT_SOURCE.  It's said to deprecate
> _BSD_SOURCE and _SVID_SOURCE, but why?

TBH, I'm not sure *why* glibc did this, but as a lot of software is 
written with glibc in mind -- even when it is portable -- we need to 
follow their lead.

> 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?

I think you know what that means.  Would you like to suggest a better 
wording?

> Is it a Newlib invention?

Of course not, these are all based on glibc feature test macros. 
Although I did consider adding a _NEWLIB_SOURCE for newlib-specific 
symbols, but adding support for _DEFAULT_SOURCE made that moot.

Hmm, now that you mention it, maybe we should at least have an internal 
__NEWLIB_VISIBLE triggered by _DEFAULT_SOURCE (along the lines of BSD 
and SVID) instead of using __MISC_VISIBLE, so that newlib-specific 
functions are better marked.  Thoughts?

> 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.

Again, I'm not against more documentation, and I do wish to continue to 
be on top of this going forward.  But it's not clear to me if your 
comments are an objection to the patches in their current state, or a 
wish for further improvements in this area.

--
Yaakov