head: head_backwards for non-seekable files like /proc/* or fifos (named pipes) #5732
Conversation
|
GNU testsuite comparison: |
src/uu/head/src/head.rs
Outdated
| let n = match usize::try_from(n) { | ||
| Ok(value) => value, | ||
| Err(e) => { | ||
| show!(USimpleError::new( | ||
| 1, | ||
| format!("{e}: number of bytes is too large") | ||
| )); | ||
| return Ok(()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
can be simplify with something like
| let n = match usize::try_from(n) { | |
| Ok(value) => value, | |
| Err(e) => { | |
| show!(USimpleError::new( | |
| 1, | |
| format!("{e}: number of bytes is too large") | |
| )); | |
| return Ok(()); | |
| } | |
| }; | |
| let n = usize::try_from(n).map_err(|e| { | |
| show!(USimpleError::new(1, format!("{e}: number of bytes is too large"))); | |
| e | |
| })?; |
There was a problem hiding this comment.
I experimented with it. The problem: The proposed alternative is actually not 100% the same. It makes the function return with an Err(..) (which needs to be converted first). But originally it just prints the error and then returns with Ok().
I personally think this is a bit smelly anyway. But i moved this code just from a different location (cleanup commit will come soon).
There was a problem hiding this comment.
additionally: I actually can't test this part of code. On 64bit machine usize and u64 is the same. Thus this convertion will only fail on 32 bit (or lower if this exists here)
src/uu/head/src/head.rs
Outdated
| n: u64, | ||
| separator: u8, | ||
| ) -> std::io::Result<()> { | ||
| let n = match usize::try_from(n) { |
There was a problem hiding this comment.
as it seems to be the same, maybe create a function for this ?
There was a problem hiding this comment.
ok, i'll do some more experiments :-)
|
GNU testsuite comparison: |
Signed-off-by: Ulrich Hornung <hornunguli@gmx.de>
|
@sylvestre can you please check why android was failing? I guess its something with the infrastructure |
src/uu/head/src/head.rs
Outdated
| } | ||
|
|
||
| fn read_but_last_n_bytes(input: &mut impl std::io::BufRead, n: usize) -> std::io::Result<()> { | ||
| fn checked_convert_to_usize_or_print_error(n: u64) -> Option<usize> { |
There was a problem hiding this comment.
please rename this function to explain the goal, not the implementation
like
get_bytes()
There was a problem hiding this comment.
The function is used at two places. Once for bytes and once for lines. So we could call it 'get_number_of_bytes_or_lines'.
But to be honest, I do currently not understand why this should improve the readability. The only reason for this function to be called is that the downcast from u64 to usize might not be possible if we are on a 32 bit platform and the user wants us to print all but last with very large number.
Could you please elaborate your proposal a bit more?
There was a problem hiding this comment.
I provided a new proposal. Let me know if your good with it.
|
GNU testsuite comparison: |
…s_nonseekable_files # Conflicts: # src/uu/head/src/head.rs
|
GNU testsuite comparison: |
|
small different with upstream: $ ./target/debug/coreutils head -c -1 /sys/kernel/profiling |
works now. How did you find this? |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
excellent :) |
this test tests/head/head-c :) |
addresses issue #5703
/sys/kernel/profiling: File is seekable but metadata of file provides wrong size information (4096). Apply GNU compatible special handling for small files.