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

Tracking issue for stable Rust #1

Closed
HeroicKatora opened this issue Apr 10, 2019 · 4 comments · Fixed by #7
Closed

Tracking issue for stable Rust #1

HeroicKatora opened this issue Apr 10, 2019 · 4 comments · Fixed by #7

Comments

@HeroicKatora
Copy link
Member

The only blocking issue for compiling this on stable Rust seems to be the dependency zerocopy. It requires an unstable feature to provide interpreting RefCell derived Ref and RefMut as byte buffers that is not used here.

@joshlf who advertised the library originally in image-rs/image#885 , I suppose this could also be relevant as user experience. There are a few other utils that required additional implementation and might be useful in zerocopy in general, if you find them interesting. Since I don't know where else to submit these issues, we can potentially discuss them here, on gitter or any other place? The two methods are:

  • align_to: Just like [T]::align_to but safe. Note that a better implementation could require ptr::align_offset which is not yet stable.

    canvas/src/buf.rs

    Lines 93 to 111 in 0c591e0

    fn align_to<B, T>(slice: B) -> (B, LayoutVerified<B, [T]>, B)
    where B: ByteSlice, T: FromBytes
    {
    let align = mem::align_of::<T>();
    let addr = slice.as_ptr() as usize;
    // Calculate the next aligned address. We don't particularly care about overflows as they do
    // not invalidate modular arithmetic over the field with size power of two, and any actual
    // overflows in the pointers should be handled properly by the split implementation of `slice`.
    let next = addr.wrapping_add(align).wrapping_sub(1) & !(align - 1);
    let padding = next.wrapping_sub(addr);
    let (pre, slice) = slice.split_at(padding);
    // Now calculate the remaining length for the slice.
    let (slice, post) = prefix_slice::<_, T>(slice);
    let layout = LayoutVerified::new_slice(slice).unwrap_or_else(
    || unreachable!("Verified len and align"));
    (pre, layout, post)
    }
  • prefix_slice: Just like the single element methods but for a slice layout.

    canvas/src/buf.rs

    Lines 113 to 118 in 0c591e0

    fn prefix_slice<B, T>(slice: B) -> (B, B) where B: ByteSlice
    {
    let size = mem::size_of::<T>();
    let len = (slice.len() / size) * size;
    slice.split_at(len)
    }
@HeroicKatora HeroicKatora changed the title Stable Rust Tracking issue for stable Rust Apr 10, 2019
@joshlf
Copy link

joshlf commented Apr 10, 2019

It requires an unstable feature to provide interpreting RefCell derived Ref and RefMut as byte buffers that is not used here.

That's been stabilized in nightly, and will be stabilized in stable as soon as the next stable release lands.

There are a few other utils that required additional implementation and might be useful in zerocopy in general, if you find them interesting. Since I don't know where else to submit these issues, we can potentially discuss them here, on gitter or any other place?

That's very useful, thanks! Yeah, discussing them here is fine; unfortunately Fuchsia doesn't have a public bug tracker yet, which makes these sorts of things difficult.

@joshlf
Copy link

joshlf commented Apr 10, 2019

align_to sounds totally feasible. I'd be happy to add it. I'm not sure I understand prefix_slice, though. It doesn't seem to use any unsafe code; is it just a convenience thing?

@HeroicKatora
Copy link
Member Author

HeroicKatora commented Apr 10, 2019

That's been stabilized in nightly, and will be stabilized in stable as soon as the next stable release lands.

Ah, I didn't know that already completed its FCP. Must have missed it in the weekly summary.

Yes, prefix_slice is mostly convenience with a pinch of API consistency. There is new, which requires and exact size, and new_from_prefix which does not but instead gives back the remainder. But for slices there is only new_slice but no equivalent to get to pass a block that is not a multiple of the required size. This helper was quite applicable to two rather different places in the code, so I extracted it into a function and figured it could be useful in general.

For align_to it could also be useful to have one which only provides Option<(B, LayoutVerified<T, B>, B) instead of a verified slice. This would be the case when one wants to manually segment memory for a repr(C) struct, I think.

@HeroicKatora
Copy link
Member Author

Whoops, messed up the name when responding and referred to align_to instead of prefix_slice. It should now actually make sense.

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 a pull request may close this issue.

2 participants