bmap::copy also can truncate file #26
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
obbardc
suggested changes
Sep 30, 2022
bmap/src/lib.rs
Outdated
| @@ -4,6 +4,7 @@ mod discarder; | |||
| pub use crate::discarder::*; | |||
| use sha2::{Digest, Sha256}; | |||
| use thiserror::Error; | |||
| use nix::unistd::ftruncate; | |||
There was a problem hiding this comment.
suggest to split this into two commits:
- to move the ftruncate into the
bmap::copyfunction, - to call ftruncate only on a file :-)
bmap/src/lib.rs
Outdated
| @@ -41,6 +42,11 @@ where | |||
| HashType::Sha256 => Sha256::new(), | |||
| }; | |||
|
|
|||
| //truncate if the target is a file and not a block device | |||
| match output { | |||
| std::fs::OpenOptions => ftruncate(output.as_raw_fd(), map.image_size() as i64).context("Failed to truncate file")?, | |||
Razaloc
force-pushed
the
wip/rafaelgarrui/cleanmain
branch
from
cf31930
to
65918a5
Compare
|
Right now |
Razaloc
force-pushed
the
wip/rafaelgarrui/cleanmain
branch
2 times, most recently
from
8c6d0f5
to
ecefb62
Compare
Razaloc
commented
Oct 3, 2022
| @@ -104,8 +102,6 @@ fn copy(c: Copy) -> Result<()> { | |||
| .create(true) | |||
| .open(c.dest)?; | |||
There was a problem hiding this comment.
Here I think it is already being assumed that the output is going to be a file. Should this be absorbed by the copy library function too and only be executed if the output is actually a file ?
ftruncate was executed into the main function and it should be done into the copying process Signed-off-by: Rafael Garcia Ruiz <[email protected]>
Razaloc
force-pushed
the
wip/rafaelgarrui/cleanmain
branch
from
ecefb62
to
c25e939
Compare
sjoerdsimons
reviewed
Oct 4, 2022
| @@ -35,12 +37,14 @@ pub enum CopyError { | |||
| pub fn copy<I, O>(input: &mut I, output: &mut O, map: &Bmap) -> Result<(), CopyError> | |||
| where | |||
| I: Read + SeekForward, | |||
| O: Write + SeekForward, | |||
| O: Write + SeekForward + AsRawFd, | |||
There was a problem hiding this comment.
This is a too heavy requirement as it will limit where we can use the copy function; E.g. that blocks using it if the output is not something that can be a raw file (e.g. if it's a usb protocol where we write to some on-device flash)
Razaloc
added a commit
that referenced
this pull request
Oct 5, 2022
helpers mod contains functions and structs needed to setup common inputs/outputs Closes #26 Signed-off-by: Rafael Garcia Ruiz <[email protected]>
Razaloc
added a commit
that referenced
this pull request
Oct 5, 2022
helpers mod contains functions and structs needed to setup common inputs/outputs Closes #26 Signed-off-by: Rafael Garcia Ruiz <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ftruncate was executed into the main function and it should be done into the copying process if it's needed
Closes: #24
Signed-off-by: Rafael Garcia Ruiz [email protected]