deps: update to snafu v0.8

Most of the changes are from the v0.7 update we missed and were
mechanically applied with the snafu migration tool.

This switch also requires moving to std::backtrace::Backtrace, which
required a few other changes (including removing the global setting for
whether we want to capture backtraces).

If we really wanted to keep the global setting knob, we could wrap
std::backtrace::Backtrace in our own type but I don't think the global
setting really made sense in the first place (and now that there are no
backtraces in the C FFI, the standard RUST_* environment variables can
still be used for Rust programs).

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Cargo.toml

@@ -48,4 +48,4 @@ bitflags = "^2"
 lazy_static = "^1"
 libc = "^0.2"
 rand = "^0.8"
-snafu = { version = "^0.6", features = ["backtraces-impl-backtrace-crate"] }
+snafu = "^0.8"

src/capi/core.rs

@@ -41,7 +41,7 @@ use snafu::ResultExt;
 fn parse_path<'a>(path: *const c_char) -> Result<&'a Path, Error> {
     ensure!(
         !path.is_null(),
-        error::InvalidArgument {
+        error::InvalidArgumentSnafu {
             name: "path",
             description: "cannot be NULL",
         }
@@ -105,7 +105,7 @@ pub extern "C" fn pathrs_reopen(fd: RawFd, flags: c_int) -> RawFd {
             // Rust sets O_CLOEXEC by default, without an opt-out. We need to
             // disable it if we weren't asked to do O_CLOEXEC.
             if flags.0 & libc::O_CLOEXEC == 0 {
-                syscalls::fcntl_unset_cloexec(file.as_raw_fd()).context(error::RawOsError {
+                syscalls::fcntl_unset_cloexec(file.as_raw_fd()).context(error::RawOsSnafu {
                     operation: "clear O_CLOEXEC on fd",
                 })?;
             }
@@ -248,11 +248,11 @@ pub extern "C" fn pathrs_mknod(
             libc::S_IFBLK => InodeType::BlockDevice(&perms, dev),
             libc::S_IFCHR => InodeType::CharacterDevice(&perms, dev),
             libc::S_IFIFO => InodeType::Fifo(&perms),
-            libc::S_IFSOCK => error::NotImplemented {
+            libc::S_IFSOCK => error::NotImplementedSnafu {
                 feature: "mknod(S_IFSOCK)",
             }
             .fail()?,
-            _ => error::InvalidArgument {
+            _ => error::InvalidArgumentSnafu {
                 name: "mode",
                 description: "invalid S_IFMT mask",
             }

src/error.rs

@@ -24,58 +24,15 @@
 //       resolved:
 //
 //  * https://github.com/shepmaster/snafu/issues/188.
-//  * `std::error::Backtrace` is stabilised.
 //  * `std::error::Error::chain` is stabilised.
 //  * I figure out a nice way to implement GlobalBacktrace...
 
 #[doc(inline)]
 pub use crate::syscalls::{Error as SyscallError, FrozenFd};
 
-use std::{
-    error::Error as StdError,
-    io::Error as IOError,
-    sync::atomic::{AtomicBool, Ordering},
-};
+use std::{backtrace::Backtrace, error::Error as StdError, io::Error as IOError};
 
-use snafu::{GenerateBacktrace, ResultExt};
-
-/// A wrapper around [`backtrace::Backtrace`].
-///
-/// The primary reason for this is that it allows for custom configuration of
-/// whether backtraces are generated by libpathrs. You may configure this by
-/// modifying [`BACKTRACES_ENABLED`].
-///
-/// # Stability
-/// Note that this interface will drastically change once
-/// `std::error::Backtrace` is stabilised.
-///
-/// [`backtrace::Backtrace`]: https://docs.rs/backtrace/*/backtrace/struct.Backtrace.html
-/// [`BACKTRACES_ENABLED`]: static.BACKTRACES_ENABLED.html
-// NOTE: Once std's Backtrace is finalised this will need to be changed.
-#[derive(Debug)]
-pub struct Backtrace(pub Option<backtrace::Backtrace>);
-
-/// Controls whether backtraces will be generated during error handling within
-/// libpathrs.
-///
-/// By default, backtraces are disabled for release builds and enabled otherwise.
-// TODO: This should probably be a getter+setter setup but I couldn't figure out
-//       nice names for the getter and setter.
-pub static BACKTRACES_ENABLED: AtomicBool = AtomicBool::new(cfg!(debug_assertions));
-
-impl GenerateBacktrace for Backtrace {
-    fn generate() -> Self {
-        if BACKTRACES_ENABLED.load(Ordering::SeqCst) {
-            Backtrace(Some(backtrace::Backtrace::new()))
-        } else {
-            Backtrace(None)
-        }
-    }
-
-    fn as_backtrace(&self) -> Option<&snafu::Backtrace> {
-        self.0.as_ref()
-    }
-}
+use snafu::ResultExt;
 
 /// The primary error type returned by libpathrs.
 ///
@@ -90,15 +47,15 @@ impl GenerateBacktrace for Backtrace {
 /// [`BACKTRACES_ENABLED`]: static.BACKTRACES_ENABLED.html
 /// [`Error::chain`]: https://doc.rust-lang.org/nightly/std/error/trait.Error.html#method.chain
 #[derive(Snafu, Debug)]
-#[snafu(visibility = "pub(crate)")]
+#[snafu(visibility(pub(crate)))]
 pub enum Error {
     /// The requested feature is not yet implemented.
     #[snafu(display("feature '{}' not implemented", feature))]
     NotImplemented {
         /// Feature which is not implemented.
         feature: String,
         /// Backtrace captured at time of error.
-        backtrace: Backtrace,
+        backtrace: Option<Backtrace>,
     },
 
     /// The requested feature is not supported by this kernel.
@@ -107,7 +64,7 @@ pub enum Error {
         /// Feature which is not supported.
         feature: String,
         /// Backtrace captured at time of error.
-        backtrace: Backtrace,
+        backtrace: Option<Backtrace>,
     },
 
     /// One of the provided arguments in invalid.
@@ -118,7 +75,7 @@ pub enum Error {
         /// Description of what makes the argument invalid.
         description: String,
         /// Backtrace captured at time of error.
-        backtrace: Backtrace,
+        backtrace: Option<Backtrace>,
     },
 
     /// libpathrs has detected some form of safety requirement violation.
@@ -129,7 +86,7 @@ pub enum Error {
         /// Description of safety requirement which was violated.
         description: String,
         /// Backtrace captured at time of error.
-        backtrace: Backtrace,
+        backtrace: Option<Backtrace>,
     },
 
     /// The requested libpathrs operation resulted in an [`IOError`]. This
@@ -146,7 +103,7 @@ pub enum Error {
         /// Underlying error.
         source: IOError,
         /// Backtrace captured at time of error.
-        backtrace: Backtrace,
+        backtrace: Option<Backtrace>,
     },
 
     /// The requested libpathrs operation resulted in a [`SyscallError`] by
@@ -190,7 +147,7 @@ pub(crate) trait ErrorExt {
 
 impl<T> ErrorExt for Result<T, Error> {
     fn wrap<S: Into<String>>(self, context: S) -> Self {
-        self.context(Wrapped {
+        self.context(WrappedSnafu {
             context: context.into(),
         })
     }

src/lib.rs

@@ -119,7 +119,6 @@
 // libpathrs only supports Linux at the moment.
 #![cfg(target_os = "linux")]
 
-extern crate backtrace;
 #[macro_use]
 extern crate bitflags;
 #[macro_use]

src/resolvers/kernel.rs

@@ -37,7 +37,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
     path: P,
     flags: ResolverFlags,
 ) -> Result<Handle, Error> {
-    ensure!(*IS_SUPPORTED, error::NotSupported { feature: "openat2" });
+    ensure!(*IS_SUPPORTED, error::NotSupportedSnafu { feature: "openat2" });
 
     let mut how = syscalls::OpenHow::default();
     how.flags = libc::O_PATH as u64;
@@ -64,7 +64,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
                 // TODO: Add wrapper for known-bad openat2 return codes.
                 //Some(libc::EXDEV) | Some(libc::ELOOP) => { ... }
                 _ => {
-                    return Err(err).context(error::RawOsError {
+                    return Err(err).context(error::RawOsSnafu {
                         operation: "openat2 subpath",
                     })?
                 }

src/resolvers/user.rs

@@ -96,7 +96,7 @@ fn check_current<P: AsRef<Path>>(current: &File, root: &File, expected: P) -> Re
     // The paths should be identical.
     ensure!(
         current_path == full_path,
-        error::SafetyViolation {
+        error::SafetyViolationSnafu {
             description: "fd doesn't match expected path"
         }
     );
@@ -111,7 +111,7 @@ fn check_current<P: AsRef<Path>>(current: &File, root: &File, expected: P) -> Re
         .wrap("get root path to double-check it hasn't moved")?;
     ensure!(
         root_path == new_root_path,
-        error::SafetyViolation {
+        error::SafetyViolationSnafu {
             description: "root moved during lookup"
         }
     );
@@ -168,7 +168,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
                 // ever happen, but it's better to be safe.
                 ensure!(
                     !part.as_bytes().contains(&b'/'),
-                    error::SafetyViolation {
+                    error::SafetyViolationSnafu {
                         description: "component of path resolution contains '/'",
                     }
                 );
@@ -187,7 +187,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
 
         // Get our next element.
         let next = syscalls::openat(current.as_raw_fd(), part, libc::O_PATH, 0).context(
-            error::RawOsError {
+            error::RawOsSnafu {
                 operation: "open next component of resolution",
             },
         )?;
@@ -211,7 +211,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
         // NOTE: File::metadata definitely does an fstat(2) here.
         let next_type = next
             .metadata()
-            .context(error::OsError {
+            .context(error::OsSnafu {
                 operation: "fstat of next component",
             })?
             .file_type();
@@ -225,7 +225,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
 
         // Don't continue walking if user asked for no symlinks.
         if flags.contains(ResolverFlags::NO_SYMLINKS) {
-            return error::SafetyViolation {
+            return error::SafetyViolationSnafu {
                 description: "next is a symlink and symlink resolution disabled",
             }
             .fail();
@@ -238,7 +238,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
             .is_dangerous()
             .wrap("check if next is on a dangerous filesystem")?
         {
-            return error::SafetyViolation {
+            return error::SafetyViolationSnafu {
                 description: "next is a symlink on a dangerous filesystem",
             }
             .fail();
@@ -248,7 +248,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
         // hitting filesystem loops and DoSing.
         symlink_traversals += 1;
         if symlink_traversals >= MAX_SYMLINK_TRAVERSALS {
-            return Err(IOError::from_raw_os_error(libc::ELOOP)).context(error::OsError {
+            return Err(IOError::from_raw_os_error(libc::ELOOP)).context(error::OsSnafu {
                 operation: "emulated symlink resolution",
             })?;
         }
@@ -258,7 +258,7 @@ pub(crate) fn resolve<P: AsRef<Path>>(
         //      path to the symlink. However, since readlink(2) doesn't follow
         //      symlink components we can just do it manually safely.
         let contents =
-            syscalls::readlinkat(current.as_raw_fd(), part).context(error::RawOsError {
+            syscalls::readlinkat(current.as_raw_fd(), part).context(error::RawOsSnafu {
                 operation: "readlink next symlink component",
             })?;
 

src/root.rs

@@ -103,7 +103,7 @@ fn path_split(path: &'_ Path) -> Result<(&'_ Path, &'_ Path), Error> {
     let parent = path.parent().unwrap_or_else(|| "/".as_ref());
 
     // Now construct the trailing portion of the target.
-    let name = path.file_name().context(error::InvalidArgument {
+    let name = path.file_name().context(error::InvalidArgumentSnafu {
         name: "path",
         description: "no trailing component",
     })?;
@@ -112,7 +112,7 @@ fn path_split(path: &'_ Path) -> Result<(&'_ Path, &'_ Path), Error> {
     // If there are any other path components we must bail.
     ensure!(
         !name.as_bytes().contains(&b'/'),
-        error::SafetyViolation {
+        error::SafetyViolationSnafu {
             description: "trailing component of split pathname contains '/'",
         }
     );
@@ -201,7 +201,7 @@ impl Root {
     /// [`Resolver`]: struct.Resolver.html
     pub fn open<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
         let file = syscalls::openat(libc::AT_FDCWD, path, libc::O_PATH | libc::O_DIRECTORY, 0)
-            .context(error::RawOsError {
+            .context(error::RawOsSnafu {
                 operation: "open root handle",
             })?;
         Ok(Root::from_file_unchecked(file))
@@ -336,7 +336,7 @@ impl Root {
                 syscalls::mknodat(dirfd, name, libc::S_IFBLK | mode, *dev)
             }
         }
-        .context(error::RawOsError {
+        .context(error::RawOsSnafu {
             operation: "pathrs create",
         })
     }
@@ -390,7 +390,7 @@ impl Root {
         // can't be done with the emulated backend that might be a bad idea.
         let flags = flags.0 | libc::O_CREAT;
         let file =
-            syscalls::openat(dirfd, name, flags, perm.mode()).context(error::RawOsError {
+            syscalls::openat(dirfd, name, flags, perm.mode()).context(error::RawOsSnafu {
                 operation: "pathrs create_file",
             })?;
 
@@ -458,7 +458,7 @@ impl Root {
 
         // If we ever are here, then last_error must be Some.
         Err(last_error.expect("unlinkat loop failed so last_error must exist")).context(
-            error::RawOsError {
+            error::RawOsSnafu {
                 operation: "pathrs remove",
             },
         )
@@ -497,7 +497,7 @@ impl Root {
         let dst_dirfd = dst_dir.as_raw_fd();
 
         syscalls::renameat2(src_dirfd, src_name, dst_dirfd, dst_name, flags.0).context(
-            error::RawOsError {
+            error::RawOsSnafu {
                 operation: "pathrs rename",
             },
         )