Support array sizes over 32 about protocol HOT 8 CLOSED

Hey there,

I don't think this is possible at the moment, but it'd be very easy to add.

Any type which implements Parcel can be used - at the moment, it looks like there aren't any implementations of Parcel for arrays, only Vecs.

Here is the Parcel implementation for Vec<T>.

I will add implementations for all array sizes up to 32, and then include 32, 64, 128, etc.

from protocol.

Context: All types that implement Parcel must also implement Clone.

It looks like the builtin implementation of Clone for array types requires T: Copy.

I think that this is overly limiting for this library. It makes sense to support sending/receiving [char; 16], I see no reason why we shouldn't also support [MyArbitraryNonCopyableType; 16].

It would probably require quite an extra trait on our end to work around this. It looks like the problem is described in more detail in this discourse thread.

I'm going to commit it anyway because if we decided to remove the Copy restriction in the future, it would not be a breaking change.

from protocol.

Fixed in 960aaff.

Because Rust only implements Debug and PartialEq for array sizes up to 32, only arrays this size or smaller can be used.

from protocol.

Also, I've release the new commit(s) to crates.io.

from protocol.

That was incredibly fast, thanks!

Everything will be fine when serializing a parcel; the issue I'm seeing with the 32 size constraint is that sized arrays must be used when the size of a vector of parcels should be explicitly sized at compile-time.

That's opening a wholly different kind of box. I'm thinking about an example case where a parcel would have a sized array of, say, comment fields, which must not exceed an explicit number of items. If a vector of u8 is at the end of a buffer, this shouldn't be a problem; but it will be once it is in the middle of a parcel (and exceeding a size of 32).

In the following code, segment_commands of a Macho binary is a vector of SegmentCommand parcels:

define_packet!(Packet {
    header: MachHeader,
    segment_commands: Vec<SegmentCommand>,
    segment_data: Vec<u8>
});

define_packet!(MachHeader {
    magic: u32,
    cputype: i64,
    cpusubtype: i64,
    filetype: u32,
    ncmds: u32,
    sizeofcmds: u32,
    flags: u32
});

define_packet!(SegmentCommand {
    cmd: u32,
    cmdsize: u32,
    segname: [u8; 16],
    vmaddr: u32,
    vmsize: u32,
    fileoff: u32,
    filesize: u32,
    maxprot: i64,
    initprot: i64,
    nsects: u32,
    flags: u32
});

That works just fine.

Now imagine a crypto protocol:

define_packet!(ED25519);

define_packet_kind!(Mode: u32 {
    0x00 => ED25519
});

define_packet!(Packet {
    mode: Mode,
    public_key: [u8; 32],
    signature: [u8; 64],
    data: Vec<u8>
});

Especially when porting parsers from C having big sized arrays is useful. Is there any way this could be implemented using macro-magic?

🍺

from protocol.

We cannot manually implement PartialEq, Debug and friends for arrays ourselves because Rust's impl rules forbid implementing an external trait for an external type, which makes sense but can be a pain sometimes.

One way to get it working could be to wrap the array type in a custom type. With this, we could probably write a macro to make it easier

#[derive(Clone)]
struct Wrapper<T> {
    inner: [T; 64],
}

impl PartialEq for Wrapper ...
impl Debug for Wrapper ...

EDIT: I think it will be possible to remove the Debug and PartialEq constraints because we only ever use these traits on concrete types in the crate.

from protocol.

Looks like I was building the wrong repository!

I've looked more into it and I think that we can still remove the constraints, but it will be a bit more work.

The only uses of Clone (from a very quick look) at the moment are hacky workarounds for simply cloning Vec<T> or List<T> to Dyn<T> to make serialisation easier and reduce duplication.

It is definitely possible to rewrite this so that we have shared generic helper functions that are used by the implementations of both Vec and DynArray.

I likely won't have the time to work on this at the moment though, but PRs are definitely welcome :)

from protocol.

Fixed in version v0.3.1

from protocol.