Coalesce platform-specific xattr modules

Now we have one Ur-module that contains functionality common to both supported platforms.

The benefits of doing it this way are that:

1. It doesn't implement a dummy interface - rather, there will be less code generated when the feature is not present;
2. The code shared between them can be kept in sync. The other two modules were something like 80% the same.

src/feature/mod.rs

@@ -1,13 +1,5 @@
 // Extended attribute support
-
-#[cfg(target_os = "macos")] mod xattr_darwin;
-#[cfg(target_os = "macos")] pub use self::xattr_darwin::Attribute;
-
-#[cfg(target_os = "linux")] mod xattr_linux;
-#[cfg(target_os = "linux")] pub use self::xattr_linux::Attribute;
-
-#[cfg(not(any(target_os = "macos", target_os = "linux")))] mod xattr_dummy;
-#[cfg(not(any(target_os = "macos", target_os = "linux")))] pub use self::xattr_dummy::Attribute;
+pub mod xattr;
 
 // Git support
 

src/feature/xattr.rs

@@ -0,0 +1,237 @@
+//! Extended attribute support for Darwin and Linux systems.
+extern crate libc;
+
+use std::io;
+use std::path::Path;
+
+
+pub const ENABLED: bool = cfg!(feature="git") && cfg!(any(target_os="macos", target_os="linux"));
+
+pub trait FileAttributes {
+    fn attributes(&self) -> io::Result<Vec<Attribute>>;
+    fn symlink_attributes(&self) -> io::Result<Vec<Attribute>>;
+}
+
+impl FileAttributes for Path {
+    fn attributes(&self) -> io::Result<Vec<Attribute>> {
+        list_attrs(lister::Lister::new(FollowSymlinks::Yes), &self)
+    }
+
+    fn symlink_attributes(&self) -> io::Result<Vec<Attribute>> {
+        list_attrs(lister::Lister::new(FollowSymlinks::No), &self)
+    }
+}
+
+/// Attributes which can be passed to `Attribute::list_with_flags`
+#[derive(Copy, Clone)]
+pub enum FollowSymlinks {
+    Yes,
+    No
+}
+
+/// Extended attribute
+#[derive(Debug, Clone)]
+pub struct Attribute {
+    pub name: String,
+    pub size: usize,
+}
+
+pub fn list_attrs(lister: lister::Lister, path: &Path) -> io::Result<Vec<Attribute>> {
+    let c_path = match path.as_os_str().to_cstring() {
+        Some(cstring) => cstring,
+        None => return Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes")),
+    };
+
+    let bufsize = lister.listxattr_first(&c_path);
+
+    if bufsize > 0 {
+        let mut buf = vec![0u8; bufsize as usize];
+        let err = lister.listxattr_second(&c_path, &mut buf, bufsize);
+
+        if err > 0 {
+            // End indicies of the attribute names
+            // the buffer contains 0-terminates c-strings
+            let idx = buf.iter().enumerate().filter_map(|(i, v)|
+                if *v == 0 { Some(i) } else { None }
+            );
+
+            let mut names = Vec::new();
+            let mut start = 0;
+
+            for end in idx {
+                let c_end = end + 1; // end of the c-string (including 0)
+                let size = lister.getxattr(&c_path, &buf[start..c_end]);
+
+                if size > 0 {
+                    names.push(Attribute {
+                        name: lister.translate_attribute_name(&buf[start..end]),
+                        size: size as usize
+                    });
+                }
+
+                start = c_end;
+            }
+
+            Ok(names)
+        }
+        else {
+            Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
+        }
+    }
+    else {
+        Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
+    }
+}
+
+#[cfg(target_os = "macos")]
+mod lister {
+    use std::ffi::CString;
+    use libc::{c_int, size_t, ssize_t, c_char, c_void, uint32_t};
+    use super::FollowSymlinks;
+    use std::ptr;
+
+    extern "C" {
+        fn listxattr(
+            path: *const c_char, namebuf: *mut c_char,
+            size: size_t, options: c_int
+        ) -> ssize_t;
+
+        fn getxattr(
+            path: *const c_char, name: *const c_char,
+            value: *mut c_void, size: size_t, position: uint32_t,
+            options: c_int
+        ) -> ssize_t;
+    }
+
+    pub struct Lister {
+        c_flags: c_int,
+    }
+
+    impl Lister {
+        pub fn new(do_follow: FollowSymlinks) -> Lister {
+            let c_flags: c_int = match do_follow {
+                FollowSymlinks::Yes => 0x0001,
+                FollowSymlinks::No  => 0x0000,
+            };
+
+            Lister { c_flags: c_flags }
+        }
+
+        pub fn translate_attribute_name(&self, input: &[u8]) -> String {
+            use std::str::from_utf8_unchecked;
+
+            unsafe {
+                from_utf8_unchecked(input).into()
+            }
+        }
+
+        pub fn listxattr_first(&self, c_path: &CString) -> ssize_t {
+            unsafe {
+                listxattr(c_path.as_ptr(), ptr::null_mut(), 0, self.c_flags)
+            }
+        }
+
+        pub fn listxattr_second(&self, c_path: &CString, buf: &mut Vec<u8>, bufsize: ssize_t) -> ssize_t {
+            unsafe {
+                listxattr(
+                    c_path.as_ptr(),
+                    buf.as_mut_ptr() as *mut c_char,
+                    bufsize as size_t, self.c_flags
+                )
+            }
+        }
+
+        pub fn getxattr(&self, c_path: &CString, buf: &[u8]) -> ssize_t {
+            unsafe {
+                getxattr(
+                    c_path.as_ptr(),
+                    buf.as_ptr() as *const c_char,
+                    ptr::null_mut(), 0, 0, self.c_flags
+                )
+            }
+        }
+    }
+}
+
+#[cfg(target_os = "linux")]
+mod lister {
+    use std::ffi::CString;
+    use libc::{size_t, ssize_t, c_char, c_void};
+    use super::FollowSymlinks;
+    use std::ptr;
+
+    extern "C" {
+        fn listxattr(
+            path: *const c_char, list: *mut c_char, size: size_t
+        ) -> ssize_t;
+
+        fn llistxattr(
+            path: *const c_char, list: *mut c_char, size: size_t
+        ) -> ssize_t;
+
+        fn getxattr(
+            path: *const c_char, name: *const c_char,
+            value: *mut c_void, size: size_t
+        ) -> ssize_t;
+
+        fn lgetxattr(
+            path: *const c_char, name: *const c_char,
+            value: *mut c_void, size: size_t
+        ) -> ssize_t;
+    }
+
+    pub struct Lister {
+        follow_symlinks: FollowSymlinks,
+    }
+
+    impl Lister {
+        pub fn new(follow_symlinks: FollowSymlinks) -> Lister {
+            Lister { follow_symlinks: follow_symlinks }
+        }
+
+        pub fn translate_attribute_name(&self, input: &[u8]) -> String {
+            String::from_utf8_lossy(input).into_owned()
+        }
+
+        pub fn listxattr_first(&self, c_path: &CString) -> ssize_t {
+            let listxattr = match self.follow_symlinks {
+                FollowSymlinks::Yes => listxattr,
+                FollowSymlinks::No  => llistxattr,
+            };
+
+            unsafe {
+                listxattr(c_path.as_ptr(), ptr::null_mut(), 0)
+            }
+        }
+
+        pub fn listxattr_second(&self, c_path: &CString, buf: &mut Vec<u8>, bufsize: ssize_t) -> ssize_t {
+            let listxattr = match self.follow_symlinks {
+                FollowSymlinks::Yes => listxattr,
+                FollowSymlinks::No  => llistxattr,
+            };
+
+            unsafe {
+                listxattr(
+                    c_path.as_ptr(),
+                    buf.as_mut_ptr() as *mut c_char,
+                    bufsize as size_t
+                )
+            }
+        }
+
+        pub fn getxattr(&self, c_path: &CString, buf: &[u8]) -> ssize_t {
+            let getxattr = match self.follow_symlinks {
+                FollowSymlinks::Yes => getxattr,
+                FollowSymlinks::No  => lgetxattr,
+            };
+
+            unsafe {
+                getxattr(
+                    c_path.as_ptr(),
+                    buf.as_ptr() as *const c_char,
+                    ptr::null_mut(), 0
+                )
+            }
+        }
+    }
+}

src/feature/xattr_darwin.rs

@@ -1,133 +0,0 @@
-//! Extended attribute support for darwin
-extern crate libc;
-
-use std::io;
-use std::mem;
-use std::path::Path;
-use std::ptr;
-
-use self::libc::{c_int, size_t, ssize_t, c_char, c_void, uint32_t};
-
-
-/// Don't follow symbolic links
-const XATTR_NOFOLLOW: c_int = 0x0001;
-
-/// Expose HFS Compression extended attributes
-const XATTR_SHOWCOMPRESSION: c_int = 0x0020;
-
-
-extern "C" {
-    fn listxattr(path: *const c_char, namebuf: *mut c_char,
-                 size: size_t, options: c_int) -> ssize_t;
-    fn getxattr(path: *const c_char, name: *const c_char,
-                value: *mut c_void, size: size_t, position: uint32_t,
-                options: c_int) -> ssize_t;
-}
-
-
-/// Attributes which can be passed to `Attribute::list_with_flags`
-#[derive(Copy, Clone)]
-pub enum ListFlags {
-    /// Don't follow symbolic links
-    NoFollow = XATTR_NOFOLLOW as isize,
-    /// Expose HFS Compression extended attributes
-    ShowCompression = XATTR_SHOWCOMPRESSION as isize
-}
-
-
-/// Extended attribute
-#[derive(Debug, Clone)]
-pub struct Attribute {
-    name: String,
-    size: usize,
-}
-
-impl Attribute {
-    /// Lists the extended attribute of `path`.
-    /// Does follow symlinks by default.
-    pub fn list_attrs(path: &Path, flags: &[ListFlags]) -> io::Result<Vec<Attribute>> {
-        let mut c_flags: c_int = 0;
-        for &flag in flags.iter() {
-            c_flags |= flag as c_int
-        }
-
-        let c_path = match path.as_os_str().to_cstring() {
-            Some(cstring) => cstring,
-            None => return Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes")),
-        };
-
-        let bufsize = unsafe {
-            listxattr(c_path.as_ptr(), ptr::null_mut(), 0, c_flags)
-        };
-
-        if bufsize > 0 {
-            let mut buf = vec![0u8; bufsize as usize];
-            let err = unsafe { listxattr(
-                c_path.as_ptr(),
-                buf.as_mut_ptr() as *mut c_char,
-                bufsize as size_t, c_flags
-            )};
-            if err > 0 {
-                // End indicies of the attribute names
-                // the buffer contains 0-terminates c-strings
-                let idx = buf.iter().enumerate().filter_map(|(i, v)|
-                    if *v == 0 { Some(i) } else { None }
-                );
-                let mut names = Vec::new();
-                let mut start = 0;
-                for end in idx {
-                    let c_end = end + 1; // end of the c-string (including 0)
-                    let size = unsafe {
-                        getxattr(
-                            c_path.as_ptr(),
-                            buf[start..c_end].as_ptr() as *const c_char,
-                            ptr::null_mut(), 0, 0, c_flags
-                        )
-                    };
-                    if size > 0 {
-                        names.push(Attribute {
-                            name: unsafe {
-                                // buf is guaranteed to contain valid utf8 strings
-                                // see man listxattr
-                                mem::transmute::<&[u8], &str>(&buf[start..end]).to_string()
-                            },
-                            size: size as usize
-                        });
-                    }
-                    start = c_end;
-                }
-                Ok(names)
-            } else {
-                Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
-            }
-        } else {
-            Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
-        }
-    }
-
-    /// Getter for name
-    pub fn name(&self) -> &str {
-        &self.name
-    }
-
-    /// Getter for size
-    pub fn size(&self) -> usize {
-        self.size
-    }
-
-    /// Lists the extended attributes.
-    /// Follows symlinks like `metadata`
-    pub fn list(path: &Path) -> io::Result<Vec<Attribute>> {
-        Attribute::list_attrs(path, &[])
-    }
-    /// Lists the extended attributes.
-    /// Does not follow symlinks like `symlink_metadata`
-    pub fn llist(path: &Path) -> io::Result<Vec<Attribute>> {
-        Attribute::list_attrs(path, &[ListFlags::NoFollow])
-    }
-
-    /// Returns true if the extended attribute feature is implemented on this platform.
-    #[inline(always)]
-    pub fn feature_implemented() -> bool { true }
-}
-

src/feature/xattr_dummy.rs

@@ -1,32 +0,0 @@
-use std::io;
-use std::path::Path;
-
-#[derive(Clone)]
-pub struct Attribute;
-
-impl Attribute {
-
-    /// Getter for name
-    pub fn name(&self) -> &str {
-        unimplemented!()
-    }
-
-    /// Getter for size
-    pub fn size(&self) -> usize {
-        unimplemented!()
-    }
-
-    /// Lists the extended attributes. Follows symlinks like `metadata`
-    pub fn list(_: &Path) -> io::Result<Vec<Attribute>> {
-        Ok(Vec::new())
-    }
-
-    /// Lists the extended attributes. Does not follow symlinks like `symlink_metadata`
-    pub fn llist(_: &Path) -> io::Result<Vec<Attribute>> {
-        Ok(Vec::new())
-    }
-
-    pub fn feature_implemented() -> bool { false }
-}
-
-

src/feature/xattr_linux.rs

@@ -1,122 +0,0 @@
-//! Extended attribute support for darwin
-extern crate libc;
-
-use std::ffi::CString;
-use std::io;
-use std::path::Path;
-use std::ptr;
-
-use self::libc::{size_t, ssize_t, c_char, c_void};
-
-
-extern "C" {
-    fn listxattr(path: *const c_char, list: *mut c_char, size: size_t) -> ssize_t;
-    fn llistxattr(path: *const c_char, list: *mut c_char, size: size_t) -> ssize_t;
-    fn getxattr(path: *const c_char, name: *const c_char,
-                value: *mut c_void, size: size_t
-    ) -> ssize_t;
-    fn lgetxattr(path: *const c_char, name: *const c_char,
-                value: *mut c_void, size: size_t
-    ) -> ssize_t;
-}
-
-
-/// Attributes which can be passed to `Attribute::list_with_flags`
-#[derive(Copy, Clone)]
-pub enum FollowSymlinks {
-    Yes,
-    No
-}
-
-
-/// Extended attribute
-#[derive(Debug, Clone)]
-pub struct Attribute {
-    name: String,
-    size: usize,
-}
-
-impl Attribute {
-    /// Lists the extended attribute of `path`.
-    /// Does follow symlinks by default.
-    pub fn list_attrs(path: &Path, do_follow: FollowSymlinks) -> io::Result<Vec<Attribute>> {
-        let (listxattr, getxattr) = match do_follow {
-            FollowSymlinks::Yes => (listxattr, getxattr),
-            FollowSymlinks::No => (llistxattr, lgetxattr),
-        };
-
-        let c_path = match path.as_os_str().to_cstring() {
-            Some(cstring) => cstring,
-            None => return Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes")),
-        };
-
-        let bufsize = unsafe {
-            listxattr(c_path.as_ptr(), ptr::null_mut(), 0)
-        };
-
-        if bufsize > 0 {
-            let mut buf = vec![0u8; bufsize as usize];
-            let err = unsafe { listxattr(
-                c_path.as_ptr(),
-                buf.as_mut_ptr() as *mut c_char,
-                bufsize as size_t
-            )};
-            if err > 0 {
-                // End indicies of the attribute names
-                // the buffer contains 0-terminates c-strings
-                let idx = buf.iter().enumerate().filter_map(|(i, v)|
-                    if *v == 0 { Some(i) } else { None }
-                );
-                let mut names = Vec::new();
-                let mut start = 0;
-                for end in idx {
-                    let c_end = end + 1; // end of the c-string (including 0)
-                    let size = unsafe {
-                        getxattr(
-                            c_path.as_ptr(),
-                            buf[start..c_end].as_ptr() as *const c_char,
-                            ptr::null_mut(), 0
-                        )
-                    };
-                    if size > 0 {
-                        names.push(Attribute {
-                            name: String::from_utf8_lossy(&buf[start..end]).into_owned(),
-                            size: size as usize
-                        });
-                    }
-                    start = c_end;
-                }
-                Ok(names)
-            } else {
-                Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
-            }
-        } else {
-            Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
-        }
-    }
-
-    /// Getter for name
-    pub fn name(&self) -> &str {
-        &self.name
-    }
-
-    /// Getter for size
-    pub fn size(&self) -> usize {
-        self.size
-    }
-
-    /// Lists the extended attributes.
-    /// Follows symlinks like `metadata`
-    pub fn list(path: &Path) -> io::Result<Vec<Attribute>> {
-        Attribute::list_attrs(path, FollowSymlinks::Yes)
-    }
-    /// Lists the extended attributes.
-    /// Does not follow symlinks like `symlink_metadata`
-    pub fn llist(path: &Path) -> io::Result<Vec<Attribute>> {
-        Attribute::list_attrs(path, FollowSymlinks::No)
-    }
-
-    /// Returns true if the extended attribute feature is implemented on this platform.
-    #[inline(always)]
-    pub fn feature_implemented() -> bool { true }
-}

src/file.rs

@@ -12,7 +12,7 @@ use unicode_width::UnicodeWidthStr;
 
 use dir::Dir;
 use options::TimeType;
-use feature::Attribute;
+use feature::xattr::{Attribute, FileAttributes};
 
 use self::fields as f;
 
@@ -93,7 +93,7 @@ impl<'dir> File<'dir> {
             dir:    parent,
             metadata:   metadata,
             ext:    ext(&filename),
-            xattrs: Attribute::llist(path).unwrap_or(Vec::new()),
+            xattrs: path.symlink_attributes().unwrap_or(Vec::new()),
             name:   filename.to_string(),
             this:   this,
         }
@@ -199,7 +199,7 @@ impl<'dir> File<'dir> {
                 dir:    self.dir,
                 metadata:   metadata,
                 ext:    ext(&filename),
-                xattrs: Attribute::list(&target_path).unwrap_or(Vec::new()),
+                xattrs: target_path.attributes().unwrap_or(Vec::new()),
                 name:   filename.to_string(),
                 this:   None,
             })

src/options.rs

@@ -11,7 +11,7 @@ use colours::Colours;
 use column::Column;
 use column::Column::*;
 use dir::Dir;
-use feature::Attribute;
+use feature::xattr;
 use file::File;
 use output::{Grid, Details, GridDetails, Lines};
 use term::dimensions;
@@ -62,7 +62,7 @@ impl Options {
             opts.optflag("", "git", "show git status");
         }
 
-        if Attribute::feature_implemented() {
+        if xattr::ENABLED {
             opts.optflag("@", "extended", "display extended attribute keys and sizes in long (-l) output");
         }
 
@@ -281,7 +281,7 @@ impl View {
                     columns: Some(try!(Columns::deduce(matches))),
                     header: matches.opt_present("header"),
                     recurse: dir_action.recurse_options().map(|o| (o, filter)),
-                    xattr: Attribute::feature_implemented() && matches.opt_present("extended"),
+                    xattr: xattr::ENABLED && matches.opt_present("extended"),
                     colours: if dimensions().is_some() { Colours::colourful() } else { Colours::plain() },
                 };
 
@@ -302,7 +302,7 @@ impl View {
             else if matches.opt_present("level") && !matches.opt_present("recurse") && !matches.opt_present("tree") {
                 Err(Useless2("level", "recurse", "tree"))
             }
-            else if Attribute::feature_implemented() && matches.opt_present("extended") {
+            else if xattr::ENABLED && matches.opt_present("extended") {
                 Err(Useless("extended", false, "long"))
             }
             else {
@@ -640,7 +640,7 @@ impl Columns {
 mod test {
     use super::Options;
     use super::Misfire;
-    use feature::Attribute;
+    use feature::xattr;
 
     fn is_helpful<T>(misfire: Result<T, Misfire>) -> bool {
         match misfire {
@@ -742,7 +742,7 @@ mod test {
 
     #[test]
     fn extended_without_long() {
-        if Attribute::feature_implemented() {
+        if xattr::ENABLED {
             let opts = Options::getopts(&[ "--extended".to_string() ]);
             assert_eq!(opts.unwrap_err(), Misfire::Useless("extended", false, "long"))
         }

src/output/details.rs

@@ -7,7 +7,7 @@ use std::string::ToString;
 use colours::Colours;
 use column::{Alignment, Column, Cell};
 use dir::Dir;
-use feature::Attribute;
+use feature::xattr::Attribute;
 use file::fields as f;
 use file::File;
 use options::{Columns, FileFilter, RecurseOptions, SizeFormat};
@@ -508,11 +508,11 @@ impl<U> Table<U> where U: Users {
             filename_length += row.name.length;
 
             if xattr {
-                let width = row.attrs.iter().map(|a| a.name().len()).max().unwrap_or(0);
+                let width = row.attrs.iter().map(|a| a.name.len()).max().unwrap_or(0);
                 for attr in row.attrs.iter() {
-                    let name = attr.name();
+                    let name = &*attr.name;
                     let spaces: String = repeat(" ").take(width - name.len()).collect();
-                    filename.push_str(&*format!("\n{}{}  {}", name, spaces, attr.size()))
+                    filename.push_str(&*format!("\n{}{}  {}", name, spaces, attr.size))
                 }
             }