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

Migrate errors to the modern thiserror #7

Merged
merged 2 commits into from
Jan 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
language: rust

rust:
- nightly-2018-07-24 # pinned toolchain for clippy
- 1.24.0 # minimum supported toolchain
- 1.35.0 # minimum supported toolchain
- stable
- beta
- nightly
Expand All @@ -13,11 +12,11 @@ matrix:

env:
global:
- CLIPPY_RUST_VERSION=nightly-2018-07-24
- CLIPPY_RUST_VERSION=1.35.0

before_script:
- bash -c 'if [[ "$TRAVIS_RUST_VERSION" == "$CLIPPY_RUST_VERSION" ]]; then
Copy link
Owner

Choose a reason for hiding this comment

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

I usually keep this pattern to avoid breaking master when new clippy lints are introduced. Would it be fine for you to keep a pinned toolchain for clippy?

rustup component add clippy-preview;
rustup component add clippy;
fi'

script:
Expand Down
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ exclude = [
libc = "0.2"
# Public dependencies, exposed through library API.
either = "1.5"
errno = "0.2"
error-chain = {version = "0.12", default-features = false}
thiserror = "1"

[package.metadata.release]
sign-commit = true
Expand Down
38 changes: 21 additions & 17 deletions src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
//! Error handling.

use errno;
use std::{ffi, io};

error_chain!{
errors {
/// Syscall error, as `errno(3)`.
Sys(errno: errno::Errno) {
description("syscall failed")
display("{}", errno)
}
}
/// Enumeration of errors possible in this library
#[derive(thiserror::Error, Debug)]
pub enum Error {
/// Cannot convert the `name` argument to a C String!
#[error("cannot convert `name` to a C string")]
NameCStringConversion(#[source] std::ffi::NulError),
/// Cannot create the memfd
#[error("cannot create memfd")]
Create(#[source] std::io::Error),
/// Cannot add new seals to the memfd
#[error("cannot add seals to the memfd")]
AddSeals(#[source] std::io::Error),
/// Cannot read the seals of a memfd
#[error("cannot read seals for a memfd")]
GetSeals(#[source] std::io::Error),
}

// doc attributes are required to workaround
// https://github.com/rust-lang-nursery/error-chain/issues/63
foreign_links {
Io(io::Error) #[doc = "I/O error."];
NulChar(ffi::NulError) #[doc = "NULL byte in conversion to C string."];
}
#[cfg(test)]
#[test]
fn error_send_sync() {
fn assert_error<E: std::error::Error + Send + Sync + 'static>() {}
assert_error::<Error>();
}
12 changes: 5 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
//!
//! ```rust
//! use memfd;
//! # use memfd::errors::Result;
//!
//! fn new_sized_memfd() -> Result<memfd::Memfd> {
//! fn new_sized_memfd() -> Result<memfd::Memfd, Box<dyn std::error::Error>> {
//! // Create a sealable memfd.
//! let opts = memfd::MemfdOptions::default().allow_sealing(true);
//! let mfd = opts.create("sized-1K")?;
Expand All @@ -24,7 +23,7 @@
//! mfd.add_seals(&seals)?;
//!
//! // Prevent further sealing changes.
//! mfd.add_seal(memfd::FileSeal::SealSeal);
//! mfd.add_seal(memfd::FileSeal::SealSeal)?;
//!
//! Ok(mfd)
//! }
Expand All @@ -33,15 +32,14 @@
#![deny(missing_docs)]

extern crate either;
extern crate errno;
#[macro_use]
extern crate error_chain;
extern crate libc;
extern crate thiserror;

pub mod errors;
mod errors;
mod memfd;
mod nr;
mod sealing;

pub use errors::Error;
pub use memfd::{HugetlbSize, Memfd, MemfdOptions};
pub use sealing::{FileSeal, SealsHashSet};
30 changes: 10 additions & 20 deletions src/memfd.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use either;
use errno;
use errors;
use libc;
use nr;
use sealing;
Expand Down Expand Up @@ -62,18 +60,16 @@ impl MemfdOptions {
}

/// Create a memfd according to configuration.
pub fn create<T: AsRef<str>>(&self, name: T) -> errors::Result<Memfd> {
let cname = ffi::CString::new(name.as_ref())?;
pub fn create<T: AsRef<str>>(&self, name: T) -> Result<Memfd, crate::Error> {
let cname =
ffi::CString::new(name.as_ref()).map_err(crate::Error::NameCStringConversion)?;
let name_ptr = cname.as_ptr();
let flags = self.bitflags();

// UNSAFE(lucab): name_ptr points to memory owned by cname.
let r = unsafe { libc::syscall(libc::SYS_memfd_create, name_ptr, flags) };
if r < 0 {
return Err(
errors::Error::from_kind(errors::ErrorKind::Sys(errno::errno()))
.chain_err(|| "memfd_create error"),
);
return Err(crate::Error::Create(std::io::Error::last_os_error()));
};

// UNSAFE(lucab): returned from kernel, checked for non-negative value.
Expand Down Expand Up @@ -168,44 +164,38 @@ impl Memfd {
}

/// Return the current set of seals.
pub fn seals(&self) -> errors::Result<sealing::SealsHashSet> {
pub fn seals(&self) -> Result<sealing::SealsHashSet, crate::Error> {
let flags = Self::file_get_seals(&self.file)?;
Ok(sealing::bitflags_to_seals(flags))
}

/// Add a single seal to the existing set of seals.
pub fn add_seal(&self, seal: sealing::FileSeal) -> errors::Result<()> {
pub fn add_seal(&self, seal: sealing::FileSeal) -> Result<(), crate::Error> {
use std::iter::FromIterator;

let set = sealing::SealsHashSet::from_iter(vec![seal]);
self.add_seals(&set)
}

/// Add some seals to the existing set of seals.
pub fn add_seals(&self, seals: &sealing::SealsHashSet) -> errors::Result<()> {
pub fn add_seals(&self, seals: &sealing::SealsHashSet) -> Result<(), crate::Error> {
let fd = self.file.as_raw_fd();
let flags = sealing::seals_to_bitflags(seals);
// UNSAFE(lucab): required syscall.
let r = unsafe { libc::syscall(libc::SYS_fcntl, fd, libc::F_ADD_SEALS, flags) };
if r < 0 {
return Err(
errors::Error::from_kind(errors::ErrorKind::Sys(errno::errno()))
.chain_err(|| "F_ADD_SEALS error"),
);
return Err(crate::Error::AddSeals(std::io::Error::last_os_error()));
};
Ok(())
}

/// Return the current sealing bitflags.
fn file_get_seals(fp: &fs::File) -> errors::Result<u64> {
fn file_get_seals(fp: &fs::File) -> Result<u64, crate::Error> {
let fd = fp.as_raw_fd();
// UNSAFE(lucab): required syscall.
let r = unsafe { libc::syscall(libc::SYS_fcntl, fd, libc::F_GET_SEALS) };
if r < 0 {
return Err(
errors::Error::from_kind(errors::ErrorKind::Sys(errno::errno()))
.chain_err(|| "F_GET_SEALS error"),
);
return Err(crate::Error::GetSeals(std::io::Error::last_os_error()));
};

Ok(r as u64)
Expand Down