Page MenuHomeFreeBSD

Make newly POSIX functions visible (take 2)
Needs ReviewPublic

Authored by shurd on Thu, Mar 20, 7:50 AM.
Tags
None
Referenced Files
F112710767: D49423.id152466.diff
Fri, Mar 21, 7:00 PM
F112687186: D49423.id152484.diff
Fri, Mar 21, 11:03 AM
Unknown Object (File)
Fri, Mar 21, 8:07 AM
Unknown Object (File)
Fri, Mar 21, 7:21 AM
Unknown Object (File)
Fri, Mar 21, 5:54 AM
Unknown Object (File)
Thu, Mar 20, 10:38 PM
Subscribers
None

Details

Summary

Some of the POSIX 202405L functions are already in the system, make
them visible when appropriate.

Test Plan

It appears this will require an exp-run, and some port
fixes (at least Python).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 63038
Build 59922: arc lint + arc unit

Event Timeline

shurd requested review of this revision.Thu, Mar 20, 7:50 AM
shurd created this revision.

Using || __BSD_VISIBLE on all of the symbols that were previously only under __BSD_VISIBLE will be needed for this to MFC, so perhaps do it here as well? (14.2 default __POSIX_VISIBLE appears to be 200809)

If doing an exp run, we should add an explicit #error for visibility errors like the user defining __BSD_VISIBLE directly too

Hrm...

./usr.sbin/rtsold/rtsol.c:#define	__BSD_VISIBLE	1	/* IN6ADDR_LINKLOCAL_ALLROUTERS_INIT */
./crypto/openssh/openbsd-compat/fnmatch.h:#define __BSD_VISIBLE 1
./sys/contrib/openzfs/include/os/freebsd/spl/sys/ccompile.h:#define	__BSD_VISIBLE 1

Would these count?

Hrm...

./usr.sbin/rtsold/rtsol.c:#define	__BSD_VISIBLE	1	/* IN6ADDR_LINKLOCAL_ALLROUTERS_INIT */
./crypto/openssh/openbsd-compat/fnmatch.h:#define __BSD_VISIBLE 1
./sys/contrib/openzfs/include/os/freebsd/spl/sys/ccompile.h:#define	__BSD_VISIBLE 1

Would these count?

These are all wrong. OpenZFS port has been hacky in dozens of ways from the start and is a constant source of pain.

I wonder if it's a good idea to take on the whole set of mutually exclusive environment selection macros with this...

I notice that Python defines both _XOPEN_SOURCE and _POSIX_C_SOURCE for example, which is also illegal per cdefs(9).

Expanding the #error cases to enforce exactly zero or one of these macros could be useful to at least see just how big of a job it would be.

As a side note, I'm thinking this isn't a good MFC candidate anymore.

  • Error on conflicting programming environments for exp-run
  • Warn/error on conflicting programming environments

I need to update the XOPEN_SOURCE and POSIX_C_SOURCE part of cdefs(9) to indicate the 'paired values' that are legal. Like 800 and 202405.
And there's values implied by different POSIX_C_SOURCE as well wrt language defines. When paired with a language standard, these combos should likely be allowed. POSIX_C_SOURCE 202405 and I think C23 or 200809 plus C99, etc.
So pedantically, you should pick one or the other, we may need to relax a little to allow the different pairings that make sense together.
Do you think it would make sense for me to update cdefs.9 to include this matrix?

Uggg, a much bigger can of worms than I'd originally thought when I made my off-hand suggestion.

sys/sys/cdefs.h
580

I think that this might be a little too picky. _BSD_SOURCE kinda implies a number of things currently:

_POSIX_C_SOURCE 202405
_XOPEN_SOURCE 800
_C23_SOURCE (spelled __ISOC_VISIBLE 2023 internally)

So if you specify something other than those, it's an error.

In D49423#1127228, @imp wrote:

I need to update the XOPEN_SOURCE and POSIX_C_SOURCE part of cdefs(9) to indicate the 'paired values' that are legal. Like 800 and 202405.
And there's values implied by different POSIX_C_SOURCE as well wrt language defines. When paired with a language standard, these combos should likely be allowed. POSIX_C_SOURCE 202405 and I think C23 or 200809 plus C99, etc.
So pedantically, you should pick one or the other, we may need to relax a little to allow the different pairings that make sense together.
Do you think it would make sense for me to update cdefs.9 to include this matrix?

Yes please, I'm trying to capture the various allowed pairings, new update incoming shortly... though it will need to be updated again to include the C standard defines it sounds like.

Uggg, a much bigger can of worms than I'd originally thought when I made my off-hand suggestion.

Heh, I didn't even think there was a can involved when *I* started!

I also noticed that if you ask for _XOPEN_SOURCE 800, you get POSIX_C_SOURCE of 202405 unconditionally and irregardless of what you'd defined earlier.
And defining POSIX_C_SOURCE takes precedence over all the other _FOO_SOURCE things, which kinda is backwards I think.

sys/sys/cdefs.h
580

So my reading of the current cdefs(9) is that you must only specify one programming environment. While we do need to allow a combination of _XOPEN_SOURCE and _POSIX_C_SOURCE per the spec, I didn't see where we need to expand that to other things (like _C23_SOURCE)

My opinion is that we don't want that kind of mess with _BSD_SOURCE... if you define _BSD_SOURCE, you don't define anything else and you take what you get (which is everything). Allowing mixins with _BSD_SOURCE seems like a corner case generator that just makes updating anything more error-prone.

In D49423#1127234, @imp wrote:

I also noticed that if you ask for _XOPEN_SOURCE 800, you get POSIX_C_SOURCE of 202405 unconditionally and irregardless of what you'd defined earlier.
And defining POSIX_C_SOURCE takes precedence over all the other _FOO_SOURCE things, which kinda is backwards I think.

Yeah, I like treating both of these as errors. In the first case it's unspecified behaviour, and in the second case there's really no clear hierarchy between these things... though if I was making one up, BSD would trump XOPEN would trump POSIX would trump C.

sys/sys/cdefs.h
580

I think that this might be a little too picky. _BSD_SOURCE kinda implies a number of things currently:

I think I prefer to say it defines (or forces) rather than implies...

Basically, _BSD_SOURCE defines those, if you also define them, you've made an error.

Yea. In general the hierarchy is BSD_SOURCE >> XOPEN_SOURCE >> POSIX_C_SOURCE >> Cxx_SOURCE. Except we never really implemented BSD_SOURCE

Except we allowed POSIX 2008 plus c11 as a common special case. And I think c23 too.

So I think there's really two classes of things it would be nice to define/enforce... what we do with a _*_SOURCE conflict, and what we do about a user defining __*_VISIBLE.

For _*_SOURCE I think it's best to warn whenever there's a conflict, and #undef all lower priority settings and let cdefs.h do the "right thing". Doing anything else really makes it much more complicated both to support and to maintain. This matches the XOPEN/POSIX requirements at least where if POSIX < XOPEN POSIX is ignored and if POSIX > XOPEN, it's unspecified.

For __*_VISIBLE, I think it may make sense to special-case __BSD_VISIBLE as an alias for _BSD_SOURCE that causes a warning, but #error on all other __*_VISIBLE macros. An exp-run with all of them causing an #error would give us a sense of if it would be needed or not (ie: Find out how many people have copied Python).

  • Honing in on requirements
  • Allow C version to be upgraded via _CXX_SOURCE and _ISOCXX_SOURCE
  • Actually undefine things we say we do.

So, I ran egrep -r '__[A-Z0-9]*_VISIBLE' /usr/ports and now I'm very sad.

I'm still interested in the exp-run results, but it looks like *all* the __*_VISIBLE will need to be warnings instead of errors for the near future.

  • Convert #error to #warning