Repository navigation
hexdump: end offset is the input size when -s skips past the end of files - #670
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
| Ok(file_size) => { | ||
| if remaining >= file_size { | ||
| remaining -= file_size; | ||
| let is_stdin = self.file_paths[self.current_file_index] == "/dev/stdin"; |
There was a problem hiding this comment.
comparing the path string is a bit fragile, hexdump /dev/stdin < file is treated as stdin here
could the reader remember that no file was given instead?
There was a problem hiding this comment.
Changed in 2b02bf8: the reader now remembers that no file was given, so a named /dev/stdin is handled like any other named file. util-linux does the same (hexdump -s 100 /dev/stdin < file prints 0000003), and there is a test for it now.
| self.current_file().seek(SeekFrom::Start(remaining)) | ||
| } else { | ||
| self.current_file() | ||
| .seek(SeekFrom::Current(remaining as i64)) |
There was a problem hiding this comment.
remaining as i64 wraps for a skip above i64::MAX, could use i64::try_from?
There was a problem hiding this comment.
Changed in 2b02bf8 to i64::try_from. A skip that does not fit is read instead of seeked.
|
|
||
| #[test] | ||
| #[cfg(target_os = "linux")] | ||
| fn test_hexdump_skip_device() { |
There was a problem hiding this comment.
why only linux? /dev/null and /dev/zero are there on all unix
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
| Ok(file_size) => { | ||
| if remaining >= file_size { | ||
| remaining -= file_size; | ||
| let reads_stdin = self.reads_stdin; |
There was a problem hiding this comment.
do we need this variable? self.reads_stdin in the guard should work
There was a problem hiding this comment.
Right, it works in the guard. Dropped it in f6d5307.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Thanks for your PR |
When
-sskips past the end of the named files, hexdump prints the requested skip as the last offset. util-linux 2.41.3 prints the size of the input, and nothing for an empty file:skip_bytesnow does what util-linux does: a regular file that the skip covers whole is skipped by its size, and the offset is the total size skipped. Stdin, devices and pipes are seeked or read forward, and the offset stays the requested skip, sohexdump -s 5 /dev/nullstill prints0000005andtest_hexdump_skip_beyond_fileis unchanged.New tests
test_hexdump_skip_beyond_named_files(fails without the change) andtest_hexdump_skip_device. The hexdump tests pass (24). Offsets match util-linux for-sfrom 3 to 2000 with-C,-xand the default format on empty and small files, several files,/dev/null,/dev/zero, a directory followed by a file, and redirected stdin.