Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve byteorder docs for ReadBytesExt/WriteBytesExt #130

Open
rivertam opened this issue Jul 31, 2018 · 7 comments
Open

improve byteorder docs for ReadBytesExt/WriteBytesExt #130

rivertam opened this issue Jul 31, 2018 · 7 comments

Comments

@rivertam
Copy link

rivertam commented Jul 31, 2018

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
Copy link
Author

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.

@BurntSushi
Copy link
Owner

@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!

@rivertam
Copy link
Author

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?)

@BurntSushi
Copy link
Owner

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.

@rivertam
Copy link
Author

rivertam commented Jul 31, 2018

@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.

@BurntSushi
Copy link
Owner

BurntSushi commented Jul 31, 2018

@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 BurntSushi reopened this Jul 31, 2018
@BurntSushi BurntSushi changed the title Failed to fill whole buffer improve byteorder docs for ReadBytesExt/WriteBytesExt Jul 31, 2018
@rivertam
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants