From af7f4ab1f2e982be89296ff04a7b656fe88509ae Mon Sep 17 00:00:00 2001 From: jayhemnani9910 <193022578+jayhemnani9910@users.noreply.github.com> Date: Wed, 21 Oct 2026 03:30:57 +0530 Subject: [PATCH 1/4] hexdump: end offset is the input size when -s skips past the end of files --- src/uu/hexdump/src/hexdump.rs | 104 +++++++++++++++++++--------------- tests/by-util/test_hexdump.rs | 40 +++++++++++++ 2 files changed, 98 insertions(+), 46 deletions(-) diff --git a/src/uu/hexdump/src/hexdump.rs b/src/uu/hexdump/src/hexdump.rs index 115d036d..34c2a2d5 100644 --- a/src/uu/hexdump/src/hexdump.rs +++ b/src/uu/hexdump/src/hexdump.rs @@ -99,58 +99,72 @@ impl ChainedFileReader { offset } - fn skip_bytes(&mut self, bytes_to_skip: u64) { + /// Returns the offset the dump starts at. A skip past the end of named + /// regular files lands on their total size; stdin, devices and pipes are + /// seeked (or read) forward, so the offset is the requested skip. + fn skip_bytes(&mut self, bytes_to_skip: u64) -> u64 { let mut remaining = bytes_to_skip; + let mut skipped_whole_file = false; while remaining > 0 && self.ensure_current_file() { - match self.current_file().seek(SeekFrom::End(0)) { - Ok(file_size) => { - if remaining >= file_size { - remaining -= file_size; + let is_stdin = self.file_paths[self.current_file_index] == "/dev/stdin"; + let regular_size = match self.current_file().get_ref().metadata() { + Ok(metadata) if metadata.is_file() && !is_stdin => Some(metadata.len()), + _ => None, + }; + + if let Some(file_size) = regular_size { + if remaining >= file_size { + remaining -= file_size; + skipped_whole_file = true; + self.current_file = None; + self.current_file_index += 1; + continue; + } + } + + let seek = if regular_size.is_some() { + self.current_file().seek(SeekFrom::Start(remaining)) + } else { + self.current_file() + .seek(SeekFrom::Current(remaining as i64)) + }; + if seek.is_ok() { + return bytes_to_skip; + } + + // This file doesn't support seeking, fall back to dummy reads + while remaining > 0 { + let mut dummy_buf = vec![0u8; remaining.min(65536) as usize]; + match self.current_file().read(&mut dummy_buf) { + Ok(0) => { self.current_file = None; self.current_file_index += 1; - } else { - match self.current_file().seek(SeekFrom::Start(remaining)) { - Ok(_) => return, - Err(e) => { - uucore::show_error!( - "cannot seek '{}': {}", - self.file_paths[self.current_file_index], - e - ); - set_exit_code(1); - self.current_file = None; - self.current_file_index += 1; - } - } + break; } - } - Err(_) => { - // This file doesn't support seeking, fall back to dummy reads - while remaining > 0 { - let mut dummy_buf = vec![0u8; remaining.min(65536) as usize]; - match self.current_file().read(&mut dummy_buf) { - Ok(0) => { - self.current_file = None; - self.current_file_index += 1; - break; - } - Ok(n) => remaining -= n as u64, - Err(e) => { - uucore::show_error!( - "cannot read '{}': {}", - self.file_paths[self.current_file_index], - e - ); - set_exit_code(1); - self.current_file = None; - self.current_file_index += 1; - break; - } - } + Ok(n) => remaining -= n as u64, + Err(e) => { + uucore::show_error!( + "cannot read '{}': {}", + self.file_paths[self.current_file_index], + e + ); + set_exit_code(1); + self.current_file = None; + self.current_file_index += 1; + break; } } } + if remaining == 0 { + return bytes_to_skip; + } + } + + if skipped_whole_file { + bytes_to_skip - remaining + } else { + bytes_to_skip } } } @@ -222,9 +236,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { fn run_hexdump(options: HexdumpOptions) -> UResult<()> { let mut reader = ChainedFileReader::new(options.files.clone(), options.length); - reader.skip_bytes(options.skip); - - let mut offset = options.skip; + let mut offset = reader.skip_bytes(options.skip); let mut last_line: Vec = Vec::new(); let mut squeezing = false; diff --git a/tests/by-util/test_hexdump.rs b/tests/by-util/test_hexdump.rs index ed84ac9b..a20d5b84 100644 --- a/tests/by-util/test_hexdump.rs +++ b/tests/by-util/test_hexdump.rs @@ -301,6 +301,46 @@ fn test_hexdump_invalid_skip_and_length() { .stderr_contains("invalid length"); } +#[test] +fn test_hexdump_skip_beyond_named_files() { + let scene = TestScenario::new("hexdump"); + scene.fixtures.write("file1.txt", "abc"); + scene.fixtures.write("file2.txt", "def"); + scene.fixtures.write("empty.txt", ""); + + scene + .ucmd() + .args(&["-C", "-s", "100", "file1.txt", "file2.txt"]) + .succeeds() + .stdout_is("00000006\n"); + + scene + .ucmd() + .args(&["-s", "5", "file1.txt"]) + .succeeds() + .stdout_is("0000003\n"); + + scene + .ucmd() + .args(&["-s", "5", "empty.txt"]) + .succeeds() + .stdout_is(""); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_hexdump_skip_device() { + new_ucmd!() + .args(&["-s", "5", "/dev/null"]) + .succeeds() + .stdout_is("0000005\n"); + + new_ucmd!() + .args(&["-C", "-s", "5", "-n", "3", "/dev/zero"]) + .succeeds() + .stdout_is("00000005 00 00 00 |...|\n00000008\n"); +} + #[test] #[cfg(unix)] fn test_hexdump_skip_beyond_file() { From 758a21ec642b317b6af8a213aec50104acfd6cf7 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Wed, 21 Oct 2026 12:59:49 +0530 Subject: [PATCH 2/4] hexdump: run the device skip test on every Unix --- tests/by-util/test_hexdump.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_hexdump.rs b/tests/by-util/test_hexdump.rs index a20d5b84..ed069cf8 100644 --- a/tests/by-util/test_hexdump.rs +++ b/tests/by-util/test_hexdump.rs @@ -328,7 +328,7 @@ fn test_hexdump_skip_beyond_named_files() { } #[test] -#[cfg(target_os = "linux")] +#[cfg(unix)] fn test_hexdump_skip_device() { new_ucmd!() .args(&["-s", "5", "/dev/null"]) From 2b02bf8f038cd1509e76a2c5882f443f3b513ce1 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Wed, 21 Oct 2026 13:01:53 +0530 Subject: [PATCH 3/4] hexdump: tell stdin from a named /dev/stdin, and don't wrap a huge skip --- src/uu/hexdump/src/hexdump.rs | 30 ++++++++++++++++-------------- tests/by-util/test_hexdump.rs | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/uu/hexdump/src/hexdump.rs b/src/uu/hexdump/src/hexdump.rs index 34c2a2d5..26f249c7 100644 --- a/src/uu/hexdump/src/hexdump.rs +++ b/src/uu/hexdump/src/hexdump.rs @@ -16,6 +16,8 @@ use uucore::{ struct ChainedFileReader { file_paths: Vec, + /// No file was given, so the input is stdin + reads_stdin: bool, current_file: Option>, current_file_index: usize, remaining_bytes: Option, @@ -23,9 +25,11 @@ struct ChainedFileReader { } impl ChainedFileReader { - fn new(file_paths: Vec, length_limit: Option) -> Self { + fn new(file_paths: Option>, length_limit: Option) -> Self { Self { - file_paths, + reads_stdin: file_paths.is_none(), + // TODO: /dev/stdin doesn't work on Windows + file_paths: file_paths.unwrap_or_else(|| vec!["/dev/stdin".to_string()]), current_file: None, current_file_index: 0, remaining_bytes: length_limit, @@ -107,9 +111,9 @@ impl ChainedFileReader { let mut skipped_whole_file = false; while remaining > 0 && self.ensure_current_file() { - let is_stdin = self.file_paths[self.current_file_index] == "/dev/stdin"; + let reads_stdin = self.reads_stdin; let regular_size = match self.current_file().get_ref().metadata() { - Ok(metadata) if metadata.is_file() && !is_stdin => Some(metadata.len()), + Ok(metadata) if metadata.is_file() && !reads_stdin => Some(metadata.len()), _ => None, }; @@ -123,13 +127,13 @@ impl ChainedFileReader { } } - let seek = if regular_size.is_some() { - self.current_file().seek(SeekFrom::Start(remaining)) - } else { - self.current_file() - .seek(SeekFrom::Current(remaining as i64)) + let seeked = match regular_size { + Some(_) => self.current_file().seek(SeekFrom::Start(remaining)).is_ok(), + // A skip past i64::MAX can't be seeked, so it is read instead + None => i64::try_from(remaining) + .is_ok_and(|n| self.current_file().seek(SeekFrom::Current(n)).is_ok()), }; - if seek.is_ok() { + if seeked { return bytes_to_skip; } @@ -190,7 +194,7 @@ struct HexdumpOptions { length: Option, skip: u64, no_squeezing: bool, - files: Vec, + files: Option>, } #[uucore::main] @@ -225,11 +229,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { length: matches.get_one::("length").copied(), skip: matches.get_one::("skip").copied().unwrap_or(0), no_squeezing: matches.get_flag("no-squeezing"), - // TODO: /dev/stdin doesn't work on Windows files: matches .get_many::("files") - .map(|files| files.cloned().collect()) - .unwrap_or_else(|| vec!["/dev/stdin".to_string()]), + .map(|files| files.cloned().collect()), }; run_hexdump(options) } diff --git a/tests/by-util/test_hexdump.rs b/tests/by-util/test_hexdump.rs index ed069cf8..cebaa606 100644 --- a/tests/by-util/test_hexdump.rs +++ b/tests/by-util/test_hexdump.rs @@ -327,6 +327,21 @@ fn test_hexdump_skip_beyond_named_files() { .stdout_is(""); } +#[test] +#[cfg(unix)] +fn test_hexdump_skip_beyond_named_stdin() { + let scene = TestScenario::new("hexdump"); + scene.fixtures.write("file1.txt", "abc"); + + // Named, /dev/stdin is a regular file here, so the offset is its size + scene + .ucmd() + .args(&["-s", "100", "/dev/stdin"]) + .set_stdin(std::fs::File::open(scene.fixtures.plus("file1.txt")).unwrap()) + .succeeds() + .stdout_is("0000003\n"); +} + #[test] #[cfg(unix)] fn test_hexdump_skip_device() { From f6d530730b14c89719f9d9d46f6dc10464fbb764 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:16:00 +0530 Subject: [PATCH 4/4] hexdump: read reads_stdin in the guard directly --- src/uu/hexdump/src/hexdump.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/uu/hexdump/src/hexdump.rs b/src/uu/hexdump/src/hexdump.rs index 26f249c7..f2861049 100644 --- a/src/uu/hexdump/src/hexdump.rs +++ b/src/uu/hexdump/src/hexdump.rs @@ -111,9 +111,8 @@ impl ChainedFileReader { let mut skipped_whole_file = false; while remaining > 0 && self.ensure_current_file() { - let reads_stdin = self.reads_stdin; let regular_size = match self.current_file().get_ref().metadata() { - Ok(metadata) if metadata.is_file() && !reads_stdin => Some(metadata.len()), + Ok(metadata) if metadata.is_file() && !self.reads_stdin => Some(metadata.len()), _ => None, };