Allow arbitrary closures in start_core#31
Conversation
src/lib.rs
Outdated
| where | ||
| // TODO: change to FnOnce() -> ! when the never type is stabilized: | ||
| // https://github.com/rust-lang/rust/issues/35121 | ||
| F: FnOnce() + Send + 'static, |
There was a problem hiding this comment.
Nit: This constraint on F can go directly on the generic parameter declaration a few lines up, like the other parameters.
There was a problem hiding this comment.
I figured it makes sense to put this here given the constraint is a bit longer than others + there is quite a long comment, but I'm fine with both.
qwandor
left a comment
qwandor
left a comment
There was a problem hiding this comment.
I like this approach, very clever. Some minor comments below.
src/lib.rs
Outdated
| let rust_entry = ManuallyDrop::new(rust_entry); | ||
|
|
||
| let stack_bottom = stack.cast::<u8>(); | ||
| let align_offfset = stack.align_offset(core::mem::align_of::<F>()); |
There was a problem hiding this comment.
Shouldn't this be stack_bottom.align_offset(...)? Otherwise it will give you an offset in units of Stack<N>, not bytes. Though given that the stack already has a 4 KiB alignment it is unlikely that this would ever be nonzero.
There was a problem hiding this comment.
Ah, fair! Sad that align_offset/align_of don't return some sort of generic newtype (Alignment<T>(usize)) to avoid such issues.
I'm wondering how this slipped through my testing, but I guess I was lucky enough to always get valid and mapped memory addresses accidentally.
There was a problem hiding this comment.
Probably it was always returning 0 either way.
src/entry.rs
Outdated
There was a problem hiding this comment.
This is now the other way around. Either update this comment, or change the order of fields in StartCoreStack. I'd prefer the latter, because there's a small chance someone could be relying on the order.
There was a problem hiding this comment.
Sure, I've updated the struct then (and the comment a little bit to match the latest semantics).
2 participants
This changes
start_coreso that it receives an arbitraryFnOnceobject, rather than afn(u64)pointer. This makes the API more ergonomic, and most importantly, allows passing more than just oneu64argument to the secondary core.