tail: Clean up and test suffix multiplier

Makes `parse_size` return a `Result` where the `Err` part indicates whether
there was a parsing error, or the parse size is too big to store. Also makes the
value parsed a `u64` rather than a `usize`.

Adds unit tests for `parse_size` and integration tests using the suffix
multiplier in a number passed with the `-n` flag.

src/tail/tail.rs

@@ -16,6 +16,8 @@ extern crate getopts;
 extern crate uucore;
 
 use std::collections::VecDeque;
+use std::error::Error;
+use std::fmt;
 use std::fs::File;
 use std::io::{BufRead, BufReader, Read, Seek, SeekFrom, stdin, stdout, Write};
 use std::path::Path;
@@ -27,8 +29,8 @@ static NAME: &'static str = "tail";
 static VERSION: &'static str = env!("CARGO_PKG_VERSION");
 
 enum FilterMode {
-    Bytes(usize),
+    Bytes(u64),
-    Lines(usize, u8), // (number of lines, delimiter)
+    Lines(u64, u8), // (number of lines, delimiter)
 }
 
 struct Settings {
@@ -106,9 +108,9 @@ pub fn uumain(args: Vec<String>) -> i32 {
                 slice = &slice[1..];
             }
             match parse_size(slice) {
-                Some(m) => settings.mode = FilterMode::Lines(m, '\n' as u8),
+                Ok(m) => settings.mode = FilterMode::Lines(m, '\n' as u8),
-                None => {
+                Err(e) => {
-                    show_error!("invalid number of lines ({})", slice);
+                    show_error!("{}", e.description());
                     return 1;
                 }
             }
@@ -121,9 +123,9 @@ pub fn uumain(args: Vec<String>) -> i32 {
                     slice = &slice[1..];
                 }
                 match parse_size(slice) {
-                    Some(m) => settings.mode = FilterMode::Bytes(m),
+                    Ok(m) => settings.mode = FilterMode::Bytes(m),
-                    None => {
+                    Err(e) => {
-                        show_error!("invalid number of bytes ({})", slice);
+                        show_error!("{}", e.description());
                         return 1;
                     }
                 }
@@ -167,33 +169,69 @@ pub fn uumain(args: Vec<String>) -> i32 {
     0
 }
 
-fn parse_size(mut size_slice: &str) -> Option<usize> {
+#[derive(Debug, PartialEq, Eq)]
+pub enum ParseSizeErr {
+    ParseFailure(String),
+    SizeTooBig(String),
+}
+
+impl Error for ParseSizeErr {
+    fn description(&self) -> &str {
+        match *self {
+            ParseSizeErr::ParseFailure(ref s) => &*s,
+            ParseSizeErr::SizeTooBig(ref s) => &*s,
+        }
+    }
+}
+
+impl fmt::Display for ParseSizeErr {
+    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        write!(f, "{}", Error::description(self))
+    }
+}
+
+impl ParseSizeErr {
+    fn parse_failure(s: &str) -> ParseSizeErr {
+        ParseSizeErr::ParseFailure(format!("invalid size: '{}'", s))
+    }
+
+    fn size_too_big(s: &str) -> ParseSizeErr {
+        ParseSizeErr::SizeTooBig(
+            format!("invalid size: '{}': Value too large to be stored in data type", s))
+    }
+}
+
+pub type ParseSizeResult = Result<u64, ParseSizeErr>;
+
+pub fn parse_size(mut size_slice: &str) -> Result<u64, ParseSizeErr> {
     let mut base =
         if size_slice.chars().last().unwrap_or('_') == 'B' {
             size_slice = &size_slice[..size_slice.len() - 1];
-            1000usize
+            1000u64
         } else {
-            1024usize
+            1024u64
         };
+
     let exponent =
         if size_slice.len() > 0 {
             let mut has_suffix = true;
             let exp = match size_slice.chars().last().unwrap_or('_') {
-                'K' => 1usize,
+                'K' | 'k' => 1u64,
-                'M' => 2usize,
+                'M' => 2u64,
-                'G' => 3usize,
+                'G' => 3u64,
-                'T' => 4usize,
+                'T' => 4u64,
-                'P' => 5usize,
+                'P' => 5u64,
-                'E' => 6usize,
+                'E' => 6u64,
-                'Z' => 7usize,
+                'Z' | 'Y' => {
-                'Y' => 8usize,
+                    return Err(ParseSizeErr::size_too_big(size_slice));
+                },
                 'b' => {
-                    base = 512usize;
+                    base = 512u64;
-                    1usize
+                    1u64
                 }
                 _ => {
                     has_suffix = false;
-                    0usize
+                    0u64
                 }
             };
             if has_suffix {
@@ -201,22 +239,20 @@ fn parse_size(mut size_slice: &str) -> Option<usize> {
             }
             exp
         } else {
-            0usize
+            0u64
         };
 
-    let mut multiplier = 1usize;
+    let mut multiplier = 1u64;
-    for _ in 0usize .. exponent {
+    for _ in 0u64 .. exponent {
         multiplier *= base;
     }
-    if base == 1000usize && exponent == 0usize {
+    if base == 1000u64 && exponent == 0u64 {
         // sole B is not a valid suffix
-        None
+        Err(ParseSizeErr::parse_failure(size_slice))
     } else {
-        let value: Option<usize> = size_slice.parse().ok();
+        let value: Option<u64> = size_slice.parse().ok();
-        match value {
+        value.map(|v| Ok(multiplier * v))
-            Some(v) => Some(multiplier * v),
+             .unwrap_or(Err(ParseSizeErr::parse_failure(size_slice)))
-            _ => None
-        }
     }
 }
 
@@ -224,7 +260,7 @@ fn parse_size(mut size_slice: &str) -> Option<usize> {
 //
 // In case is found, the options vector will get rid of that object so that
 // getopts works correctly.
-fn obsolete(options: &[String]) -> (Vec<String>, Option<usize>) {
+fn obsolete(options: &[String]) -> (Vec<String>, Option<u64>) {
     let mut options: Vec<String> = options.to_vec();
     let mut a = 0;
     let b = options.len();
@@ -242,7 +278,7 @@ fn obsolete(options: &[String]) -> (Vec<String>, Option<usize>) {
                 // If this is the last number
                 if pos == len - 1 {
                     options.remove(a);
-                    let number: Option<usize> = from_utf8(&current[1..len]).unwrap().parse().ok();
+                    let number: Option<u64> = from_utf8(&current[1..len]).unwrap().parse().ok();
                     return (options, Some(number.unwrap()));
                 }
             }
@@ -378,7 +414,7 @@ fn unbounded_tail<T: Read>(mut reader: BufReader<T>, settings: &Settings) {
             let mut ringbuf: VecDeque<String> = VecDeque::new();
             let mut skip = if settings.beginning {
                 let temp = count;
-                count = ::std::usize::MAX;
+                count = ::std::u64::MAX;
                 temp - 1
             } else {
                 0
@@ -391,7 +427,7 @@ fn unbounded_tail<T: Read>(mut reader: BufReader<T>, settings: &Settings) {
                         if skip > 0 {
                             skip -= 1;
                         } else {
-                            if count <= ringbuf.len() {
+                            if count <= ringbuf.len() as u64 {
                                 ringbuf.pop_front();
                             }
                             ringbuf.push_back(datum);
@@ -409,7 +445,7 @@ fn unbounded_tail<T: Read>(mut reader: BufReader<T>, settings: &Settings) {
             let mut ringbuf: VecDeque<u8> = VecDeque::new();
             let mut skip = if settings.beginning {
                 let temp = count;
-                count = ::std::usize::MAX;
+                count = ::std::u64::MAX;
                 temp - 1
             } else {
                 0
@@ -422,7 +458,7 @@ fn unbounded_tail<T: Read>(mut reader: BufReader<T>, settings: &Settings) {
                         if skip > 0 {
                             skip -= 1;
                         } else {
-                            if count <= ringbuf.len() {
+                            if count <= ringbuf.len() as u64 {
                                 ringbuf.pop_front();
                             }
                             ringbuf.push_back(datum[0]);

tests/tail.rs

@@ -1,3 +1,6 @@
+extern crate uu_tail;
+use uu_tail::parse_size;
+
 use std::io::Write;
 
 #[macro_use]
@@ -108,3 +111,63 @@ fn test_bytes_big() {
         assert_eq!(actual_char, expected_char);
     }
 }
+
+#[test]
+fn test_parse_size() {
+    // No suffix.
+    assert_eq!(Ok(1234), parse_size("1234"));
+
+    // kB is 1000
+    assert_eq!(Ok(9 * 1000), parse_size("9kB"));
+
+    // K is 1024
+    assert_eq!(Ok(2 * 1024), parse_size("2K"));
+
+    let suffixes = [
+        ('M', 2u32),
+        ('G', 3u32),
+        ('T', 4u32),
+        ('P', 5u32),
+        ('E', 6u32),
+    ];
+
+    for &(c, exp) in &suffixes {
+        let s = format!("2{}B", c);
+        assert_eq!(Ok(2 * (1000 as u64).pow(exp)), parse_size(&s));
+
+        let s = format!("2{}", c);
+        assert_eq!(Ok(2 * (1024 as u64).pow(exp)), parse_size(&s));
+    }
+
+    // Sizes that are too big.
+    assert!(parse_size("1Z").is_err());
+    assert!(parse_size("1Y").is_err());
+
+    // Bad number
+    assert!(parse_size("328hdsf3290").is_err());
+}
+
+#[test]
+fn test_lines_with_size_suffix() {
+    const FILE: &'static str = "test_lines_with_size_suffix.txt";
+    const EXPECTED_FILE: &'static str = "test_lines_with_size_suffix_expected.txt";
+    const LINES: usize = 3_000;
+    const N_ARG: usize = 2 * 1024;
+
+    let (at, mut ucmd) = testing(UTIL_NAME);
+
+    let mut big_input = at.make_scoped_file(FILE);
+    for i in 0..LINES {
+        writeln!(&mut big_input, "Line {}", i).expect("Could not write to FILE");
+    }
+    big_input.flush().expect("Could not flush FILE");
+
+    let mut big_expected = at.make_scoped_file(EXPECTED_FILE);
+    for i in (LINES - N_ARG)..LINES {
+        writeln!(&mut big_expected, "Line {}", i).expect("Could not write to EXPECTED_FILE");
+    }
+    big_expected.flush().expect("Could not flush EXPECTED_FILE");
+
+    let result = ucmd.arg(FILE).arg("-n").arg("2K").run();
+    assert_eq!(result.stdout, at.read(EXPECTED_FILE));
+}