std: map ENOTSUP to ErrorKind::Unsupported

[valentynkit]

View all comments

ENOTSUP and EOPNOTSUPP both mean the operation isn't supported. They're the same value on some targets (Linux, FreeBSD), where the existing EOPNOTSUPP => Unsupported arm (#139822) already covers both, and different on others (Apple, OpenBSD), where ENOTSUP decodes to Uncategorized instead. I don't see a reason to treat it differently, so this maps ENOTSUP to Unsupported as well.

It uses a match guard rather than an or-pattern, since the two are equal on the targets where they alias and an or-pattern would be unreachable there. Same shape as the EAGAIN/EWOULDBLOCK arm just below:

x if x == libc::EOPNOTSUPP || x == libc::ENOTSUP => Unsupported,

This was raised once before (#125228) and closed, since both errnos were left out of the original Unsupported PR (#78880). #139822 has since added EOPNOTSUPP, so the same reasoning now covers ENOTSUP.

I didn't add a test, since the decode arms aren't tested today.

r? libs

[workingjubilee]

No reasoning was given in the PR where the behavior was changed. It was simply claimed to be incorrect.

[workingjubilee]

As far as I understand it, ErrorKind::Unsupported is intended to map to "never succeeds", which is not what this error code indicates. Making the behavior more incorrect because a previous PR made it incorrect, by itself simply claiming it was incorrect, does not seem desirable to me. I'm not sure what else we should be doing though.

[ds84182]

https://lists.gnu.org/archive/html/bug-glibc/2002-08/msg00017.html

POSIX 1003.1-200 defines the errors EOPNOTSUPP and ENOTSUP as follows:

[ENOTSUP] Not supported. The implementation does not support this feature of the Realtime Option Group.

[EOPNOTSUPP] Operation not supported on socket. The type of socket (address family or protocol) does not support the requested operation.

So EOPNOTSUPP is specifically for sockets and ENOTSUP is for everything else. The generally accepted error message seems to be "Operation is not supported on transport endpoint" for EOPNOTSUPP.

[valentynkit]

No reasoning was given in the PR where the behavior was changed. It was simply claimed to be incorrect.

You're right that I didn't justify in PR the mapping beyond calling the old one incorrect, so I won't lean on it as precedent.

[ENOTSUP]
Not supported (may be the same value as [EOPNOTSUPP]).
The split only shows up on Apple and the BSDs, and only because ENOTSUP got left out. So this isn't adding a new behavior, it's making one errno categorize the same way across targets instead of dropping to Uncategorized on Apple alone.

The reason I'd keep the two together is simpler: ENOTSUP and EOPNOTSUPP are the same integer on Linux, Android and FreeBSD, so an ENOTSUP error already decodes to Unsupported there through the existing arm.

So EOPNOTSUPP is specifically for sockets and ENOTSUP is for everything else. The generally accepted error message seems to be "Operation is not supported on transport endpoint" for EOPNOTSUPP.

You're right that neither strictly means "never succeeds on this platform" (both can be per-socket or per-fs), so I'm happy to take the broader question to a separate issue if you'd rather reconsider Unsupported for both, reverting #139822 included.

The goal is portability: an ENOTSUP fallback that matches ErrorKind::Unsupported shouldn't work on Linux but silently miss on macOS. Left as-is, ENOTSUP just stays inconsistent with EOPNOTSUPP on the targets where they differ.

[workingjubilee]

You're right that I didn't justify in PR the mapping beyond calling the old one incorrect, so I won't lean on it as precedent.

...Typo? That was by @ozgureyilmaz, so surely it was not your responsibility to justify the PR that you did not author.

But yes, I think we can agree that whatever is decided, the reasoning should be able to stand on its own. And I can see the argument for making the change. At the very least, surely we should be offering something better than just "Uncategorized".

[jhpratt]

I don't particularly understand the implications of this change.

r? @workingjubilee; feel free to reassign if wanted

[rustbot]

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

[workingjubilee]

honestly I'm not sure if anyone does :ferris_clueless:

[valentynkit]

Okay, so currently Unsupported doc itself says "the operation can never succeed", but that already isn't true for what maps there.

/// This operation is unsupported on this platform.
///
/// This means that the operation can never succeed.

EOPNOTSUPP already in Unsupported is per socket and ENOTSUP is per-fd/fs, both can suceed on a different object.
Only ENOSYS really means "never".

Not pushing any of these, just mapping what I see:

1. Map ENOTSUP like EOPNOTSUPP (this PR) so one errno stops decoding to two kinds across targets, they're already the same integer on Linux.
2. Reverting Fix: Map EOPNOTSUPP to ErrorKind::Unsupported on Unix #139822 and move EOPNOTSUPP back to Uncategorized so both of them will be in Uncategorized.
   And Unsupported kind will be for truly "never" codes like ENOSYS.
   Does moving it back count as a contract break? Based on docs errno -> kind isn't a stability guarantee.
3. Give the conditional "not supported on this object" codes their own kind, so both EOPNOTSUPP and ENOTSUP will be on new seperate category something like OperationsNotSupported (just a placeholder, a clearer one could be picked), starting ACP for this etc...
4. Or leave it as-is and close this out. If the inconsistency isn't worth the churn, that's a fine answer too and I'm happy to close.

I feel like option 1, is a minimal change that fixes the inconsistency, and avoid potential confusion.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[valentynkit]

Rebased onto master.
It's been open a while, so: worth proceeding, or should I close it?

[joshtriplett]

@rfcbot merge libs-api

[rust-rfcbot]

@joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @the8472

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[nia-e]

Per today's libs-api meeting, we're happy with this; since it's a commitment to use that error kind, this will need FCP though.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[RalfJung]

Note that there is also ENOSYS mapping to the same thing. Seems better to group all 3 together.

View changes since the review

[valentynkit]

Thanks for highlighting this, you are right ENOSYS maps to Unsupported as well.

Currently the guard was added because Linux libc defines ENOTSUP as EOPNOTSUPP (same value), so an or-pattern would be unreachable and dead code on Linux (95 | 95 => ...).
ENOSYS have distinct value, so it doesn't need a guard, and keeping it as pattern has some benefits (compiler could catch if it will collide with another arm).

I could add it, if you prefer to have 3 of them grouped together anyway, if not, i could just move ENOSYS arm right above this guard, so Unsupported case will be near each other.

[RalfJung]

IMO it'd be best to have them all next to each other -- whether as one arm or as two (one using the if ... || ...) doesn't matter much IMO.

[valentynkit]

Yeah agree, moved ENOSYS to have Unsupported cases next to each other.