byteorder icon indicating copy to clipboard operation
byteorder copied to clipboard

improve byteorder docs for ReadBytesExt/WriteBytesExt

Open rivertam opened this issue 6 years ago • 7 comments

I believe I'm doing this correctly. I'm trying to create an f64 from 4 u16s.

extern crate byteorder;

use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};

// hello
fn main() {
    let mut cursor = std::io::Cursor::new(vec![]);
    cursor.write_u16::<LittleEndian>(0u16).unwrap();
    cursor.write_u16::<LittleEndian>(1u16).unwrap();
    cursor.write_u16::<LittleEndian>(2u16).unwrap();
    cursor.write_u16::<LittleEndian>(3u16).unwrap();

    println!("{:?}", cursor);

    let f = cursor.read_f64::<LittleEndian>().unwrap();
    println!("{}", f);
}

My println outputs Cursor { inner: [0, 0, 1, 0, 2, 0, 3, 0], pos: 8 }, but the read_f64 line panics with:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: UnexpectedEof, error: StringError
("failed to fill whole buffer") }', libcore/result.rs:945:5

Am I doing something wrong? 8 x 8 = 64, so I believe the cursor has the correct number of bytes in it. I see no reason why this would be erroring in this way.

rivertam avatar Jul 31 '18 18:07 rivertam

I needed to say cursor.set_position(0). I wish this were documented in the byteorder docs somewhere, but it is reasonable to expect that and potentially to expect users to know that about Cursors as well.

rivertam avatar Jul 31 '18 19:07 rivertam

@rivertam I think it's a little weird to expect this in the byteorder docs. This is really a property of io::Cursor. The io::Cursor docs could definitely be improved to talk about cursor position and how it works though!

BurntSushi avatar Jul 31 '18 19:07 BurntSushi

Fair enough. The reason I say this is really that I'd never heard of Cursor before using byteorder. I also use byteorder mostly to convert between two numeric types, and there are tons of example of using Cursor but never for this purpose. (is there a better way?)

rivertam avatar Jul 31 '18 20:07 rivertam

You might be able to get away with the Read/Write impls on Vec<u8> and/or &[u8]. It's possible that when byteorder was written (before Rust 1.0) that those impls didn't exist or didn't work for various reasons. That might also suggest that byteorder's examples could be updated if we are indeed unnecessarily using io::Cursor.

BurntSushi avatar Jul 31 '18 21:07 BurntSushi

@BurntSushi when I try with a vec I get:

error[E0599]: no method named `read_f64` found for type `std::vec::Vec<u8>` in
the current scope
  --> src/main.rs:56:11
   |
56 |     bytes.read_f64::<LittleEndian>().unwrap()
   |           ^^^^^^^^
   |
   = note: the method `read_f64` exists but the following trait bounds were not
 satisfied:
           `std::vec::Vec<u8> : byteorder::ReadBytesExt`
           `[u8] : byteorder::ReadBytesExt`

and when I try with a &[u8] or &mut [u8] I get (duplicates culled)

error[E0599]: no method named `write_u16` found for type `&[u8; 8]` in the curr
ent scope
  --> src/main.rs:54:11
   |
54 |     bytes.write_u16::<LittleEndian>(rand::random()).unwrap();
   |           ^^^^^^^^^
   |
   = note: the method `write_u16` exists but the following trait bounds were no
t satisfied:
           `&[u8; 8] : byteorder::WriteBytesExt`
           `[u8; 8] : byteorder::WriteBytesExt`
           `[u8] : byteorder::WriteBytesExt`

error[E0599]: no method named `read_f64` found for type `&[u8; 8]` in the curre
nt scope
  --> src/main.rs:56:11
   |
56 |     bytes.read_f64::<LittleEndian>().unwrap()
   |           ^^^^^^^^
   |
   = note: the method `read_f64` exists but the following trait bounds were not
 satisfied:
           `&[u8; 8] : byteorder::ReadBytesExt`
           `[u8; 8] : byteorder::ReadBytesExt`
           `[u8] : byteorder::ReadBytesExt`

I would definitely rather use a slice or a smallvec because heap allocations just seem very wasteful here.

rivertam avatar Jul 31 '18 22:07 rivertam

@rivertam Basically, all you need is something that impls Write/Read. This can get a little tricky, but it works. The trick here is that:

  • Vec<u8> impls io::Write
  • &mut [u8] impls io::Read
  • &[u8] impls io::Read

So in your compilation errors, you appear to be using an array, which usually coerces to a slice, but won't in the presence of generics. So if x is a [u8; 8], then you can get a &[u8] forcefully with &x[..] or a &mut [u8] with &mut x[..].

Here's an example that uses a plain Vec without a cursor: http://play.rust-lang.org/?gist=d380e6c45d0b64251ca1e1432211c3ed&version=stable&mode=debug&edition=2015 --- The key trick there is using (&*cusor).read_u64() instead of cursor.read_u64().

And here's an example that uses a plain [u8; 8] without a cursor: http://play.rust-lang.org/?gist=ba3eb34e95e3f610e6c6de97321dde31&version=stable&mode=debug&edition=2015 --- The key here is the forceful coercion of arrays into slices to appease generics, and also notice that we need to index the array when writing. Namely, the Write impl for Vec will expand the vec as needed by appending bytes to the end, but the Write impl for &mut [u8] won't, which makes sense if you think about it.

With all that said, if you don't want heap allocation, then it might make sense to not even bother with the ReadBytesExt/WriteBytesExt APIs and just use the ByteOrder trait directly. Here's an example that does just that: http://play.rust-lang.org/?gist=bd91b24a2709ee1b4dcb59524a352d5f&version=stable&mode=debug&edition=2015

The last example is probably how I'd write the code in your original comment myself. I would only use the ReadBytesExt/WriteBytesExt APIs if I specifically need to use a reader or a writer (or write code that is generic over one).

I agree that all of this should probably make it in the byteorder docs somehow, so I'll re-purpose this ticket for just that. :-)

BurntSushi avatar Jul 31 '18 22:07 BurntSushi

@BurntSushi Oh wow thank you so much. You da bomb

rivertam avatar Jul 31 '18 22:07 rivertam