Repository: uutils/coreutils (language: rust)

Issue:
sort: -S/--buffer-size does not support memory percentages %
GNU sort v. Rust sort:
```
$ /usr/bin/sort --buffer-size=50% < /dev/null
$ target/release/coreutils sort --buffer-size=50% < /dev/null
sort: invalid --buffer-size argument '50%'
```
Relevant excerpts from GNU sort manpage:
```
       -S, --buffer-size=SIZE
              use SIZE for main memory buffer

[...]

       SIZE may be followed by the following multiplicative suffixes: % 1% of memory, b 1, K 1024 (default), and so on for M, G, T, P, E, Z, Y.
```

(this could be a good first issue)


Discussion / hints:
There is an even bigger problem here, which is that `sort` precomputes data for each line that will be sorted (this makes comparing them faster). Due to that the exact amount of memory usage depends on the number of lines in a chunk (shorter lines -> more lines in a chunk -> more memory usage).
Right now, when you specify a buffer size, the actual memory usage will be less or more than that. I think we should first solve that problem.
@miDeb I wasn't aware of that issue, but it looks to me the two can be fixed independently from one another.
I will take a look at this one (only the percentage parsing for now)
Is this open for contribution? 
Sure, go ahead!
Is it ok to use sysinfo crate to get memory size? 
@zeroishero I think we already use that crate in some utils, so should be fine!
Ok, then. I can probably do this. I will start it soon.
@tertsdiepraam  I don't see sysinfo being used. I meant this crate:
https://crates.io/crates/sysinfo


Hmmm, I was sure we were using that somewhere, but maybe that was some other repo. If it's hard to get the info some other way, it might be fine to use that. You can make a PR with that first and then we'll figure out what to do. The crate is by Guillaume Gomez, so I definitely trust it 😄 

Fix patch:
diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs
index 59836b1a1e4..e26b3495316 100644
--- a/src/uu/dd/src/parseargs.rs
+++ b/src/uu/dd/src/parseargs.rs
@@ -517,9 +517,7 @@ fn parse_bytes_no_x(full: &str, s: &str) -> Result<u64, ParseError> {
         (None, None, None) => match parser.parse_u64(s) {
             Ok(n) => (n, 1),
             Err(ParseSizeError::SizeTooBig(_)) => (u64::MAX, 1),
-            Err(ParseSizeError::InvalidSuffix(_) | ParseSizeError::ParseFailure(_)) => {
-                return Err(ParseError::InvalidNumber(full.to_string()))
-            }
+            Err(_) => return Err(ParseError::InvalidNumber(full.to_string())),
         },
         (Some(i), None, None) => (parse_bytes_only(s, i)?, 1),
         (None, Some(i), None) => (parse_bytes_only(s, i)?, 2),
diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs
index 092c8381290..8602d8af7af 100644
--- a/src/uu/df/src/df.rs
+++ b/src/uu/df/src/df.rs
@@ -189,6 +189,7 @@ impl Options {
                         .to_string(),
                 ),
                 ParseSizeError::ParseFailure(s) => OptionsError::InvalidBlockSize(s),
+                ParseSizeError::PhysicalMem(s) => OptionsError::InvalidBlockSize(s),
             })?,
             header_mode: {
                 if matches.get_flag(OPT_HUMAN_READABLE_BINARY)
diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs
index 2392497a935..bd017f1d515 100644
--- a/src/uu/du/src/du.rs
+++ b/src/uu/du/src/du.rs
@@ -1120,7 +1120,9 @@ fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String
         ParseSizeError::InvalidSuffix(_) => {
             format!("invalid suffix in --{} argument {}", option, s.quote())
         }
-        ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()),
+        ParseSizeError::ParseFailure(_) | ParseSizeError::PhysicalMem(_) => {
+            format!("invalid --{} argument {}", option, s.quote())
+        }
         ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()),
     }
 }
diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs
index 6dd75d30792..fcb72c1ae70 100644
--- a/src/uu/od/src/od.rs
+++ b/src/uu/od/src/od.rs
@@ -626,7 +626,9 @@ fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String
         ParseSizeError::InvalidSuffix(_) => {
             format!("invalid suffix in --{} argument {}", option, s.quote())
         }
-        ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()),
+        ParseSizeError::ParseFailure(_) | ParseSizeError::PhysicalMem(_) => {
+            format!("invalid --{} argument {}", option, s.quote())
+        }
         ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()),
     }
 }
diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs
index f2677e66136..edff2baae85 100644
--- a/src/uu/sort/src/sort.rs
+++ b/src/uu/sort/src/sort.rs
@@ -289,7 +289,7 @@ impl GlobalSettings {
         // GNU sort (8.32) invalid:  b, B, 1B,                         p, e, z, y
         let size = Parser::default()
             .with_allow_list(&[
-                "b", "k", "K", "m", "M", "g", "G", "t", "T", "P", "E", "Z", "Y", "R", "Q",
+                "b", "k", "K", "m", "M", "g", "G", "t", "T", "P", "E", "Z", "Y", "R", "Q", "%",
             ])
             .with_default_unit("K")
             .with_b_byte_count(true)
@@ -1855,7 +1855,9 @@ fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String
         ParseSizeError::InvalidSuffix(_) => {
             format!("invalid suffix in --{} argument {}", option, s.quote())
         }
-        ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()),
+        ParseSizeError::ParseFailure(_) | ParseSizeError::PhysicalMem(_) => {
+            format!("invalid --{} argument {}", option, s.quote())
+        }
         ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()),
     }
 }
diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs
index c99df6e39ce..b18e1695d70 100644
--- a/src/uucore/src/lib/parser/parse_size.rs
+++ b/src/uucore/src/lib/parser/parse_size.rs
@@ -8,10 +8,70 @@
 
 use std::error::Error;
 use std::fmt;
-use std::num::IntErrorKind;
+#[cfg(target_os = "linux")]
+use std::io::BufRead;
+use std::num::{IntErrorKind, ParseIntError};
 
 use crate::display::Quotable;
 
+/// Error arising from trying to compute system memory.
+enum SystemError {
+    IOError,
+    ParseError,
+    NotFound,
+}
+
+impl From<std::io::Error> for SystemError {
+    fn from(_: std::io::Error) -> Self {
+        Self::IOError
+    }
+}
+
+impl From<ParseIntError> for SystemError {
+    fn from(_: ParseIntError) -> Self {
+        Self::ParseError
+    }
+}
+
+/// Get the total number of bytes of physical memory.
+///
+/// The information is read from the `/proc/meminfo` file.
+///
+/// # Errors
+///
+/// If there is a problem reading the file or finding the appropriate
+/// entry in the file.
+#[cfg(target_os = "linux")]
+fn total_physical_memory() -> Result<u128, SystemError> {
+    // On Linux, the `/proc/meminfo` file has a table with information
+    // about memory usage. For example,
+    //
+    //     MemTotal:        7811500 kB
+    //     MemFree:         1487876 kB
+    //     MemAvailable:    3857232 kB
+    //     ...
+    //
+    // We just need to extract the number of `MemTotal`
+    let table = std::fs::read("/proc/meminfo")?;
+    for line in table.lines() {
+        let line = line?;
+        if line.starts_with("MemTotal:") && line.ends_with("kB") {
+            let num_kilobytes: u128 = line[9..line.len() - 2].trim().parse()?;
+            let num_bytes = 1024 * num_kilobytes;
+            return Ok(num_bytes);
+        }
+    }
+    Err(SystemError::NotFound)
+}
+
+/// Get the total number of bytes of physical memory.
+///
+/// TODO Implement this for non-Linux systems.
+#[cfg(not(target_os = "linux"))]
+fn total_physical_memory() -> Result<u128, SystemError> {
+    Err(SystemError::NotFound)
+}
+
 /// Parser for sizes in SI or IEC units (multiples of 1000 or 1024 bytes).
 ///
 /// The [`Parser::parse`] function performs the parse.
@@ -133,6 +193,16 @@ impl<'parser> Parser<'parser> {
             }
         }
 
+        // Special case: for percentage, just compute the given fraction
+        // of the total physical memory on the machine, if possible.
+        if unit == "%" {
+            let number: u128 = Self::parse_number(&numeric_string, 10, size)?;
+            return match total_physical_memory() {
+                Ok(total) => Ok((number / 100) * total),
+                Err(_) => Err(ParseSizeError::PhysicalMem(size.to_string())),
+            };
+        }
+
         // Compute the factor the unit represents.
         // empty string means the factor is 1.
         //
@@ -320,6 +390,9 @@ pub enum ParseSizeError {
 
     /// Overflow
     SizeTooBig(String),
+
+    /// Could not determine total physical memory size.
+    PhysicalMem(String),
 }
 
 impl Error for ParseSizeError {
@@ -328,6 +401,7 @@ impl Error for ParseSizeError {
             Self::InvalidSuffix(ref s) => s,
             Self::ParseFailure(ref s) => s,
             Self::SizeTooBig(ref s) => s,
+            Self::PhysicalMem(ref s) => s,
         }
     }
 }
@@ -335,7 +409,10 @@ impl Error for ParseSizeError {
 impl fmt::Display for ParseSizeError {
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
         let s = match self {
-            Self::InvalidSuffix(s) | Self::ParseFailure(s) | Self::SizeTooBig(s) => s,
+            Self::InvalidSuffix(s)
+            | Self::ParseFailure(s)
+            | Self::SizeTooBig(s)
+            | Self::PhysicalMem(s) => s,
         };
         write!(f, "{s}")
     }
@@ -674,4 +751,16 @@ mod tests {
         assert_eq!(Ok(94722), parse_size_u64("0x17202"));
         assert_eq!(Ok(44251 * 1024), parse_size_u128("0xACDBK"));
     }
+
+    #[test]
+    #[cfg(target_os = "linux")]
+    fn parse_percent() {
+        assert!(parse_size_u64("0%").is_ok());
+        assert!(parse_size_u64("50%").is_ok());
+        assert!(parse_size_u64("100%").is_ok());
+        assert!(parse_size_u64("100000%").is_ok());
+        assert!(parse_size_u64("-1%").is_err());
+        assert!(parse_size_u64("1.0%").is_err());
+        assert!(parse_size_u64("0x1%").is_err());
+    }
 }


Test patch:
diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs
index 31889058e15..a134cbc2078 100644
--- a/tests/by-util/test_sort.rs
+++ b/tests/by-util/test_sort.rs
@@ -29,6 +29,10 @@ fn test_helper(file_name: &str, possible_args: &[&str]) {
 
 #[test]
 fn test_buffer_sizes() {
+    #[cfg(target_os = "linux")]
+    let buffer_sizes = ["0", "50K", "50k", "1M", "100M", "0%", "10%"];
+    // TODO Percentage sizes are not yet supported beyond Linux.
+    #[cfg(not(target_os = "linux"))]
     let buffer_sizes = ["0", "50K", "50k", "1M", "100M"];
     for buffer_size in &buffer_sizes {
         TestScenario::new(util_name!())
@@ -73,6 +77,15 @@ fn test_invalid_buffer_size() {
         .code_is(2)
         .stderr_only("sort: invalid suffix in --buffer-size argument '100f'\n");
 
+    // TODO Percentage sizes are not yet supported beyond Linux.
+    #[cfg(target_os = "linux")]
+    new_ucmd!()
+        .arg("-S")
+        .arg("0x123%")
+        .fails()
+        .code_is(2)
+        .stderr_only("sort: invalid --buffer-size argument '0x123%'\n");
+
     new_ucmd!()
         .arg("-n")
         .arg("-S")