std::io::Read: Pass by reference or not?

#1

I’m working on a parser for a packet-based file format. The different packets are processed by different functions To get at the actual data, I pass in a std::io:Error. I’ve considered three different approaches, but each has issues.

Currently, I’m passing around a &mut std::io::read, i.e., something like:

fn foo_process(reader: &mut T) -> Result<Foo, std::io::Error>

But, this apprears to be incompatible with flate2’s expectations, which appears to want to own the std::io::Read. (Or, I can’t figure out how to get it to work. Example code here: https://play.rust-lang.org/?gist=217ad42f5aac811a4ea4d4113ff40ee0&version=stable ).

Moving the std::io::Readobect into the function solves that problem, but, then I have to worry about returning it and the deserialized packet, i.e., something like:

fn foo_process(reader: T) -> Result<(Foo, T), std::io::Error>

Which is ugly.

I’ve also played with boxed trait objects:

fn foo_process(reader: Boxstd::io::Read) -> Result<Foo, std::io::Error);

But this approach clutters the code with lots of boxing and unboxing, has a few Sized issues, which require some acrobatics, and has a small performance cost, which I don’t think is significant for this code. Although, I’ve come furthest with this approach, I suspect it is less idomatic.

So, my question is: should I be passing around references, moving objects, or using boxes?

Thanks!

#2

You just need to modify the code to:

fn new (reader: &'a mut R) -> Self {
    Foo { deflate: DeflateDecoder::new(reader) }
}

That is, you need to specify &'a mut R because that’s how long you intend to keep it borrowed.

#3

Always take generic Reads by value. Why? Because &mut R implements Read! (how do you think by_ref works? :stuck_out_tongue:)

The same is true for Write and Iterator.

#4

Probably true although now you’re committing to it being Sized (likely not a practical limitation). But if that’s not an issue, then given this is a file-based reader the whole &mut is irrelevant because &File: Read.

#5

Why does that limitation exist even though the impl is:


impl<'a, R: Read + ?Sized> Read for &'a mut R
#6

I don’t think ?Sized is an issue. If you have a type that is Read + ?Sized (such as the trait object Read), then it’s already behind an &mut or some type from which you can obtain a &mut. That’s what you give to your fn<R: Read>(r: R) function, exactly the same as if you had written it as fn<R: Read>(r: &mut R).

#7

I agree - I can’t think of any non-contrived issues. Certainly if you control all the code (namely, the place where the value originates) then a good arrangement is possible.

#8

That’s an available impl of the trait, but the Foo<R: Read> has its own Sized constraint there (it’s there with the &mut Rversion as well, but that’s easily correctable). That’s the bit I was referring to.

#9

I’m still puzzled. I’d like to see an example of an API generic in some type R where R: Read together with an invalid usage of the API, which can be made valid by changing the API to use &mut R instead of R (and adding R: ?Sized). Even if it is contrived.

#10

The contrived example is if some code you don’t control gives you an R: Read + ?Sized based &mut R - say that’s all you have when they call into your code. Whether they should arrange things differently is a different question. But as I mentioned, that’s pretty contrived. Your assumption (fairly valid but not required) is that you’re starting with a place where you can form a &mut R yourself and pass that to a fn<R: Read>(r: R).

#11

The Rust API Guidelines recommends generic methods that take R: io::Read by value.

#12

Thanks! This fixed my immediate problem.