std::io::Cursor::position behavior changed in the beta

[pietroalbini]

Seems like the behavior of std::io::Cursor::position changed, because the test suite of rmp now fails on beta and nightly while working fine on stable (crater log). One example of such an error:

#[test]
fn from_str32_read_str_len_eof() {
    let buf: &[u8] = &[0xdb, 0x00, 0x00, 0x00];
    let mut cur = Cursor::new(buf);

    read_str_len(&mut cur).err().unwrap();
    assert_eq!(4, cur.position());
}

---- func::decode::string::from_str32_read_str_len_eof stdout ----
        thread 'func::decode::string::from_str32_read_str_len_eof' panicked at 'assertion failed: `(left == right)`
  left: `4`,
 right: `1`', rmp/tests/func/decode/string.rs:120:5

My guess is this regression is related to #46485, which is the only pull request that changed the behavior for Cursor<&[u8]>.

cc @3Hren

[3Hren]

Not sure it's the Cursor's issue. The code is: https://github.com/3Hren/msgpack-rust/blob/master/rmp/src/decode/str.rs#L75

Seems like either byteorder can now read atomically, or Read::read_exact does the same. Both cases are valid, but forces me to rewrite test conditions.

[alexcrichton]

Thanks for the report @pietroalbini! @3Hren will this be difficult to update for you? I do think I agree that both cases are the same here in terms of correctness, but if it's difficult to update we could consider reverting.

The issue here I think can be boiled down to:

use std::io::{Cursor, Read};

fn main() {
    let mut io = Cursor::new(&[1, 2][..]);
    assert!(io.read_exact(&mut [0; 4]).is_err());
    println!("{}", io.position());
}

This currently prints 2 on stable (1.23.0) and 0 on beta. A side effect of #46485 is that a call to read_exact will fail without conuming bytes if there's not enough space, while before the bytes were consumed eagerly.

[3Hren]

It's okay for me, no need for reverting :)

[alexcrichton]

Ok! I'll bring this up with @rust-lang/libs team soon as well, but we're pretty likely to close this.

[alexcrichton]

The libs team discussed this today and concluded to close, but thanks regardless for the report @pietroalbini and the willingness to update @3Hren!