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

Add System V shared memory APIs #1989

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add System V shared memory APIs #1989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
([#1912](https://github.com/nix-rust/nix/pull/1912))
- Added `mq_timedreceive` to `::nix::mqueue`.
([#1966])(https://github.com/nix-rust/nix/pull/1966)
- Added System V shared memory APIs `shmget`, `shmat`, `shmdt` and `shmctl`.
([#1989])(https://github.com/nix-rust/nix/pull/1989)

### Changed

Expand Down
63 changes: 62 additions & 1 deletion src/sys/mman.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::Result;
#[cfg(not(target_os = "android"))]
#[cfg(feature = "fs")]
use crate::{fcntl::OFlag, sys::stat::Mode};
use libc::{self, c_int, c_void, off_t, size_t};
use libc::{self, c_int, c_void, key_t, off_t, shmid_ds, size_t};
use std::{num::NonZeroUsize, os::unix::io::{AsRawFd, AsFd}};

libc_bitflags! {
Expand Down Expand Up @@ -603,3 +603,64 @@ pub fn shm_unlink<P: ?Sized + NixPath>(name: &P) -> Result<()> {

Errno::result(ret).map(drop)
}

/// Creates and returns a new, or returns an existing, System V shared memory
/// segment identifier.
///
/// For more information, see [`shmget(2)`].
///
/// [`shmget(2)`]: https://man7.org/linux/man-pages/man2/shmget.2.html
pub fn shmget(key: key_t, size: size_t, shmflg: c_int) -> Result<c_int> {
Copy link
Contributor

Choose a reason for hiding this comment

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

In general (not just for shmget): I would not expose key_t directly. It is just a type alias, and thus not safe. Looking at the docs, some sort of newtype or enum would be more appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

shmflg should be an enum of the valid values, not a raw C integer. I believe that there is a special macro in nix (libc_enum! or something like that) that will streamline the creation of this.

Copy link
Contributor

Choose a reason for hiding this comment

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

At the very least the result type should be a newtype pattern. I.e:

-> Result<ShmKey>

where

#[derive(Debug, PartialEq, Eq, Hash)]
struct ShmKey(c_int)

impl ShmKey {
    unsafe fn to_raw(&self) -> c_int {
        self.0
    }
    // unsafe from_raw, other methods
}

This helps ensure that you will get sensible compiler errors.

Errno::result(unsafe { libc::shmget(key, size, shmflg) })
}

/// Attaches the System V shared memory segment identified by `shmid` to the
/// address space of the calling process.
///
/// For more information, see [`shmat(2)`].
///
/// # Safety
///
/// `shmid` should be a valid shared memory identifier and
/// `shmaddr` must meet the requirements described in the [`shmat(2)`] man page.
///
/// [`shmat(2)`]: https://man7.org/linux/man-pages/man2/shmat.2.html
pub unsafe fn shmat(
shmid: c_int,
shmaddr: *const c_void,
shmflg: c_int,
Copy link
Contributor

Choose a reason for hiding this comment

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

Again: shmflg, shmid should be wrapped in safe abstractions. Reading the man pages it even seems like shmat and shmget take different sets of flags. So two different enums probably?

) -> Result<*mut c_void> {
Errno::result(libc::shmat(shmid, shmaddr, shmflg))
}

/// Performs the reverse of [`shmat`], detaching the shared memory segment at
/// the given address from the address space of the calling process.
///
/// For more information, see [`shmdt(2)`].
///
/// # Safety
///
/// `shmaddr` must meet the requirements described in the [`shmdt(2)`] man page.
///
/// [`shmdt(2)`]: https://man7.org/linux/man-pages/man2/shmdt.2.html
pub unsafe fn shmdt(shmaddr: *const c_void) -> Result<()> {
Errno::result(libc::shmdt(shmaddr)).map(drop)
}

/// Performs control operation specified by `cmd` on the System V shared
/// memory segment given by `shmid`.
///
/// For more information, see [`shmctl(2)`].
///
/// # Safety
///
/// All arguments should be valid and meet the requirements described in the [`shmctl(2)`] man page.
///
/// [`shmctl(2)`]: https://man7.org/linux/man-pages/man2/shmctl.2.html
pub unsafe fn shmctl(
Copy link
Contributor

Choose a reason for hiding this comment

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

Again: shmflg & cmd should be wrapped in safe abstractions. Note that at least on Linux there seems to be some Linux specific commands. So cfg compile time guards will be needed to construct the enum.

We also don't want to expose shmid_ds directly to the user I would say. It too needs rustification (though I haven't looked into the details of that).

shmid: c_int,
cmd: c_int,
buf: *mut shmid_ds,
) -> Result<c_int> {
Errno::result(libc::shmctl(shmid, cmd, buf))
Copy link
Contributor

Choose a reason for hiding this comment

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

It also seems the meaning of the return value changes depending on the specific command, so it is not correct to treat it as errno unconditionally:

Quoting man 2 shmctl on my system:

A successful IPC_INFO or SHM_INFO operation returns the index of the highest used entry in the kernel's internal array recording information about all shared memory segments. (This information can be used with repeated SHM_STAT or SHM_STAT_ANY operations to obtain information about all shared memory segments on the system.) A successful SHM_STAT operation returns the identifier of the shared memory segment whose index was given in shmid. Other operations return 0 on success.

}