Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sort: support percent arguments to -S option #7181

Merged
merged 3 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/uu/dd/src/parseargs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions src/uu/df/src/df.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion src/uu/du/src/du.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/uu/od/src/od.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
}
6 changes: 4 additions & 2 deletions src/uu/sort/src/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,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",
"b", "k", "K", "m", "M", "g", "G", "t", "T", "P", "E", "Z", "Y", "%",
])
.with_default_unit("K")
.with_b_byte_count(true)
Expand Down Expand Up @@ -1845,7 +1845,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()),
}
}
Expand Down
93 changes: 91 additions & 2 deletions src/uucore/src/lib/parser/parse_size.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please open a new issue for this?

#[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.
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -327,6 +397,9 @@ pub enum ParseSizeError {

/// Overflow
SizeTooBig(String),

/// Could not determine total physical memory size.
PhysicalMem(String),
}

impl Error for ParseSizeError {
Expand All @@ -335,14 +408,18 @@ impl Error for ParseSizeError {
Self::InvalidSuffix(ref s) => s,
Self::ParseFailure(ref s) => s,
Self::SizeTooBig(ref s) => s,
Self::PhysicalMem(ref s) => s,
}
}
}

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}")
}
Expand Down Expand Up @@ -681,4 +758,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());
}
}
13 changes: 13 additions & 0 deletions tests/by-util/test_sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!())
Expand Down Expand Up @@ -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")
Expand Down
Loading