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

Port from libc to rustix on Unix platforms #847

Open
notgull opened this issue Dec 7, 2023 · 4 comments · May be fixed by #878
Open

Port from libc to rustix on Unix platforms #847

notgull opened this issue Dec 7, 2023 · 4 comments · May be fixed by #878

Comments

@notgull
Copy link

notgull commented Dec 7, 2023

rustix is a system call wrapper that wraps raw system calls on Linux and libc on other platforms. It would allow crossterm to refactor out error handling and unsafe syscall handling code from its codebase. In terms of speed, rustix bypasses libc for Linux system calls. In some cases, this can allow system calls to be directly inlined into their calling functions, lowering the instruction count.

rustix currently supports all of the system calls that this crate uses. The only problem would be that it would bump the MSRV to 1.63, but if that isn't an issue then I can write a PR for this.
@notgull
Copy link
Author

notgull commented Jan 5, 2024

@TimonPost What are your thoughts on this? If I submitted a PR, would it be accepted?

@TimonPost
Copy link
Member

TimonPost commented Jan 6, 2024 •
edited

Im doubting a little, a pr is welcome, but i think in general we got pretty simple usecases and most of them havent changed for years now. It seems rustix might bring a couple of extra dependencies - which would be my biggest consern. So could be good to check that out, certainly removing unsafe is better, but unsafe isnt a big issue directly if used right.

@notgull
Copy link
Author

notgull commented Jan 6, 2024

It seems rustix might bring a couple of extra dependencies - which would be my biggest consern.

rustix brings in two dependencies on Linux: bitflags (which is already used in this crate) and linux-raw-sys. On other platforms rustix doesn't bring in the raw Linux bindings and instead brings in libc and libc_errno.

@TimonPost
Copy link
Member

even bringing in bitflags could cause duplicated versions to be included. It creates a dependency between rustix and crossterm where they need to use the same versions. Tho bitflags probably is quite stable. But again feel free to open PR and check what impact it has on the Cagro.lock

notgull added a commit to forkgull/crossterm that referenced this issue Apr 23, 2024
@notgull
feat: Use rustix instead of libc
2d3e685 
Closes crossterm-rs#847

rustix is a wrapper around either raw Linux system calls or libc, based
on the current platform. The main advantage is that it can make programs
much more efficient, since system calls can be inlined directly into
the functions that call them. I've seen rustix reduce instruction counts
in my programs when I've made the switch in another programs.

In addition, it reduces the amount of unsafe code.

Signed-off-by: John Nunley <dev@notgull.net>
@notgull notgull linked a pull request Apr 23, 2024 that will close this issue
Open
notgull added a commit to forkgull/crossterm that referenced this issue May 4, 2024
@notgull
feat: Use rustix instead of libc
4ca0c8e 
Closes crossterm-rs#847

rustix is a wrapper around either raw Linux system calls or libc, based
on the current platform. The main advantage is that it can make programs
much more efficient, since system calls can be inlined directly into
the functions that call them. I've seen rustix reduce instruction counts
in my programs when I've made the switch in another programs.

In addition, it reduces the amount of unsafe code.

Signed-off-by: John Nunley <dev@notgull.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use rustix instead of libc forkgull/crossterm
2 participants
@notgull @TimonPost