clj-commons / gloss

speaks in bytes, so you don't have to
Eclipse Public License 1.0
483 stars 57 forks source link

Unify Single/MultiBufferSequence behaviour: non-zero position and take-contiguous-bytes #19

Closed geoffsalmon closed 9 years ago

geoffsalmon commented 11 years ago

Because MultiBufferSequence supports buffers with non-zero positions with take and drop bytes, it was surprising that SingleBufferSequence didn't. Another way to implement this would be to slice buffers with non-zero positions in create-buf-seq. The way I've done it here seems more symmetrical with MultiBufferSequence's implementation though.

Also take-contiguous-bytes was behaving differently for Single versus Multi BufferSequence when n was <= 0 or > byte-count. Not sure what behaviour you want. As implemented here, n <= 0 returns an empty buffer and n > byte-count returns all the bytes.

I erred on the side of not calling duplicate and assuming the caller won't muck with returned buffers, but maybe that isn't safe enough. There's a single empty-buffer that's allocated and returned from take-contiguous-bytes when appropriate. Should it be duplicated or allocated each time? SingleBufferSequence take-contiguous-bytes- returns buffer in n >= byte-count case. Should that be duplicated?

geoffsalmon commented 11 years ago

Actually, hold off on this for a bit. I think I've found a problem with it.

geoffsalmon commented 11 years ago

Ok, so trusting the calling code not to modify the buffer was a bad idea. Even my own code messed that up! So now SingleBufferSequence take-contiguous-bytes does duplicate when returning the entire buffer. The empty-buffer is not duplicated though. That still seems unnecessary. Is there anything the caller can do to mess with a capacity zero buffer? It could change the order, but does that matter?