BufMut implementation for `&mut [u8]` slices is a footgun

[phil-opp]

Consider code like this:

use bytes::BufMut;

fn main() {
    let mut buf = Vec::new();
    buf.put_u8(42);
    print_slice(&buf, 1);
}

fn print_slice(slice: &[u8], len: usize) {
    println!("{:?}", &slice[..len]);
}

We create a new Vec based buffer, write an u8 to it, and then print the buffer, which results in the output [42]. (In reality we would for example send it over a network interface instead of printing it.)

Now we want to switch to a stack based buffer (e.g. to avoid the allocation). We see that BufMut is implemented for &mut [u8], so we can just change a single line of code:

-    let mut buf = Vec::new();
+    let mut buf: &mut [u8] = &mut [0; 10];

Instead of using a vector, we now use a stack allocated array with capacity 10 as buffer. Unfortunately, this silently broke our code: The print_slice function now prints [0], even though we wrote the byte 42 to the buffer. In reality, this might mean that we suddenly only send packets containing zeros over the network, which is not good.

(Try it on the playground)

So what's going here? The problem is that put_u8 (and all other BufMut methods) behave differently for Vec and &mut [u8]. For Vec only the len attribute is increased to advance the internal cursor, which doesn't affect the slice that the vector derefs to. For &mut [u8], there is no length attribute, so the BufMut implementation advances the start address of the slice for every cursor movement, which changes the underlying slice.

I'm not sure about the best solution to this problem, but I just spent some considerable time debugging an error caused by this, so I thought that I should at least report it.

[seanmonstar]

I believe the same situation exists if you use std::io::Write.

[phil-opp]

Hmm, you're right: https://doc.rust-lang.org/stable/std/io/trait.Write.html#impl-Write-12. I still think that this implementation isn't a good idea, but I understand if you want to follow the implementation of the standard library. Maybe we could at least document this behavior better?

[carllerche]

I think we should model std here. I will leave the issue open to track improving the docs.

[Ralith]

I find the current behavior useful as well. It's pretty weird to write let mut buf = &mut [0; 10] instead of let mut buf = [0; 10] and then pass a temporary &mut.