Repository navigation
Should Buffer.concat really return the source buffer when length == 1? #1891
Description
Activity
I do agree that returning the original buffer if length is 1 does seem slightly unintuitive, should mention that the way it currently works is documented:
If the list has exactly one item, then the first item of the list is returned.
Most people end up doing this optimization themselves.
That is, you see code all the time handling:
if (arrayOfBuffers.length === 1) return arrayBuffers[0] // ...
The most annoying part of the current behaviour is that if you really want a
Bufferto be copied every time, you would have to handle it withnew Buffer(Buffer.concat(arrayOfBuffers))unless you directly check for it.@trevnorris is there any specific reason for this behaviour?
@thefourtheye Optimization to avoid creating a new Buffer and doing a copy and making more work for the GC?
@thefourtheye legacy? it's been that way since before I seriously started working on node.
+1 on fixing it, it sounds like a good idea
I think that, if possible, we should fix this, so that people don't have to worry about this special case when using
concat.I agree that this should be fixed. Reminds me of the "Zalgo" problem with callbacks. Either the callback should always be called asynchronously, or always be called synchronously. Likewise, the object returned by
Buffer.concatshould always be safe to mutate, or never safe to mutate. (where safe means "you won't be surprised by aliasing")- added a commit that references this issue
on Jun 10, 2015 Done in f4f16bf.
@trevnorris you're a beast 👍 so many small but useful fixes.
@benjamingr thanks. that patch was all @thefourtheye. I just ported it to the "next" branch. :)
Well @thefourtheye already got credit for being awesome yesterday in front of a thousand people - I guess saying thanks again wouldn't hurt! thanks!
@trevnorris Thanks :-) @benjamingr Oh, I am the one who has to thank you, for being such an inspiration. Thanks a lot man :-)
Thanks guys!
woah, that's how it's done! good work!
- added a commit that references this issue
on Jun 17, 2015 - added 9 commits that reference this issue
on Jul 22, 2015
Buffer.concatreturns a new Buffer instance that contains all given buffers combined. However, if the array of buffers that is passed has length 1, the original buffer from that array is returned as-is. Not a copy. That means that manipulation ofBuffer.concat's output has no effect on the source buffers if they had length > 1, yet alters the source buffer when length == 1.This seems very unpredictable (Array.prototype.concat for example always returns a copy) and is bound to at least confuse people, and at worst cause weird bugs.
Original discussion: #1825 (comment)