Skip to content

Commit

Permalink
ls: introduce 2nd blocksize & fix todos in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@cakebaker
cakebaker committed Dec 22, 2023
1 parent 546201b commit a8a5931
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 43 deletions.
70 changes: 36 additions & 34 deletions src/uu/ls/src/ls.rs
View file
Expand Up @@ -161,8 +161,8 @@ pub mod options {

const DEFAULT_TERM_WIDTH: u16 = 80;
const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512;
#[cfg(unix)]
const DEFAULT_BLOCK_SIZE: u64 = 1024;
const DEFAULT_FILE_SIZE_BLOCK_SIZE: u64 = 1;

#[derive(Debug)]
enum LsError {
Expand Down Expand Up @@ -409,7 +409,9 @@ pub struct Config {
color: Option<LsColors>,
long: LongFormat,
alloc_size: bool,
block_size: Option<u64>,
file_size_block_size: u64,
#[allow(dead_code)]
block_size: u64, // is never read on Windows
width: u16,
// Dir and vdir needs access to this field
pub quoting_style: QuotingStyle,
Expand Down Expand Up @@ -822,6 +824,7 @@ impl Config {
let env_var_block_size = std::env::var_os("BLOCK_SIZE");
let env_var_ls_block_size = std::env::var_os("LS_BLOCK_SIZE");
let env_var_posixly_correct = std::env::var_os("POSIXLY_CORRECT");
let mut is_env_var_blocksize = false;

let raw_block_size = if let Some(opt_block_size) = opt_block_size {
OsString::from(opt_block_size)
Expand All @@ -831,6 +834,7 @@ impl Config {
} else if let Some(env_var_block_size) = env_var_block_size {
env_var_block_size
} else if let Some(env_var_blocksize) = env_var_blocksize {
is_env_var_blocksize = true;
env_var_blocksize
} else {
OsString::from("")
Expand All @@ -839,9 +843,16 @@ impl Config {
OsString::from("")
};

let block_size: Option<u64> = if !opt_si && !opt_hr && !raw_block_size.is_empty() {
let (file_size_block_size, block_size) = if !opt_si && !opt_hr && !raw_block_size.is_empty()
{
match parse_size_u64(&raw_block_size.to_string_lossy()) {
Ok(size) => Some(size),
Ok(size) => {
if is_env_var_blocksize {
(DEFAULT_FILE_SIZE_BLOCK_SIZE, size)
} else {
(size, size)
}
}
Err(_) => {
// only fail if invalid block size was specified with --block-size,
// ignore invalid block size from env vars
Expand All @@ -850,13 +861,19 @@ impl Config {
invalid_block_size.clone(),
)));
}
None
if is_env_var_blocksize {
(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
} else {
(DEFAULT_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
}
}
}
} else if env_var_posixly_correct.is_some() {
Some(POSIXLY_CORRECT_BLOCK_SIZE)
(DEFAULT_FILE_SIZE_BLOCK_SIZE, POSIXLY_CORRECT_BLOCK_SIZE)
} else if opt_si {
(DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000)
} else {
None
(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
};

let long = {
Expand Down Expand Up @@ -1062,6 +1079,7 @@ impl Config {
inode: options.get_flag(options::INODE),
long,
alloc_size: options.get_flag(options::size::ALLOCATION_SIZE),
file_size_block_size,
block_size,
width,
quoting_style,
Expand Down Expand Up @@ -2479,13 +2497,7 @@ fn get_block_size(md: &Metadata, config: &Config) -> u64 {
};
match config.size_format {
SizeFormat::Binary | SizeFormat::Decimal => raw_blocks,
SizeFormat::Bytes => {
if let Some(user_block_size) = config.block_size {
raw_blocks / user_block_size
} else {
raw_blocks / DEFAULT_BLOCK_SIZE
}
}
SizeFormat::Bytes => raw_blocks / config.block_size,
}
}
#[cfg(not(unix))]
Expand Down Expand Up @@ -2955,26 +2967,16 @@ fn display_len_or_rdev(metadata: &Metadata, config: &Config) -> SizeOrDeviceId {
return SizeOrDeviceId::Device(major.to_string(), minor.to_string());
}
}
// Reported file len only adjusted for block_size when block_size is set
if let Some(user_block_size) = config.block_size {
// ordinary division of unsigned integers rounds down,
// this is similar to the Rust API for division that rounds up,
// currently in nightly only, however once
// https://github.com/rust-lang/rust/pull/88582 : "div_ceil"
// is stable we should use that instead
let len_adjusted = {
let d = metadata.len() / user_block_size;
let r = metadata.len() % user_block_size;
if r == 0 {
d
} else {
d + 1
}
};
SizeOrDeviceId::Size(display_size(len_adjusted, config))
} else {
SizeOrDeviceId::Size(display_size(metadata.len(), config))
}
let len_adjusted = {
let d = metadata.len() / config.file_size_block_size;
let r = metadata.len() % config.file_size_block_size;
if r == 0 {
d
} else {
d + 1
}
};
SizeOrDeviceId::Size(display_size(len_adjusted, config))
}

fn display_size(size: u64, config: &Config) -> String {
Expand Down
47 changes: 38 additions & 9 deletions tests/by-util/test_ls.rs
View file
Expand Up @@ -3862,8 +3862,8 @@ fn test_posixly_correct_and_block_size_env_vars() {
.arg("-l")
.env("POSIXLY_CORRECT", "some_value")
.succeeds()
.stdout_contains_line("total 8");
//.stdout_contains(" 1024 "); // TODO needs second internal blocksize
.stdout_contains_line("total 8")
.stdout_contains(" 1024 ");

scene
.ucmd()
Expand All @@ -3886,8 +3886,8 @@ fn test_posixly_correct_and_block_size_env_vars() {
.arg("-l")
.env("BLOCKSIZE", "512")
.succeeds()
.stdout_contains_line("total 8");
//.stdout_contains(" 1024 "); // TODO needs second internal blocksize
.stdout_contains_line("total 8")
.stdout_contains(" 1024 ");
}

#[test]
Expand All @@ -3900,13 +3900,42 @@ fn test_ls_invalid_block_size() {
.stderr_is("ls: invalid --block-size argument 'invalid'\n");
}

// TODO ensure the correct block size is used when using -l because
// the output of "ls -l" and "BLOCK_SIZE=invalid ls -l" is different
#[cfg(all(unix, feature = "dd"))]
#[test]
fn test_ls_invalid_block_size_in_env_var() {
new_ucmd!().env("LS_BLOCK_SIZE", "invalid").succeeds();
new_ucmd!().env("BLOCK_SIZE", "invalid").succeeds();
new_ucmd!().env("BLOCKSIZE", "invalid").succeeds();
let scene = TestScenario::new(util_name!());

scene
.ccmd("dd")
.arg("if=/dev/zero")
.arg("of=file")
.arg("bs=1024")
.arg("count=1")
.succeeds();

scene
.ucmd()
.arg("-og")
.env("LS_BLOCK_SIZE", "invalid")
.succeeds()
.stdout_contains_line("total 4")
.stdout_contains(" 1 1 "); // hardlink count + file size

scene
.ucmd()
.arg("-og")
.env("BLOCK_SIZE", "invalid")
.succeeds()
.stdout_contains_line("total 4")
.stdout_contains(" 1 1 "); // hardlink count + file size

scene
.ucmd()
.arg("-og")
.env("BLOCKSIZE", "invalid")
.succeeds()
.stdout_contains_line("total 4")
.stdout_contains(" 1024 ");
}

#[cfg(all(unix, feature = "dd"))]
Expand Down

0 comments on commit a8a5931

Please sign in to comment.