Skip to content

hexdump: end offset is the input size when -s skips past the end of files #670

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.

Sign up for GitHub

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

Merged
sylvestre merged 4 commits into from
Oct 21, 2026
+120 −52
Merged

hexdump: end offset is the input size when -s skips past the end of files #670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af7f4ab
hexdump: end offset is the input size when -s skips past the end of f…
jayhemnani9910 Oct 20, 2026
758a21e
hexdump: run the device skip test on every Unix
jayhemnani9910 Oct 21, 2026
2b02bf8
hexdump: tell stdin from a named /dev/stdin, and don't wrap a huge skip
jayhemnani9910 Oct 21, 2026
f6d5307
hexdump: read reads_stdin in the guard directly
jayhemnani9910 Oct 21, 2026
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
117 changes: 65 additions & 52 deletions src/uu/hexdump/src/hexdump.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,20 @@ use uucore::{

struct ChainedFileReader {
file_paths: Vec<String>,
/// No file was given, so the input is stdin
reads_stdin: bool,
current_file: Option<BufReader<File>>,
current_file_index: usize,
remaining_bytes: Option<u64>,
open_error_count: usize,
}

impl ChainedFileReader {
fn new(file_paths: Vec<String>, length_limit: Option<u64>) -> Self {
fn new(file_paths: Option<Vec<String>>, length_limit: Option<u64>) -> 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,
Expand Down Expand Up @@ -99,58 +103,71 @@ 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 regular_size = match self.current_file().get_ref().metadata() {
Ok(metadata) if metadata.is_file() && !self.reads_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 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 seeked {
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
}
}
}
Expand All @@ -176,7 +193,7 @@ struct HexdumpOptions {
length: Option<u64>,
skip: u64,
no_squeezing: bool,
files: Vec<String>,
files: Option<Vec<String>>,
}

#[uucore::main]
Expand Down Expand Up @@ -211,20 +228,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
length: matches.get_one::<u64>("length").copied(),
skip: matches.get_one::<u64>("skip").copied().unwrap_or(0),
no_squeezing: matches.get_flag("no-squeezing"),
// TODO: /dev/stdin doesn't work on Windows
files: matches
.get_many::<String>("files")
.map(|files| files.cloned().collect())
.unwrap_or_else(|| vec!["/dev/stdin".to_string()]),
.map(|files| files.cloned().collect()),
};
run_hexdump(options)
}

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<u8> = Vec::new();
let mut squeezing = false;

Expand Down
55 changes: 55 additions & 0 deletions tests/by-util/test_hexdump.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,61 @@ 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(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() {

@sylvestre sylvestre Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why only linux? /dev/null and /dev/zero are there on all unix

@jayhemnani9910 jayhemnani9910 Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, it is cfg(unix) now (758a21e).

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() {
Expand Down
Loading