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

Add HandlerFuncType trait #439

Merged
merged 2 commits into from Sep 24, 2023
Merged

Add HandlerFuncType trait #439

merged 2 commits into from Sep 24, 2023

Conversation

brandonchinn178
Copy link
Contributor

Fixes #438

@brandonchinn178 brandonchinn178 marked this pull request as ready for review September 23, 2023 22:40
/// This method is only usable with the `abi_x86_interrupt` feature enabled. Without it, the
/// unsafe [`Entry::set_handler_addr`] method has to be used instead.
#[inline]
pub fn set_handler_fn(&mut self, handler: &F) -> &mut EntryOptions {
Copy link
Contributor

Choose a reason for hiding this comment

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

For backwards compatibility, set_handler_fn needs to take the handler by value, not by reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha. I didn't realize that function pointers are Copy, so it doesn't matter if it's owned or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just for my own understanding (I'm fairly new to Rust), why does set_handler_fn take the handler function by value? If there's nothing in set_handler_fn that requires the function to be moved, isn't it better to take a reference so that the caller can keep using the argument? Or is it specifically because it's a function, there's never a reason to pass a function by reference, and you should always Copy as needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya this can be a confusing thing initially, especially because the function Traits (Fn, FnMut, FnOnce) are often handled differently.

A type like fn(u8) -> u8 or extern "x86-interrupt" fn(InterruptStackFrame) -> ! is a function pointer, so it references a specific function. It this way, it is similar to &T or *const T in that it is:

  • pointer sized
  • a shared reference to something immutable.

So for the same reason we usually don't use &&T or &*const T, using &fn(u8) -> u8 is uncommon.

src/structures/idt.rs Outdated Show resolved Hide resolved
src/structures/idt.rs Outdated Show resolved Hide resolved
@brandonchinn178
Copy link
Contributor Author

@josephlr how's that?

Signed-off-by: Joe Richey <joerichey@google.com>
Copy link
Contributor

@josephlr josephlr left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@josephlr josephlr merged commit d486e86 into rust-osdev:master Sep 24, 2023
12 checks passed
@brandonchinn178 brandonchinn178 deleted the handler-func-type branch September 24, 2023 03:46
@Freax13 Freax13 mentioned this pull request Jan 14, 2024
4 tasks
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 for Entry handler function types?
2 participants