src/lib.rs doctest fails on s390x / IBM System Z (big endian)

[decathorpe]

https://github.com/m4b/scroll/blob/master/src/lib.rs#L55

test src/lib.rs - (line 55) ... FAILED
(...)
failures:
---- src/lib.rs - (line 55) stdout ----
Test executable failed (exit code 101).
stderr:
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `173`,                                     # AD
 right: `190`', src/lib.rs:22:1                    # BE

All other tests and doctests pass. So it looks like the example data that's defined for the example is wrong / typo'd?

It's hard for me to tell with assertion is actually triggering the panic, because the line numbers inside the doctest are messed up (probably due to the #cfgs), but I think it's this one: assert_eq!(byte, 0xbe); (matching the 190 / BE "right" side in the assertion)

[m4b]

@decathorpe darn i thought i responded to this; so yea, this is an easy fix; the example is wrong :) and your analysis is correct:

//! let byte: u8 = bytes.pread(2).unwrap();
//! assert_eq!(byte, 0xbe);

this needs to be fixed; note we read at offset 2 in the bytes array; since this byte array is swapped for big/little machines, the assert is correct, since it will be different on the big / little machine.

I think this will fix:

//! #[cfg(target_endian = "little")]
//! assert_eq!(byte, 0xbe);
//! #[cfg(target_endian = "big")]
//! assert_eq!(byte, 0xad);

other examples that read at an offset into the big/little endian array also likely need to be fixed.

If you have the ability to test this, would you be able to make a PR with the fixes? :)

[decathorpe]

Thanks! I'll apply your suggested fix and if it works on all architectures I have access to, I'll prepare a PR.

[decathorpe]

Oh. Interesting. The first failure is fixed, but then the next one confuses me:

let be_number: u32 = bytes.pread_with(0, scroll::BE).unwrap();
assert_eq!(be_number, 0xdeadbeef);

This fails on s390x with this error message:

stderr:
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `4022250974`,
 right: `3735928559`', src/lib.rs:31:1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
    src/lib.rs - (line 55)

This assertion expects 0xdeadbeef but gets 0xefbeadde (which is the definition of bytes in the case of BE).
Shouldn't pread_with(0, scroll::BE) already account for the different byte order here, and return 0xdeadbeef?

[m4b]

Ah nope that’s correct return too :) so on little endian architectures it reads out correctly deadbeef if we read it out big endian style. On big endian architectures if we read it big endian then the bytes appear as they are left to right, which is efbeadde.

So it and the u16 read also will need cfgs to alter their output depending on whether it’s little or big. So basically same fix. Because the bytes themselves are basically the opposite of whichever arch would read them naturally in memory, each non zero offset non byte read of the bytes is swapped and opposite and needs a cfg

[m4b]

And the first example of reading the u32 without an endian specifier is correct because it defaults to host endianness so it is correct in both cases.

It might be good to swap the read snd not the assert so if it’s a big endian arch we read out LE and if little we read BE. That way in either case we assert it’s equal to deadbeef

Ditto for the u16 read

[decathorpe]

I'm apparently unable to figure out how to do this correctly ... sorry. LE / BE stuff always ties my brain in knots.
But if you come up with a patch that should work, I can at least test it on s390x hardware.

[m4b]

@decathorpe you can try latest master to see if the lib.rs doc tests now pass on this BE system; please reopen if you test and find other issues, and thanks again!