Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add iopl/ioperm to all linuxes #3720

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Add iopl/ioperm to all linuxes #3720

merged 1 commit into from
Oct 15, 2024

Conversation

espindola
Copy link
Contributor

@espindola espindola commented May 24, 2024 •
edited
Loading

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented May 24, 2024

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) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@espindola
Copy link
Contributor Author

@JohnTitor is this ok?

fslongjin
fslongjin approved these changes Jul 17, 2024
View reviewed changes
Copy link

@fslongjin fslongjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can work well

@espindola
Copy link
Contributor Author

so, what is the process? I got an email saying @fslongjin approved this, what is next? Do we need another review?

@tgross35
Copy link
Contributor

These are Linux syscalls so should be available everywhere, not just x86. It should probably mirror ioctl and be at least in unix/linux_like/linux/{gnu,musl,android} (near ioctl). Please try this, CI will test the other platforms for you. The semver tests will need to be updated too.

I don't think there is any good reason to add iopl unless you have the xserver usecase, considering it has been deprecated since 2020 mkerrisk/man-pages@6caf6c4.

@rustbot author

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Aug 15, 2024
@espindola
Copy link
Contributor Author

iopl is used by https://github.com/pop-os/system76-firmware

I will try to add this to all platforms.

@espindola
Copy link
Contributor Author

Since you mentioned ioctl, I have rebased this on top of my other MR so that there is only one place to add these.

@espindola espindola changed the title Add iopl/ioperm to musl x86_64 Add iopl/ioperm to all linuxes Aug 15, 2024
@espindola
Copy link
Contributor Author

These are Linux syscalls so should be available everywhere, not just x86. It should probably mirror ioctl and be at least in unix/linux_like/linux/{gnu,musl,android} (near ioctl). Please try this, CI will test the other platforms for you. The semver tests will need to be updated too.

I just tried, and it looks like it is not available on arm or ppc. OK to leave it as x86_64/i686 only?

@espindola espindola force-pushed the musl-iopl branch 3 times, most recently from 34a6d10 to 083499a Compare August 15, 2024 18:50
@espindola
Add iopl/ioperm to musl x86_64
083499a 
Note that I think this is available on more than x86_64, but that is
all that I have to test. This also matches what was done for glibc
systems.
@espindola espindola requested a review from fslongjin August 23, 2024 14:32
@espindola
Copy link
Contributor Author

so, in summary, iopl is needed, and it can't be declared next to iocl, because it is not available everywhere ioctl is.

@tgross35
Copy link
Contributor

Fyi you can

@rustbot review

To get the labels updated that this is ready for reveiew. I'll take another look.

@tgross35 tgross35 added this pull request to the merge queue Oct 15, 2024
Merged via the queue into rust-lang:main with commit f48d010 Oct 15, 2024
39 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Oct 15, 2024
@espindola @tgross35
Add iopl/ioperm to musl x86_64
f0422d5 
Note that I think this is available on more than x86_64, but that is
all that I have to test. This also matches what was done for glibc
systems.

(backport <rust-lang#3720>)
(cherry picked from commit 083499a)
@tgross35 tgross35 mentioned this pull request Oct 15, 2024
Merged
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross35 tgross35 tgross35 approved these changes

@fslongjin fslongjin Awaiting requested review from fslongjin

Assignees

@JohnTitor JohnTitor

Labels
S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@espindola @rustbot @tgross35 @fslongjin @JohnTitor