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

fix signature of Step::steps_between implementations #513

Merged
merged 5 commits into from
Nov 29, 2024

Conversation

Freax13
Copy link
Member

@Freax13 Freax13 commented Nov 24, 2024

A recent nightly changed the signature of Step::steps_between to match the signature of Iterator::size_hint. This PR changes our implementations to match the new signature. This PR also fixes the Step implementation of Page on 32-bit platforms.

Closes #512
Cc @isaka-james @tsatke

@Freax13
Copy link
Member Author

Freax13 commented Nov 24, 2024

It also simplifies them by forwarding to the Step implementations of the wrapped types (e.g. VirtAddr calls the Step implementation of u64).

Oops, this obviously doesn't work on stable, duh. I'll revert back to the old implementations and fix them instead.

tsatke added a commit to tsatke/devos that referenced this pull request Nov 24, 2024
…2apic releases a new version that uses the fix
@Freax13
Copy link
Member Author

Freax13 commented Nov 24, 2024

Re @tsoutsman tsatke/devos@547946e:

force a toolchain version until #513 is merged and x2apic releases a new version that uses the fix

We'll likely do a minor release (we can't do patch releases because we're pre 1.0), so you won't have to wait on x2apic to bump its dependency on x86_64. Running cargo update -p x86_64 should suffice.

@Freax13 Freax13 force-pushed the fix/step-nightly-breakage branch 3 times, most recently from 7df91fa to 3700f6d Compare November 24, 2024 08:45
@Freax13 Freax13 marked this pull request as draft November 24, 2024 08:45
@Freax13 Freax13 force-pushed the fix/step-nightly-breakage branch 3 times, most recently from f14c81e to c5843de Compare November 24, 2024 09:03
@Freax13 Freax13 marked this pull request as ready for review November 24, 2024 09:03
src/addr.rs Outdated Show resolved Hide resolved
A recent nightly changed the signature of Step::steps_between to match
the signature of Iterator::size_hint. This patch changes our
implementations to match the new signature.
Without the as_slice call, the value passed to from_ptr was &[i32; 5]
which is *not* a slice.
Previously, we scaled count as a usize, not a u64. This is bad because
stepping forward by 0x10_0000 is perfectly fine, yet we were always
rejecting this because 0x10_0000 * 0x1000 doesn't fit in usize on
32-bit platforms. Similarly, we would fail to return a step count above
or equal to 0x10_0000 because the call to
<VirtAddr as Step>::steps_between would fail.
Never scale usize values, instead, always scale u64 values. To make
this easier to work with, this patch adds variants of the Step
functions to VirtAddr that take and return u64 instead of usize.
This patch also adds some regression tests.
@bjorn3
Copy link

bjorn3 commented Nov 24, 2024

This should probably also be backported to the 0.14 series. Blog os depends on bootloader 0.9 which uses x86_64 0.14.

Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thank you!

@Freax13 Freax13 merged commit 51d602c into master Nov 29, 2024
11 of 13 checks passed
@Freax13 Freax13 deleted the fix/step-nightly-breakage branch November 29, 2024 16:35
@Freax13
Copy link
Member Author

Freax13 commented Nov 29, 2024

I'm preparing a release now. I'll work on the backport tomorrow.

@Freax13
Copy link
Member Author

Freax13 commented Nov 29, 2024

0.15.2 release PR is here: #519

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trait Mismatch Error When Building the Project with cargo build
4 participants