Skip to content

NetBSD: add two more errno values#3040

Merged
bors merged 2 commits into
rust-lang:masterfrom
0-wiz-0:netbsd
Apr 22, 2023
Merged

NetBSD: add two more errno values#3040
bors merged 2 commits into
rust-lang:masterfrom
0-wiz-0:netbsd

Conversation

@0-wiz-0

@0-wiz-0 0-wiz-0 commented Dec 18, 2022

Copy link
Copy Markdown
Contributor

available in NetBSD-current since 2020

available in NetBSD-current since 2020
@rustbot

rustbot commented Dec 18, 2022

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information.

@workingjubilee

Copy link
Copy Markdown
Member

Why would this be a "breakage candidate"? Because it changes ELAST? ELAST on OS that use this convention is defined as being the largest errno. It's only intended to be used to allow checks to see if an error is within the known values for errno, and things of that nature, it's not really directly used by any API. It is "versioning in userspace", as it were.

@JohnTitor

Copy link
Copy Markdown
Member

Why would this be a "breakage candidate"? Because it changes ELAST? ELAST on OS that use this convention is defined as being the largest errno. It's only intended to be used to allow checks to see if an error is within the known values for errno, and things of that nature, it's not really directly used by any API. It is "versioning in userspace", as it were.

Yes, and I think it could cause a bug (or any weird behavior due to lack of errno) if the last value points to a newer number while the user env is older.
As you mentioned on #3131, we just should not provide such a const, I think.

@workingjubilee

Copy link
Copy Markdown
Member

Hmm. I think that would be due to invalid reasoning on the part of the programmer, which we can't completely protect people from, but I agree that it's in any case quite silly from the Rust PoV.

One option:

  • Merge this change with the updated ELAST value, but also with a deprecation notice. People who are using ELAST "correctly" don't have to change anything immediately, so it doesn't introduce bugs in code that was written to be future-proof, but they can use the deprecation notice to prepare. People who were using it incorrectly have a heads up.

I think in this case having an errno with a larger value than ELAST is equally likely to introduce bugs, unfortunately.

@workingjubilee

Copy link
Copy Markdown
Member

Noting that #1902 was handled by leaving the value in place and deprecating it, though that time we said we'd change it.

@JohnTitor

Copy link
Copy Markdown
Member

@0-wiz-0 Could you apply this?

  • Merge this change with the updated ELAST value, but also with a deprecation notice. People who are using ELAST "correctly" don't have to change anything immediately, so it doesn't introduce bugs in code that was written to be future-proof, but they can use the deprecation notice to prepare. People who were using it incorrectly have a heads up.

@rustbot author

@0-wiz-0

0-wiz-0 commented Apr 21, 2023

Copy link
Copy Markdown
Contributor Author

@JohnTitor I've added a deprecation warning as requested. Did I do it correctly?

@JohnTitor

Copy link
Copy Markdown
Member

Yeah, thanks!
@bors r+

@bors

bors commented Apr 22, 2023

Copy link
Copy Markdown
Contributor

📌 Commit 06fdfac has been approved by JohnTitor

It is now in the queue for this repository.

@bors

bors commented Apr 22, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit 06fdfac with merge 9e90211...

@bors

bors commented Apr 22, 2023

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 9e90211 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants