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

Allow the GDT to be of any length #360

Merged
merged 2 commits into from
Mar 27, 2022
Merged

Allow the GDT to be of any length #360

merged 2 commits into from
Mar 27, 2022

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented Mar 27, 2022

This uses a const generic MAX parameter to specify the length. This is arguably a breaking change as it bumps the MSRV to 1.59 though its use of default const generics. It also can make some function call require additional type annotations:

// Before
let gdt = GlobalDescriptorTable::from_raw_slice(slice);
// After
let gdt = GlobalDescriptorTable::<8>::from_raw_slice(slice);

Fixes #333

Signed-off-by: Joe Richey joerichey@google.com

@josephlr josephlr requested review from phil-opp and Freax13 March 27, 2022 06:57
@josephlr josephlr mentioned this pull request Mar 27, 2022
13 tasks
Copy link
Member

@Freax13 Freax13 left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, only two minor things that need to be improved.

src/structures/gdt.rs Outdated Show resolved Hide resolved
src/structures/gdt.rs Outdated Show resolved Hide resolved
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr josephlr requested a review from Freax13 March 27, 2022 22:38
@josephlr
Copy link
Contributor Author

@Freax13 I updated the comments and messages. I think even with @phil-opp's approval I also need yours to merge this.

@josephlr josephlr enabled auto-merge March 27, 2022 22:40
Copy link
Member

@Freax13 Freax13 left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines +51 to 54
pub struct GlobalDescriptorTable<const MAX: usize = 8> {
table: [u64; MAX],
next_free: usize,
}
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit too late, but I just noticed that this design allows creating GDT's without any entries, but a GDT must always have at least a null entry, doesn't it?

josephlr added a commit that referenced this pull request Mar 29, 2022
The GDT can have a maxium length of 2^16 bytes, and must contain at
least one null descriptor. As `MAX` counts the number of `u64` entries,
we must have `0 < MAX <= 2^13`.

Unfortunely, we cannot do this check with a `where` clause, as
`feature(generic_const_expers)` is not yet stable. However, we can do
this check with an `assert!` in `GlobalDescriptorTable::empty()`, which
is a `const fn`.

Pointed out by @Freax13 in
#360 (comment)

Signed-off-by: Joe Richey <joerichey@google.com>
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.

3 participants