Skip to content
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.

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

Enable large disk image support #2301

Merged
merged 5 commits into from Feb 27, 2023
Merged

Enable large disk image support #2301

merged 5 commits into from Feb 27, 2023

Conversation

shermp
Copy link
Collaborator

@shermp shermp commented Feb 25, 2023

This PR enables large HDD disk images to be used with Dosbox-staging. It is similar to the large disk support in the fat32 testing branches, but I have taken a slightly different approach.

This PR does not implement IDE support for hard disks, nor does it enable FAT 32 support. It is sufficient to mount and boot large disk images using the -fs none option in imgmount.

@ThomasEricB you might be interested in testing this.

@shermp shermp requested a review from kcgen February 25, 2023 05:30
@ThomasEricB
Copy link
Contributor

Thank you @shermp ! I'll get to test it as soon as @kcgen reviews the code or creates a new branch!

@kcgen kcgen added the enhancement New feature or enhancement of existing features label Feb 25, 2023
Also add a 'cross_' prefix to the defines.
@shermp
Copy link
Collaborator Author

shermp commented Feb 25, 2023

As suggested by @kcgen I have moved the fstream wrappers to cross.h so they can be used by more areas of dosbox in the future.

include/cross.h Show resolved Hide resolved
include/cross.h Show resolved Hide resolved
src/dos/drive_fat.cpp Outdated Show resolved Hide resolved
src/dos/program_imgmount.cpp Outdated Show resolved Hide resolved
src/dos/program_imgmount.cpp Outdated Show resolved Hide resolved
src/dos/program_imgmount.cpp Outdated Show resolved Hide resolved
src/ints/bios_disk.cpp Outdated Show resolved Hide resolved
src/ints/bios_disk.cpp Outdated Show resolved Hide resolved
@kcgen
Copy link
Member

kcgen commented Feb 25, 2023

looks great, @shermp - only minor comments (and many repeating, so hopefully not too tedious).

Another thing I wanted to mention for the casts is that @FeralChild64 added some really nice clamp-to-type constexpr routines:

include/math_utils.h:constexpr int8_t clamp_to_int8(const T val)
include/math_utils.h:constexpr uint8_t clamp_to_uint8(const T val)
include/math_utils.h:constexpr int16_t clamp_to_int16(const T val)
include/math_utils.h:constexpr uint16_t clamp_to_uint16(const T val)
include/math_utils.h:constexpr int32_t clamp_to_int32(const T val)
include/math_utils.h:constexpr uint32_t clamp_to_uint32(const T val)

They're especially useful to prevent value wrap-around (if clamping is acceptable).

If both clamping and wrap-around are unacceptable, then check_cast<type> will catch out-of-bounds values in debug builds (which standard-cast won't), while being equal to a normal type-cast in release builds.

@shermp
Copy link
Collaborator Author

shermp commented Feb 26, 2023

I've made some changes @kcgen , see what you think.

Copy link
Member

@kcgen kcgen left a comment

Choose a reason for hiding this comment

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

Just minor bits and a a question.
Should be done after this round.
This is a big improvement @shermp , and provides very useful functions that can be used in many more places.

include/support.h Show resolved Hide resolved
src/misc/support.cpp Outdated Show resolved Hide resolved
src/misc/support.cpp Outdated Show resolved Hide resolved
src/dos/program_imgmount.cpp Show resolved Hide resolved
src/misc/support.cpp Show resolved Hide resolved
src/misc/support.cpp Outdated Show resolved Hide resolved
@kcgen kcgen self-requested a review February 26, 2023 04:22
Copy link
Member

@kcgen kcgen left a comment

Choose a reason for hiding this comment

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

If you can squash down the intermediate commits, that would tidy it up a bit.

Feel free to merge once you're happy w/ the last round (it should let you merge once it gets through CI and all discussions set to "resolved")

Add stdio file size functions

Use stdio file size functions

Use const auto

Rename fstream_ functions to stdio_

Add missing return statement

Remove divisor variables only used once

The function names should be enough to explain the values

Switch cast to int64_t

Restore file position after finding size
@kcgen kcgen merged commit 78993a2 into main Feb 27, 2023
@shermp shermp deleted the sp/large-file-1 branch February 27, 2023 16:08
@johnnovak johnnovak added the DOS Issues related to DOS integration or DOS commands label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOS Issues related to DOS integration or DOS commands enhancement New feature or enhancement of existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants