Skip to content

Commit

Permalink
Merge #1643
Browse files Browse the repository at this point in the history
1643: Replace the IoVec struct with IoSlice and IoSliceMut from the standard library r=asomers a=notgull

As per discussion in #1637, the `IoVec<&[u8]>` and `IoVec<&mut [u8]>` types have been replaced with `std::io::IoSlice` and `IoSliceMut`, respectively. Notable changes made in this pull request include:

- The complete replacement of `IoVec` with `IoSlice*` types in both public API, private API, and tests.
- Replacing `IoVec` with `IoSlice` in docs.
- Replacing `&[IoVec<&mut [u8]>]` with `&mut [IoSliceMut]`, note that the slice requires a mutable reference now. This is how it's done in the standard library, and there might be a soundness issue in doing it the other way.

Resolves #1637 

Co-authored-by: not_a_seagull <notaseagull048@gmail.com>
  • Loading branch information
bors[bot] and notgull committed Apr 8, 2022
2 parents 3b786fd + 0b58f29 commit 256707e
Show file tree
Hide file tree
Showing 9 changed files with 292 additions and 248 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -91,6 +91,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
ignoring failures from libc. And getters on the `UtsName` struct now return
an `&OsStr` instead of `&str`.
(#[1672](https://github.com/nix-rust/nix/pull/1672))
- Replaced `IoVec` with `IoSlice` and `IoSliceMut`, and replaced `IoVec::from_slice` with
`IoSlice::new`. (#[1643](https://github.com/nix-rust/nix/pull/1643))

### Fixed

Expand Down
2 changes: 1 addition & 1 deletion src/fcntl.rs
Expand Up @@ -629,7 +629,7 @@ pub fn tee(fd_in: RawFd, fd_out: RawFd, len: usize, flags: SpliceFFlags) -> Resu
#[cfg(any(target_os = "linux", target_os = "android"))]
pub fn vmsplice(
fd: RawFd,
iov: &[crate::sys::uio::IoVec<&[u8]>],
iov: &[std::io::IoSlice<'_>],
flags: SpliceFFlags
) -> Result<usize>
{
Expand Down
111 changes: 56 additions & 55 deletions src/mount/bsd.rs
Expand Up @@ -3,15 +3,14 @@ use crate::{
Errno,
NixPath,
Result,
sys::uio::IoVec
};
use libc::{c_char, c_int, c_uint, c_void};
use std::{
borrow::Cow,
ffi::{CString, CStr},
fmt,
io,
ptr
marker::PhantomData,
};


Expand Down Expand Up @@ -198,13 +197,45 @@ pub type NmountResult = std::result::Result<(), NmountError>;
#[cfg_attr(docsrs, doc(cfg(all())))]
#[derive(Debug, Default)]
pub struct Nmount<'a>{
iov: Vec<IoVec<&'a [u8]>>,
// n.b. notgull: In reality, this is a list that contains
// both mutable and immutable pointers.
// Be careful using this.
iov: Vec<libc::iovec>,
is_owned: Vec<bool>,
marker: PhantomData<&'a ()>,
}

#[cfg(target_os = "freebsd")]
#[cfg_attr(docsrs, doc(cfg(all())))]
impl<'a> Nmount<'a> {
/// Helper function to push a slice onto the `iov` array.
fn push_slice(&mut self, val: &'a [u8], is_owned: bool) {
self.iov.push(libc::iovec {
iov_base: val.as_ptr() as *mut _,
iov_len: val.len(),
});
self.is_owned.push(is_owned);
}

/// Helper function to push a pointer and its length onto the `iov` array.
fn push_pointer_and_length(&mut self, val: *const u8, len: usize, is_owned: bool) {
self.iov.push(libc::iovec {
iov_base: val as *mut _,
iov_len: len,
});
self.is_owned.push(is_owned);
}

/// Helper function to push a `nix` path as owned.
fn push_nix_path<P: ?Sized + NixPath>(&mut self, val: &P) {
val.with_nix_path(|s| {
let len = s.to_bytes_with_nul().len();
let ptr = s.to_owned().into_raw() as *const u8;

self.push_pointer_and_length(ptr, len, true);
}).unwrap();
}

/// Add an opaque mount option.
///
/// Some file systems take binary-valued mount options. They can be set
Expand Down Expand Up @@ -239,10 +270,8 @@ impl<'a> Nmount<'a> {
len: usize
) -> &mut Self
{
self.iov.push(IoVec::from_slice(name.to_bytes_with_nul()));
self.is_owned.push(false);
self.iov.push(IoVec::from_raw_parts(val, len));
self.is_owned.push(false);
self.push_slice(name.to_bytes_with_nul(), false);
self.push_pointer_and_length(val.cast(), len, false);
self
}

Expand All @@ -258,10 +287,8 @@ impl<'a> Nmount<'a> {
/// .null_opt(&read_only);
/// ```
pub fn null_opt(&mut self, name: &'a CStr) -> &mut Self {
self.iov.push(IoVec::from_slice(name.to_bytes_with_nul()));
self.is_owned.push(false);
self.iov.push(IoVec::from_raw_parts(ptr::null_mut(), 0));
self.is_owned.push(false);
self.push_slice(name.to_bytes_with_nul(), false);
self.push_slice(&[], false);
self
}

Expand All @@ -283,17 +310,8 @@ impl<'a> Nmount<'a> {
/// ```
pub fn null_opt_owned<P: ?Sized + NixPath>(&mut self, name: &P) -> &mut Self
{
name.with_nix_path(|s| {
let len = s.to_bytes_with_nul().len();
self.iov.push(IoVec::from_raw_parts(
// Must free it later
s.to_owned().into_raw() as *mut c_void,
len
));
self.is_owned.push(true);
}).unwrap();
self.iov.push(IoVec::from_raw_parts(ptr::null_mut(), 0));
self.is_owned.push(false);
self.push_nix_path(name);
self.push_slice(&[], false);
self
}

Expand All @@ -315,10 +333,8 @@ impl<'a> Nmount<'a> {
val: &'a CStr
) -> &mut Self
{
self.iov.push(IoVec::from_slice(name.to_bytes_with_nul()));
self.is_owned.push(false);
self.iov.push(IoVec::from_slice(val.to_bytes_with_nul()));
self.is_owned.push(false);
self.push_slice(name.to_bytes_with_nul(), false);
self.push_slice(val.to_bytes_with_nul(), false);
self
}

Expand All @@ -341,24 +357,8 @@ impl<'a> Nmount<'a> {
where P1: ?Sized + NixPath,
P2: ?Sized + NixPath
{
name.with_nix_path(|s| {
let len = s.to_bytes_with_nul().len();
self.iov.push(IoVec::from_raw_parts(
// Must free it later
s.to_owned().into_raw() as *mut c_void,
len
));
self.is_owned.push(true);
}).unwrap();
val.with_nix_path(|s| {
let len = s.to_bytes_with_nul().len();
self.iov.push(IoVec::from_raw_parts(
// Must free it later
s.to_owned().into_raw() as *mut c_void,
len
));
self.is_owned.push(true);
}).unwrap();
self.push_nix_path(name);
self.push_nix_path(val);
self
}

Expand All @@ -369,18 +369,19 @@ impl<'a> Nmount<'a> {

/// Actually mount the file system.
pub fn nmount(&mut self, flags: MntFlags) -> NmountResult {
// nmount can return extra error information via a "errmsg" return
// argument.
const ERRMSG_NAME: &[u8] = b"errmsg\0";
let mut errmsg = vec![0u8; 255];
self.iov.push(IoVec::from_raw_parts(
ERRMSG_NAME.as_ptr() as *mut c_void,
ERRMSG_NAME.len()
));
self.iov.push(IoVec::from_raw_parts(
errmsg.as_mut_ptr() as *mut c_void,
errmsg.len()
));

// nmount can return extra error information via a "errmsg" return
// argument.
self.push_slice(ERRMSG_NAME, false);

// SAFETY: we are pushing a mutable iovec here, so we can't use
// the above method
self.iov.push(libc::iovec {
iov_base: errmsg.as_mut_ptr() as *mut c_void,
iov_len: errmsg.len(),
});

let niov = self.iov.len() as c_uint;
let iovp = self.iov.as_mut_ptr() as *mut libc::iovec;
Expand Down Expand Up @@ -412,7 +413,7 @@ impl<'a> Drop for Nmount<'a> {
// Free the owned string. Safe because we recorded ownership,
// and Nmount does not implement Clone.
unsafe {
drop(CString::from_raw(iov.0.iov_base as *mut c_char));
drop(CString::from_raw(iov.iov_base as *mut c_char));
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions src/sys/sendfile.rs
Expand Up @@ -68,24 +68,24 @@ cfg_if! {
target_os = "freebsd",
target_os = "ios",
target_os = "macos"))] {
use crate::sys::uio::IoVec;
use std::io::IoSlice;

#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[derive(Clone, Debug)]
struct SendfileHeaderTrailer<'a>(
libc::sf_hdtr,
Option<Vec<IoVec<&'a [u8]>>>,
Option<Vec<IoVec<&'a [u8]>>>,
Option<Vec<IoSlice<'a>>>,
Option<Vec<IoSlice<'a>>>,
);

impl<'a> SendfileHeaderTrailer<'a> {
fn new(
headers: Option<&'a [&'a [u8]]>,
trailers: Option<&'a [&'a [u8]]>
) -> SendfileHeaderTrailer<'a> {
let header_iovecs: Option<Vec<IoVec<&[u8]>>> =
headers.map(|s| s.iter().map(|b| IoVec::from_slice(b)).collect());
let trailer_iovecs: Option<Vec<IoVec<&[u8]>>> =
trailers.map(|s| s.iter().map(|b| IoVec::from_slice(b)).collect());
let header_iovecs: Option<Vec<IoSlice<'_>>> =
headers.map(|s| s.iter().map(|b| IoSlice::new(b)).collect());
let trailer_iovecs: Option<Vec<IoSlice<'_>>> =
trailers.map(|s| s.iter().map(|b| IoSlice::new(b)).collect());
SendfileHeaderTrailer(
libc::sf_hdtr {
headers: {
Expand Down

0 comments on commit 256707e

Please sign in to comment.