Inconsistency between BytesMut and Vec impl of BufMut about bytes HOT 13 CLOSED

I agree these should be consistent. If we change the behavior of Vec I'd consider it a breaking change, and if we changed the behavior of BytesMut I would not personally at least consider it a breaking change.

I would personally advocate for Vec's strategy.

from bytes.

My original thought was that I didn't want buffers to implicitly grow. I think this is a common case in networking scenarios. I often have code that fill as much of the buffer as possible then write flush, then fill more. This specifically shows up in how BufMut is implemented. Code that is generic over BufMut look at the remaining argument and if the buffer implicitly grows then the return value of remaining is usize::MAX.

That said, I understand that the inconsistency is not great and could be convinced to switch.

This change would be a breaking change I believe. I know that I would have a bunch of code that semantically breaks.

Cc @alexcrichton

from bytes.

@alexcrichton implementing this change in BytesMut would expose any existing code using BytesMut to DOS attacks... sounds like a breaking change to me :)

from bytes.

I'm confused-- if your usages always reserve the necessary space and then fill the buffer, why would it be a breaking change to changing the buffer to autofill if you didn't reserve the necessary space? It shouldn't affect this use case at all. Do you have code that relies on BytesMut panicing in order to ensure correctness?

from bytes.

@cramertj put_u8 is a helper fn that is provided by BufMut. The breaking change is BufMut::remaining_mut(). To support this change, BufMut::remaining_mut() would have to return usize::MAX instead of the current remaining capacity.

Any code that relies on remaining_mut to return the remaining capacity would break.

from bytes.

Why would you change the behavior of remaining_mut? It seems reasonable to leave it as a check on the remaining capacity, not the remaining possible size.

from bytes.

put_u8 depends on remaining_mut (it is a trait fn).

from bytes.

Basically, this fn: https://github.com/carllerche/bytes/blob/master/src/buf/buf_mut.rs#L270-L293

from bytes.

@carllerche Couldn't you change put_slice to expand the buffer instead of panicing? I don't see why that would be a breaking change unless someone was relying on the panic occurring.

from bytes.

@cramertj the remaining_mut function behavior needs to change https://github.com/carllerche/bytes/blob/master/src/buf/buf_mut.rs#L273.

from bytes.

Why does it need to change? remaining_mut could just refer to the remaining capacity without allocating rather than the remaining possible capacity. Then you can remove that assertion entirely, and replace it with an ensure_capacity call.

from bytes.

Ah, I see, have BytesMut re-implement all of the fns w/o relying on remaining_mut.

I guess that could work... I'd have to consider it more.

from bytes.

I'm closing this in favor of #170.

from bytes.