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.

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

Use POSIX shared memory in X11 backend #165

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Conversation

notgull
Copy link
Member

@notgull notgull commented Oct 14, 2023

The X11 backend used System-V shared memory to communicate shared buffers to the server, but rustix does not support SysV SHM so we had to use libc in this backend. However, there is an alternate X11 API which uses POSIX shared memory, which rustix does support. This PR switches the X11 backend to POSIX shared memory and purges the previous usages of libc.

The goal is to remove libc totally. Therefore this PR also removes the default libc dependency from rustix.

The X11 backend used System-V shared memory to communicate shared
buffers to the server, but rustix does not support SysV SHM so we had to
use libc in this backend. However, there is an alternate X11 API which
uses POSIX shared memory, which rustix does support. This PR switches
the X11 backend to POSIX shared memory and purges the previous
usages of libc.

The goal is to remove libc totally. Therefore this PR also removes the
default libc dependency from rustix.

Signed-off-by: John Nunley <[email protected]>
@notgull notgull requested a review from john01dav as a code owner October 14, 2023 17:30
@@ -758,6 +767,45 @@ impl Drop for X11Impl {
}
}

/// Create a shared memory identifier.
fn create_shm_id() -> io::Result<fd::OwnedFd> {
Copy link
Member

Choose a reason for hiding this comment

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

We should move the create_memfile logic in the wayland backend to somewhere it can be used in both backends, instead of duplicating things.

Using memfd_create is better than shm_open where supported.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if memfiles are supported for X11 MIT-SHM

Copy link
Member

Choose a reason for hiding this comment

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

I can't really find any documentation for ShmAttachFd beyond:

Create a shared memory segment. The file descriptor will be mapped at offset
zero, and the size will be obtained using fstat(). A zero size will result in a
Value error.

The file descriptor the server should mmap().

So any fd it can fstat to get the size and mmap should be fine.

https://lists.freedesktop.org/archives/xorg-devel/2013-October/038490.html

I was hoping for a patch to shmproto.txt, but there isn't one, le sigh.
We should really fix the MIT-SHM docs, Jon's reverse-engineered spec pdf
is full of enough handwave to be embarassing for us.

Lol. I guess the fact https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/blob/master/src/shm.xml exists means the blocker there is addressed, but no one has added documentation for version 1.2 of the protocol yet.

@ids1024
Copy link
Member

ids1024 commented Oct 17, 2023

The goal is to remove libc totally.

Personally I don't see that much value to completely avoiding the libc dependency as long as the crate requires std and there isn't a version of std available that doesn't link to the C standard library. But I don't specifically have any issue with the changes in question.

@notgull
Copy link
Member Author

notgull commented Oct 18, 2023

Personally I don't see that much value to completely avoiding the libc dependency as long as the crate requires std and there isn't a version of std available that doesn't link to the C standard library. But I don't specifically have any issue with the changes in question.

I think there's progress on this front.

@notgull notgull merged commit c0e8723 into master Oct 18, 2023
39 checks passed
@notgull notgull deleted the notgull/x11-posix-shm branch October 18, 2023 02:15
@notgull notgull mentioned this pull request Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants