More specific error types

[not-my-profile]

Hey, thanks for your library :) I want to use the manual slicing methods but noticed that they all return the ReadError enum, which contains variants that cannot actually occur.

For example with Ethernet2HeaderSlice.from_slice the only error variant that can actually occur is UnexpectedEndOfSlice.

Would you accept a PR changing ReadError to something like the following?

pub enum ReadError {
    IoError(Error),
    UnexpectedEndOfSlice(UnexpectedEndOfSliceError),
    DoubleVlan(DoubleVlanError),
    IpUnsupportedVersion(u8),
    Ipv4(Ipv4Error),
    Ipv6(Ipv6Error),
    IpAuthenticationHeader(IpAuthenticationHeaderError),
    Tcp(TcpError),
}

• Ethernet2HeaderSlice.from_slice would then return a Result<Ethernet2HeaderSlice<'a>, UnexpectedEndOfSliceError>
• Ipv4HeaderSlice.from_slice would then return a Result<Ipv4HeaderSlice<'a>, Ipv4Error>
• etc.

[JulianSchmid]

Hi @not-my-profile,

That is a good idea. We could even add a From implementation to enable auto conversion to read error to be backwards compatible.

Greets
Julian

[JulianSchmid]

Would you accept a PR changing ReadError to something like the following?

Yes of course. But I can not guarantee you how fast I can merge it. Last time one week escalated into 2 years, but I don't think this will happen with this :) .

Greets
Julian

[not-my-profile]

Thanks, for the quick answer :) Could you format your code with cargo fmt beforehand?

[JulianSchmid]

I decided agains using rust fmt in this library as it made the testcases where raw bytes are used harder to read (e.g.

etherparse/tests/transport/tcp.rs

Line 1473 in 36f8f7f

KIND_NOOP,

).

[not-my-profile]

Ah yeah ... adding #[rustfmt::skip] everywhere is cumbersome and #![rustfmt::skip] doesn't work on stable yet.

[JulianSchmid]

Hi, I had a closer look at the error types and I will probably restructure them quite a bit in the next release. Specifically also splitting them up based on the operation: read, from_slice, write, to_slice. At the same time I will try to split the error types so that an operation only returns an error type with values it can actually trigger.

That should hopefully also resolve your request, but will allow me to separate std::io::Error from the rest. std::io::Error has always been a bit of a thorn in my side, as it does not implement Eq or PartialEq (which makes writing tests a bit more verbose). Additionally std::io::Error is not available when building with no_std, which I really would like to support.

[JulianSchmid]

Finalized by #64