rust-osdev / bootimage

Tool to create bootable disk images from a Rust OS kernel.
Apache License 2.0
767 stars 67 forks source link

Pad boot image to block size #39

Closed toothbrush7777777 closed 5 years ago

toothbrush7777777 commented 5 years ago

Closes #35.

phil-opp commented 5 years ago

Thanks a lot! Looks good to me overall.

One small nit: Could you rename the first padding binding to something else, e.g. remainder? Then we wouldn't have two bindings with the same name but different semantics.

toothbrush7777777 commented 5 years ago

@phil-opp Done. I also formatted the changes as suggested by cargo fmt -- --check.

phil-opp commented 5 years ago

Thanks!

bors r+

bors[bot] commented 5 years ago

Canceled

toothbrush7777777 commented 5 years ago

@phil-opp Whoops, looks like I shouldn't have formatted the code further.

phil-opp commented 5 years ago

bors r+

phil-opp commented 5 years ago

bors r-

I think we can merge this manually (pending the CI checks).

bors[bot] commented 5 years ago

Canceled

phil-opp commented 5 years ago

Seems like the test failed on Windows:

Error: I/O error: failed to pad boot image to a multiple of the block size: Access is denied. (os error 5)

toothbrush7777777 commented 5 years ago

@phil-opp Any idea what the problem could be?

toothbrush7777777 commented 5 years ago

Finally passes all checks!

I found a comment which seems related:

.append(true) doesn't set all the bits of GENERIC_WRITE, it misses the FILE_WRITE_DATA bit as @retep998 mentioned. […]

https://github.com/rust-lang/rust/issues/54118#issuecomment-420299904

I guess that whatever API is used internally by Rust for File::set_len on Windows requires that flag. I've changed the code to use write(true) instead of append(true) and all the checks have now successfully passed.

phil-opp commented 5 years ago

Thanks for investigating! Let's get this merged!

phil-opp commented 5 years ago

Thanks a lot for tackling this! I will publish a new version tomorrow when I'm at my computer again.

toothbrush7777777 commented 5 years ago

@phil-opp Cool, that will be great.

phil-opp commented 5 years ago

Released as version 0.7.4.