Repository: Stebalien/xattr (language: rust)

Issue:
Method for optionally getting an attribute
Currently to get an attribute that might not be set on the file i have to either iterate through all the attributes with list or depend on the libc crate and manually handle ENODATA.

Neither of these options appeal to me and given that rust has the Option type wouldn't it be better if get returned an io::Result<Option<Vec<u8>>? Or at least that there was a version of get that returned this type?


Discussion / hints:
While having to unwrap twice will be annoying, I agree.

Fix patch:
diff --git a/src/lib.rs b/src/lib.rs
index 0f6547d3e..5ae63aa43 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -17,11 +17,11 @@ pub use sys::{XAttrs, SUPPORTED_PLATFORM};
 pub use error::UnsupportedPlatformError;
 
 /// Get an extended attribute for the specified file.
-pub fn get<N, P>(path: P, name: N) -> io::Result<Vec<u8>>
+pub fn get<N, P>(path: P, name: N) -> io::Result<Option<Vec<u8>>>
     where P: AsRef<Path>,
           N: AsRef<OsStr>
 {
-    sys::get_path(path.as_ref(), name.as_ref())
+    util::extract_noattr(sys::get_path(path.as_ref(), name.as_ref()))
 }
 
 /// Set an extended attribute on the specified file.
@@ -52,10 +52,10 @@ pub fn list<P>(path: P) -> io::Result<XAttrs>
 
 pub trait FileExt: AsRawFd {
     /// Get an extended attribute for the specified file.
-    fn get_xattr<N>(&self, name: N) -> io::Result<Vec<u8>>
+    fn get_xattr<N>(&self, name: N) -> io::Result<Option<Vec<u8>>>
         where N: AsRef<OsStr>
     {
-        sys::get_fd(self.as_raw_fd(), name.as_ref())
+        util::extract_noattr(sys::get_fd(self.as_raw_fd(), name.as_ref()))
     }
 
     /// Set an extended attribute on the specified file.
diff --git a/src/util.rs b/src/util.rs
index 74c1071d8..e21ef47af 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -5,7 +5,7 @@ use std::ptr;
 use std::ffi::CString;
 use std::path::Path;
 
-use libc::{ERANGE, ssize_t};
+use libc::{ERANGE, ENODATA, ssize_t};
 
 #[allow(dead_code)]
 pub fn name_to_c(name: &OsStr) -> io::Result<CString> {
@@ -26,6 +26,13 @@ pub fn path_to_c(path: &Path) -> io::Result<CString> {
 }
 
 
+pub fn extract_noattr(result: io::Result<Vec<u8>>) -> io::Result<Option<Vec<u8>>> {
+    result.map(|v| Some(v)).or_else(|e| match e.raw_os_error() {
+        Some(ENODATA) => Ok(None),
+        _ => Err(e),
+    })
+}
+
 pub unsafe fn allocate_loop<F: FnMut(*mut u8, usize) -> ssize_t>(mut f: F) -> io::Result<Vec<u8>> {
     let mut vec: Vec<u8> = Vec::new();
     loop {


Test patch:
diff --git a/tests/main.rs b/tests/main.rs
index 2019aa983..1141b4fc9 100644
--- a/tests/main.rs
+++ b/tests/main.rs
@@ -13,21 +13,21 @@ fn test_fd() {
     use std::os::unix::ffi::OsStrExt;
     // Only works on "real" filesystems.
     let tmp = tempfile_in("/var/tmp").unwrap();
-    assert!(tmp.get_xattr("user.test").is_err());
+    assert!(tmp.get_xattr("user.test").unwrap().is_none());
     assert_eq!(tmp.list_xattr()
                .unwrap()
                .filter(|x| x.as_bytes().starts_with(b"user."))
                .count(), 0);
 
     tmp.set_xattr("user.test", b"my test").unwrap();
-    assert_eq!(tmp.get_xattr("user.test").unwrap(), b"my test");
+    assert_eq!(tmp.get_xattr("user.test").unwrap().unwrap(), b"my test");
     assert_eq!(tmp.list_xattr()
                .unwrap()
                .filter(|x| x.as_bytes().starts_with(b"user."))
                .collect::<Vec<_>>(), vec![OsStr::new("user.test")]);
 
     tmp.remove_xattr("user.test").unwrap();
-    assert!(tmp.get_xattr("user.test").is_err());
+    assert!(tmp.get_xattr("user.test").unwrap().is_none());
     assert_eq!(tmp.list_xattr()
                .unwrap()
                .filter(|x| x.as_bytes().starts_with(b"user."))
@@ -40,27 +40,34 @@ fn test_path() {
     use std::os::unix::ffi::OsStrExt;
     // Only works on "real" filesystems.
     let tmp = NamedTempFile::new_in("/var/tmp").unwrap();
-    assert!(xattr::get(tmp.path(), "user.test").is_err());
+    assert!(xattr::get(tmp.path(), "user.test").unwrap().is_none());
     assert_eq!(xattr::list(tmp.path())
                .unwrap()
                .filter(|x| x.as_bytes().starts_with(b"user."))
                .count(), 0);
 
     xattr::set(tmp.path(), "user.test", b"my test").unwrap();
-    assert_eq!(xattr::get(tmp.path(), "user.test").unwrap(), b"my test");
+    assert_eq!(xattr::get(tmp.path(), "user.test").unwrap().unwrap(), b"my test");
     assert_eq!(xattr::list(tmp.path())
                .unwrap()
                .filter(|x| x.as_bytes().starts_with(b"user."))
                .collect::<Vec<_>>(), vec![OsStr::new("user.test")]);
 
     xattr::remove(tmp.path(), "user.test").unwrap();
-    assert!(xattr::get(tmp.path(), "user.test").is_err());
+    assert!(xattr::get(tmp.path(), "user.test").unwrap().is_none());
     assert_eq!(xattr::list(tmp.path())
                .unwrap()
                .filter(|x| x.as_bytes().starts_with(b"user."))
                .count(), 0);
 }
 
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "freebsd"))]
+fn test_missing() {
+    assert!(xattr::get("/var/empty/does-not-exist", "user.test").is_err());
+}
+
 #[test]
 #[cfg(any(target_os = "linux", target_os = "freebsd"))]
 fn test_multi() {