Derive macro calculates incorrect bit size when using a non-inclusive `bytes="x..y"` field attribute

[c-to-the-l]

Consider the following code and the two almost identical structs. One uses 0..=1 while the other uses 0..2. For the first case, the code works as expected. For the second case, it calculates the field size to be 8 bits instead of 16 bits.

use packed_struct::prelude::*;

#[derive(Debug, Clone, PackedStruct)]
#[packed_struct(endian="lsb", bit_numbering="msb0", size_bytes="2")]
pub struct StructA {
    /// works okay
    #[packed_field(bytes="0..=1", size_bytes="2")]
    field_a: u16,
}

#[derive(Debug, Clone, PackedStruct)]
#[packed_struct(endian="lsb", bit_numbering="msb0", size_bytes="2")]
pub struct StructB {
    /// doesn't work
    #[packed_field(bytes="0..2", size_bytes="2")]
    field_b: u16,
}

I would expect that 0..2 would behave identically to 0..=1, however they result in different code (and actually the 0..2 case causes a compiler error).

Compare the generated implementation of unpack for the two structs:

Generated StructA implementation, good, generates Bits<16>:

    #[inline]
    #[allow(unused_imports, unused_parens)]
    fn unpack(
        src: &Self::ByteArray,
    ) -> ::std::result::Result<StructA, ::packed_struct::PackingError> {
        use ::packed_struct::*;
        let field_a = {
            let bytes = {
                let mut b = [0; (2usize - 0usize)];
                b[..].copy_from_slice(&src[0usize..2usize]);
                b
            };
            use ::packed_struct::types::*;
            use ::packed_struct::types::bits::*;
            let res: ::std::result::Result<
                LsbInteger<_, _, Integer<u16, Bits<16>>>,
                PackingError,
            > = <LsbInteger<_, _, _>>::unpack(&bytes);
            let unpacked = res?;
            **unpacked
        };
        Ok(StructA { field_a })
    }

Generated StructB implementation - bad, uses Bits<8>:

    fn unpack(
        src: &Self::ByteArray,
    ) -> ::std::result::Result<StructB, ::packed_struct::PackingError> {
        use ::packed_struct::*;
        let field_b = {
            let bytes = {
                let mut b = [0; (1usize - 0usize)];
                b[..].copy_from_slice(&src[0usize..1usize]);
                b
            };
            use ::packed_struct::types::*;
            use ::packed_struct::types::bits::*;
            let res: ::std::result::Result<
                MsbInteger<_, _, Integer<u16, Bits<8>>>,
                PackingError,
            > = <MsbInteger<_, _, _>>::unpack(&bytes);
            let unpacked = res?;
            **unpacked
        };
        Ok(StructB { field_b })
    }

[c-to-the-l]

I think the issue is here, in the use of (end-1) when it should probably be end: https://github.com/hashmismatch/packed_struct.rs/blob/master/packed_struct_codegen/src/pack_parse_attributes.rs#L226

Looking higher up in the code, the parsing for the inclusive ..= range uses (end+1), so an exclusive .. range should use end and not end-1 as the index only needs to be offset by one, and not two.

Edit: Looking closer, this would not be an acceptable fix, as it is correct when parsing bit ranges and the code is shared for both bits and bytes. I think the usage of the multiplier here is the trap, since it results in this incorrect behaviour when using it in bytes= mode

In addition, this test looks like it is incorrect https://github.com/hashmismatch/packed_struct.rs/blob/master/packed_struct_codegen/src/pack_parse_attributes.rs#L268 - I would expect a bytes range of 1..3 to be equivalent to bits 8..=23 however the test case validates against 8..=15, which is incorrect as it is only 1 byte instead of 2

[fahimmehraj]

I was bashing my head against a wall trying to figure this bug out. Thanks!