chmod, install: move mode parsing into uucore

src/chmod/chmod.rs

@@ -3,7 +3,7 @@
 /*
  * This file is part of the uutils coreutils package.
  *
- * (c) Arcterus <arcterus@mail.com>
+ * (c) Alex Lyon <arcterus@mail.com>
  *
  * For the full copyright and license information, please view the LICENSE
  * file that was distributed with this source code.
@@ -16,16 +16,16 @@ extern crate walker;
 #[macro_use]
 extern crate uucore;
 
-use std::error::Error;
 use std::fs;
 use std::io::Write;
 use std::os::unix::fs::{MetadataExt, PermissionsExt};
 use std::path::Path;
 use walker::Walker;
+use uucore::mode;
 
 const NAME: &'static str = "chmod";
 static SUMMARY: &'static str = "Change the mode of each FILE to MODE.
- With --reference, change the mode of each FILE to that of RFILE."; 
+ With --reference, change the mode of each FILE to that of RFILE.";
 static LONG_HELP: &'static str = "
  Each MODE is of the form '[ugoa]*([-+=]([rwxXst]*|[ugo]))+|[-+=]?[0-7]+'.
 ";
@@ -197,9 +197,9 @@ impl Chmoder {
                     let arr: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
                     let result =
                         if mode.contains(arr) {
-                            self.parse_numeric(fperm, mode)
+                            mode::parse_numeric(fperm, mode)
                         } else {
-                            self.parse_symbolic(fperm, mode, file)
+                            mode::parse_symbolic(fperm, mode, file.is_dir())
                         };
                     match result {
                         Ok(mode) => {
@@ -220,126 +220,6 @@ impl Chmoder {
         Ok(())
     }
 
-    fn parse_numeric(&self, fperm: u32, mut mode: &str) -> Result<u32, String> {
-        let (op, pos) = try!(self.parse_op(mode, Some('=')));
-        mode = mode[pos..].trim_left_matches('0');
-        if mode.len() > 4 {
-            Err(format!("mode is too large ({} > 7777)", mode))
-        } else {
-            match u32::from_str_radix(mode, 8) {
-                Ok(change) => {
-                    Ok(match op {
-                        '+' => fperm | change,
-                        '-' => fperm & !change,
-                        '=' => change,
-                        _ => unreachable!()
-                    })
-                }
-                Err(err) => Err(err.description().to_owned())
-            }
-        }
-    }
-
-    fn parse_symbolic(&self, mut fperm: u32, mut mode: &str, file: &Path) -> Result<u32, String> {
-        #[cfg(unix)]
-        use libc::umask;
-
-        #[cfg(target_os = "redox")]
-        unsafe fn umask(_mask: u32) -> u32 {
-            // XXX Redox does not currently have umask
-            0
-        }
-
-        let (mask, pos) = self.parse_levels(mode);
-        if pos == mode.len() {
-            return Err(format!("invalid mode ({})", mode));
-        }
-        let respect_umask = pos == 0;
-        let last_umask = unsafe {
-            umask(0)
-        };
-        mode = &mode[pos..];
-        while mode.len() > 0 {
-            let (op, pos) = try!(self.parse_op(mode, None));
-            mode = &mode[pos..];
-            let (mut srwx, pos) = self.parse_change(mode, fperm, file);
-            if respect_umask {
-                srwx &= !(last_umask as u32);
-            }
-            mode = &mode[pos..];
-            match op {
-                '+' => fperm |= srwx & mask,
-                '-' => fperm &= !(srwx & mask),
-                '=' => fperm = (fperm & !mask) | (srwx & mask),
-                _ => unreachable!()
-            }
-        }
-        unsafe {
-            umask(last_umask);
-        }
-        Ok(fperm)
-    }
-
-    fn parse_levels(&self, mode: &str) -> (u32, usize) {
-        let mut mask = 0;
-        let mut pos = 0;
-        for ch in mode.chars() {
-            mask |= match ch {
-                'u' => 0o7700,
-                'g' => 0o7070,
-                'o' => 0o7007,
-                'a' => 0o7777,
-                _ => break
-            };
-            pos += 1;
-        }
-        if pos == 0 {
-            mask = 0o7777;  // default to 'a'
-        }
-        (mask, pos)
-    }
-
-    fn parse_op(&self, mode: &str, default: Option<char>) -> Result<(char, usize), String> {
-        match mode.chars().next() {
-            Some(ch) => match ch {
-                '+' | '-' | '=' => Ok((ch, 1)),
-                _ => match default {
-                    Some(ch) => Ok((ch, 0)),
-                    None => Err(format!("invalid operator (expected +, -, or =, but found {})", ch))
-                }
-            },
-            None => Err("unexpected end of mode".to_owned())
-        }
-    }
-
-    fn parse_change(&self, mode: &str, fperm: u32, file: &Path) -> (u32, usize) {
-        let mut srwx = fperm & 0o7000;
-        let mut pos = 0;
-        for ch in mode.chars() {
-            match ch {
-                'r' => srwx |= 0o444,
-                'w' => srwx |= 0o222,
-                'x' => srwx |= 0o111,
-                'X' => {
-                    if file.is_dir() || (fperm & 0o0111) != 0 {
-                        srwx |= 0o111
-                    }
-                }
-                's' => srwx |= 0o4000 | 0o2000,
-                't' => srwx |= 0o1000,
-                'u' => srwx = (fperm & 0o700) | ((fperm >> 3) & 0o070) | ((fperm >> 6) & 0o007),
-                'g' => srwx = ((fperm << 3) & 0o700) | (fperm & 0o070) | ((fperm >> 3) & 0o007),
-                'o' => srwx = ((fperm << 6) & 0o700) | ((fperm << 3) & 0o070) | (fperm & 0o007),
-                _ => break
-            };
-            pos += 1;
-        }
-        if pos == 0 {
-            srwx = 0;
-        }
-        (srwx, pos)
-    }
-
     fn change_file(&self, fperm: u32, mode: u32, file: &Path, path: &str) -> Result<(), i32> {
         if fperm == mode {
             if self.verbose && !self.changes {

src/install/mode.rs

@@ -3,6 +3,7 @@ extern crate libc;
 use std::io::Write;
 use std::path::Path;
 use std::fs;
+use uucore::mode;
 
 /// Takes a user-supplied string and tries to parse to u16 mode bitmask.
 pub fn parse(mode_string: &str, considering_dir: bool) -> Result<u32, String> {
@@ -10,9 +11,9 @@ pub fn parse(mode_string: &str, considering_dir: bool) -> Result<u32, String> {
 
     // Passing 000 as the existing permissions seems to mirror GNU behaviour.
     if mode_string.contains(numbers) {
-        chmod_rs::parse_numeric(0, mode_string)
+        mode::parse_numeric(0, mode_string)
     } else {
-        chmod_rs::parse_symbolic(0, mode_string, considering_dir)
+        mode::parse_symbolic(0, mode_string, considering_dir)
     }
 }
 
@@ -37,131 +38,3 @@ pub fn chmod(path: &Path, mode: u32) -> Result<(), ()> {
     // chmod on Windows only sets the readonly flag, which isn't even honored on directories
     Ok(())
 }
-
-/// Parsing functions taken from chmod.rs.
-///
-/// We keep these in a dedicated module to minimize debt of duplicated code.
-///
-mod chmod_rs {
-    extern crate libc;
-
-    pub fn parse_numeric(fperm: u32, mut mode: &str) -> Result<u32, String> {
-        let (op, pos) = try!(parse_op(mode, Some('=')));
-        mode = mode[pos..].trim_left_matches('0');
-        if mode.len() > 4 {
-            Err(format!("mode is too large ({} > 7777)", mode))
-        } else {
-            match u32::from_str_radix(mode, 8) {
-                Ok(change) => {
-                    Ok(match op {
-                        '+' => fperm | change,
-                        '-' => fperm & !change,
-                        '=' => change,
-                        _ => unreachable!()
-                    })
-                }
-                Err(_) => Err(String::from("numeric parsing error"))
-            }
-        }
-    }
-
-    pub fn parse_symbolic(mut fperm: u32, mut mode: &str, considering_dir: bool) -> Result<u32, String> {
-        #[cfg(unix)]
-        use libc::umask;
-
-        #[cfg(target_os = "redox")]
-        unsafe fn umask(_mask: u32) -> u32 {
-            // XXX Redox does not currently have umask
-            0
-        }
-
-        let (mask, pos) = parse_levels(mode);
-        if pos == mode.len() {
-            return Err(format!("invalid mode ({})", mode));
-        }
-        let respect_umask = pos == 0;
-        let last_umask = unsafe {
-            umask(0)
-        };
-        mode = &mode[pos..];
-        while mode.len() > 0 {
-            let (op, pos) = try!(parse_op(mode, None));
-            mode = &mode[pos..];
-            let (mut srwx, pos) = parse_change(mode, fperm, considering_dir);
-            if respect_umask {
-                srwx &= !(last_umask as u32);
-            }
-            mode = &mode[pos..];
-            match op {
-                '+' => fperm |= srwx & mask,
-                '-' => fperm &= !(srwx & mask),
-                '=' => fperm = (fperm & !mask) | (srwx & mask),
-                _ => unreachable!()
-            }
-        }
-        unsafe {
-            umask(last_umask);
-        }
-        Ok(fperm)
-    }
-
-    fn parse_levels(mode: &str) -> (u32, usize) {
-        let mut mask = 0;
-        let mut pos = 0;
-        for ch in mode.chars() {
-            mask |= match ch {
-                'u' => 0o7700,
-                'g' => 0o7070,
-                'o' => 0o7007,
-                'a' => 0o7777,
-                _ => break
-            };
-            pos += 1;
-        }
-        if pos == 0 {
-            mask = 0o7777;  // default to 'a'
-        }
-        (mask, pos)
-    }
-
-    fn parse_op(mode: &str, default: Option<char>) -> Result<(char, usize), String> {
-        match mode.chars().next() {
-            Some(ch) => match ch {
-                '+' | '-' | '=' => Ok((ch, 1)),
-                _ => match default {
-                    Some(ch) => Ok((ch, 0)),
-                    None => Err(format!("invalid operator (expected +, -, or =, but found {})", ch))
-                }
-            },
-            None => Err("unexpected end of mode".to_owned())
-        }
-    }
-
-    fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) {
-        let mut srwx = fperm & 0o7000;
-        let mut pos = 0;
-        for ch in mode.chars() {
-            match ch {
-                'r' => srwx |= 0o444,
-                'w' => srwx |= 0o222,
-                'x' => srwx |= 0o111,
-                'X' => {
-                    if considering_dir || (fperm & 0o0111) != 0 {
-                        srwx |= 0o111
-                    }
-                }
-                's' => srwx |= 0o4000 | 0o2000,
-                't' => srwx |= 0o1000,
-                'u' => srwx = (fperm & 0o700) | ((fperm >> 3) & 0o070) | ((fperm >> 6) & 0o007),
-                'g' => srwx = ((fperm << 3) & 0o700) | (fperm & 0o070) | ((fperm >> 3) & 0o007),
-                'o' => srwx = ((fperm << 6) & 0o700) | ((fperm << 3) & 0o070) | (fperm & 0o007),
-                _ => break
-            };
-            pos += 1;
-        }
-        if pos == 0 {
-            srwx = 0;
-        }
-        (srwx, pos)
-    }
-}

src/uucore/Cargo.toml

@@ -17,13 +17,14 @@ fs = ["libc"]
 utf8 = []
 encoding = ["data-encoding"]
 parse_time = []
+mode = ["libc"]
 utmpx = ["time", "libc"]
 process = ["libc"]
 signals = []
 entries = ["libc"]
 wide = []
 utsname = ["libc"]
-default = ["fs", "libc", "utf8", "utsname", "encoding", "parse_time", "utmpx", "process", "entries", "signals", "wide"]
+default = ["fs", "libc", "utf8", "utsname", "encoding", "parse_time", "mode", "utmpx", "process", "entries", "signals", "wide"]
 
 [lib]
 path = "lib.rs"

src/uucore/lib.rs

@@ -17,6 +17,8 @@ pub mod utf8;
 pub mod encoding;
 #[cfg(feature = "parse_time")]
 pub mod parse_time;
+#[cfg(feature = "mode")]
+pub mod mode;
 
 #[cfg(all(unix, not(target_os = "fuchsia"), feature = "utmpx"))]
 pub mod utmpx;

src/uucore/mode.rs

@@ -0,0 +1,129 @@
+// This file is part of the uutils coreutils package.
+//
+// (c) Alex Lyon <arcterus@mail.com>
+//
+// For the full copyright and license information, please view the LICENSE
+// file that was distributed with this source code.
+//
+
+use std::error::Error;
+
+pub fn parse_numeric(fperm: u32, mut mode: &str) -> Result<u32, String> {
+    let (op, pos) = parse_op(mode, Some('='))?;
+    mode = mode[pos..].trim_left_matches('0');
+    if mode.len() > 4 {
+        Err(format!("mode is too large ({} > 7777)", mode))
+    } else {
+        match u32::from_str_radix(mode, 8) {
+            Ok(change) => {
+                Ok(match op {
+                    '+' => fperm | change,
+                    '-' => fperm & !change,
+                    '=' => change,
+                    _ => unreachable!()
+                })
+            }
+            Err(err) => Err(err.description().to_owned())
+        }
+    }
+}
+
+pub fn parse_symbolic(mut fperm: u32, mut mode: &str, considering_dir: bool) -> Result<u32, String> {
+    #[cfg(unix)]
+    use libc::umask;
+
+    #[cfg(target_os = "redox")]
+    unsafe fn umask(_mask: u32) -> u32 {
+        // XXX Redox does not currently have umask
+        0
+    }
+
+    let (mask, pos) = parse_levels(mode);
+    if pos == mode.len() {
+        return Err(format!("invalid mode ({})", mode));
+    }
+    let respect_umask = pos == 0;
+    let last_umask = unsafe {
+        umask(0)
+    };
+    mode = &mode[pos..];
+    while mode.len() > 0 {
+        let (op, pos) = parse_op(mode, None)?;
+        mode = &mode[pos..];
+        let (mut srwx, pos) = parse_change(mode, fperm, considering_dir);
+        if respect_umask {
+            srwx &= !(last_umask as u32);
+        }
+        mode = &mode[pos..];
+        match op {
+            '+' => fperm |= srwx & mask,
+            '-' => fperm &= !(srwx & mask),
+            '=' => fperm = (fperm & !mask) | (srwx & mask),
+            _ => unreachable!()
+        }
+    }
+    unsafe {
+        umask(last_umask);
+    }
+    Ok(fperm)
+}
+
+fn parse_levels(mode: &str) -> (u32, usize) {
+    let mut mask = 0;
+    let mut pos = 0;
+    for ch in mode.chars() {
+        mask |= match ch {
+            'u' => 0o7700,
+            'g' => 0o7070,
+            'o' => 0o7007,
+            'a' => 0o7777,
+            _ => break
+        };
+        pos += 1;
+    }
+    if pos == 0 {
+        mask = 0o7777;  // default to 'a'
+    }
+    (mask, pos)
+}
+
+fn parse_op(mode: &str, default: Option<char>) -> Result<(char, usize), String> {
+    match mode.chars().next() {
+        Some(ch) => match ch {
+            '+' | '-' | '=' => Ok((ch, 1)),
+            _ => match default {
+                Some(ch) => Ok((ch, 0)),
+                None => Err(format!("invalid operator (expected +, -, or =, but found {})", ch))
+            }
+        },
+        None => Err("unexpected end of mode".to_owned())
+    }
+}
+
+fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) {
+    let mut srwx = fperm & 0o7000;
+    let mut pos = 0;
+    for ch in mode.chars() {
+        match ch {
+            'r' => srwx |= 0o444,
+            'w' => srwx |= 0o222,
+            'x' => srwx |= 0o111,
+            'X' => {
+                if considering_dir || (fperm & 0o0111) != 0 {
+                    srwx |= 0o111
+                }
+            }
+            's' => srwx |= 0o4000 | 0o2000,
+            't' => srwx |= 0o1000,
+            'u' => srwx = (fperm & 0o700) | ((fperm >> 3) & 0o070) | ((fperm >> 6) & 0o007),
+            'g' => srwx = ((fperm << 3) & 0o700) | (fperm & 0o070) | ((fperm >> 3) & 0o007),
+            'o' => srwx = ((fperm << 6) & 0o700) | ((fperm << 3) & 0o070) | (fperm & 0o007),
+            _ => break
+        };
+        pos += 1;
+    }
+    if pos == 0 {
+        srwx = 0;
+    }
+    (srwx, pos)
+}

tests/test_install.rs

@@ -116,7 +116,7 @@ fn test_install_mode_failing() {
     at.touch(file);
     at.mkdir(dir);
     assert!(ucmd.arg(file).arg(dir).arg(mode_arg)
-            .fails().stderr.contains("Invalid mode string: numeric parsing error"));
+            .fails().stderr.contains("Invalid mode string: invalid digit found in string"));
 
     let dest_file = &format!("{}/{}", dir, file);
     assert!(at.file_exists(file));