book icon indicating copy to clipboard operation
book copied to clipboard

In Ch20, `BufReader` is initialized using mutable borrow of `TcpStream` unnecessarily

Open bin-wang opened this issue 1 year ago • 1 comments

  • I have searched open and closed issues and pull requests for duplicates, using these search terms:

    • BufReader
  • I have checked the latest main branch to see if this has already been fixed, in this file:

    • https://github.com/rust-lang/book/blob/main/listings/ch20-web-server/listing-20-02/src/main.rs

URL to the section(s) of the book with this problem: https://doc.rust-lang.org/book/ch20-01-single-threaded.html https://doc.rust-lang.org/book/ch20-02-multithreaded.html

Description of the problem: In all the code listings in Chapter 20, a BufReader is used to read the HTTP request. The BufReader is initialized using a mutable borrow of TcpStream:

let buf_reader = BufReader::new(&mut stream);

However, it seems the mutable borrow isn't necessary here. The code works just fine when mut is removed. What's the consideration here?

Suggested fix:

Change the initialization to use immutable borrow.

let buf_reader = BufReader::new(&stream);

bin-wang avatar Jun 13 '24 13:06 bin-wang

Ah, good catch. This one is a case where the way this code was written originally did require a mutable reference, because it was originally written like this, using std::io::Read::read, which takes a buffer by mutable reference:

    let mut buffer = [0; 1024];
    stream.read(&mut buffer).unwrap();

When it switched over to using BufReader, the &mut stuck around, even though it is not necessary for BufReader::new. I’ll happily merge a PR which drops it (or try to hit it myself at some point).

chriskrycho avatar Jul 17 '24 13:07 chriskrycho