Provide Try{Into|From}Ctx impls for fixed byte array.

[Frostie314159]

This PR provides implementations for reading and writing [u8; N].

[m4b]

needs rustfmt

[m4b]

Also i'm curious how you got this building? Last time I tried this, I got overlapping impl errors, iirc

[Frostie314159]

Do you mean, the last time you tried implementing this?

[m4b]

Do you mean, the last time you tried implementing this?

Yea, last time i tried doing what you wrote basically, I recall I got a duplicate impl error due to a blanket impl

[Frostie314159]

Interesting.

[Frostie314159]

The CI failed, due to an out of date compiler.

[m4b]

looks like CI Is failing because rayon bumped their minimum rustc, i'd just update the compiler in CI config since it isn't a real dep

[m4b]

technically speaking, this will also bump the msrv to whatever the version of rust const generics landed, which is fine, but we should probably but an MSRV in cargo.toml?

[Frostie314159]

I just looked up in which release this was stabilized and found 1.51. However the CStr::from_bytes_until_nul function I use in #88 was only stabilized in 1.69. So we should take that into consideration.

[m4b]

ah, 1.69 is quite new. So I generally like to keep the MSRV as low as possible (and change it as little as possible), to avoid breaking people (like rayon just did). This is also used in goblin which has an explicit MSRV of 1.60, so that would cause a bump of that (at very least) if we bumped to 1.69

We can punt on the bytes function for now, and merge your other stuff though; also switching to a safe function in std library might be worth the bump along the way to 1.0

[Frostie314159]

We could likely resolve this, by including memchr crate, which would allows us to use the CStr::from_bytes_with_nul, which has been stable since 1.10.

[m4b]

Yea, sorry, in general there is an unspoken "no deps in scroll" rule :) There was talk once of adding uuid as an optional dep (to allow the TryFrom impls to be in scroll itself), but that never panned out, and not sure if it was a good idea anyway/kind of niche.

[Frostie314159]

Ok. Until then I'm just gonna reuse the old iterator approach. If we should ever bump the version to 1.69 we can just switch to the other method.

[Frostie314159]

On the note of const generics, we could remove the size param from the ctx_impl!, by using core::mem::size_of::<T>.

[nyurik]

I fixed the CI - MSRV in #90

[Frostie314159]

@nyurik Thanks!

[Frostie314159]

Any further changes needed?

[nyurik]

you may want to rebase on top of main?

[Frostie314159]

I just rebased it, but there is one conflict remaining, which I can't fix.

[m4b]

why exactly can't you fix the conflict?

[m4b]

i fixed it, it was just the 0x4a vs 04a issue

[m4b]

looks like failing from a rustfmt now

[m4b]

@Frostie314159 apologies for the churn on this, would you mind rebasing? I would like to see this get merged

[Frostie314159]

It's done.

[nyurik]

@Frostie314159 i have no idea how, but for some reason this PR does not show diff relative to the current main branch, but instead relative to some older revision. As long as @m4b uses "squash and merge", it should be OK though. For the future, I think it is best to not use your main branch to create a PR - makes things a bit confusing and complicated. Instead, create a dedicated branch just for the PR you want to submit.

One more thing - it might simplify things for this PR if you squash all your changes, and make them on top of the current main branch - this way everyone would see just the changes you are submitting.

Thanks for hard work!

[m4b]

Yea, I'm very confused what is happening to these PRs, I've never seen github show all the changes like this, it makes it very difficult (and somewhat nerve wracking tbh) to review. I'm somewhat hesitant to merge 25 commits into master; if it doesn't show up as a single squashed commit, I'll likely have to force push the actual content of the PR, which I don't really want to do.

[nyurik]

I just created a new PR so that it is easy to see what is going on with this one - https://github.com/m4b/scroll/pull/93

It seems like it may need some work still :(

[nyurik]

P.S. I made some comments on the #93 , which is identical to this PR (and attributes to original author), but still has some issues. My recommendation: create a new branch from the current main branch of the m4b/scroll and change just the src/ctx.rs with this content. The rest we should review separately:

impl<'a, const N: usize> TryFromCtx<'a> for [u8; N] {
    type Error = error::Error;
    fn try_from_ctx(from: &'a [u8], _ctx: ()) -> Result<(Self, usize), Self::Error> {
        // Unwrap is OK, since pread_with already asserts the length.
        Ok((from.pread_with::<&'a [u8]>(0, N)?.try_into().unwrap(), N))
    }
}

impl<const N: usize> TryIntoCtx for [u8; N] {
    type Error = error::Error;
    fn try_into_ctx(self, from: &mut [u8], _ctx: ()) -> Result<usize, Self::Error> {
        from.pwrite(self.as_slice(), 0)
    }
}

impl<'a, const N: usize> TryIntoCtx for &'a [u8; N] {
    type Error = error::Error;
    fn try_into_ctx(self, from: &mut [u8], ctx: ()) -> Result<usize, Self::Error> {
        (*self).try_into_ctx(from, ctx)
    }
}

[Frostie314159]

I think it's best if I close these PRs and start fresh, from the current master and then first implement the clippy fixes, then the unsafe and then the array impls.

[Frostie314159]

Closed in favor of #95 .