Skip to content
This repository

Add an optional maximum length argument to Buffer#write #243

Closed
bzmeteorite opened this Issue · 13 comments

4 participants

bzmeteorite Ben Noordhuis Koichi Kobayashi Hagen Rother
bzmeteorite

So here's the little reminder for you ryah.

An optional argument to Buffer#write that accepts the maximum length in bytes to write from the string. Especially useful for when you're writing UTF-8 and you want to cut the string off prematurely, but don't want to break any multibyte characters in half, but should be implemented no matter the encoding (Buffer#write has this functionality if the buffer isn't large enough).

Ben Noordhuis

I think this works as expected now?

> b = Buffer(3); b.fill(0); b.write('ëë', 0, 'utf-8'); b;
<Buffer c3 ab 00>

If that isn't what you mean, can you post a test case and the expected output?

Koichi Kobayashi
Collaborator

Probably, @bzmeteorite's request is to add length argument to Buffer.write().
Like this:

> b = Buffer(4); b.fill(0); b.write('ëë', 0, 3, 'utf-8'); b;
<Buffer c3 ab 00 00>
bzmeteorite

It's been awhile and I forget the exact details (I wish I had written a test case describing expected functionality, but this issue was mostly a reminder for ryah who said to post an issue, but he never got around to). But I believe what @koichik describes is most likely what I had meant. I believe I found a workaround that implemented this same functionality as well.

I can likely find more info later this week and post it here if anyone is still interested.

Hagen Rother
sixtus commented

See #1361 for a use case: Writing strings that do not use the 0x0 termination pattern.

Koichi Kobayashi koichik referenced this issue from a commit in koichik/node
Koichi Kobayashi koichik Add an optional maxLength argument to Buffer.write
Fixes #243.
Fixes #1361.
f2ee5aa
Koichi Kobayashi
Collaborator

Please review.

Ben Noordhuis

@koichik - I hacked test-buffer.js into an ersatz benchmark (diff) and there's a huge drop in performance.

Before:

real    0m15.674s
user    0m15.610s
sys     0m0.120s

After:

real    0m24.308s
user    0m24.010s
sys     0m0.190s
Hagen Rother
sixtus commented

Ouch, that's a lot. I guess I can work around it, but please do add a big fat warning to the API doc.

Koichi Kobayashi
Collaborator

Oops! This is slower than Utf8Length()!! orz

master:

real    0m9.768s
user    0m10.440s
sys     0m0.420s

f2ee5aa:

real    0m13.870s
user    0m14.680s
sys     0m0.660s

tuned:

real    0m9.987s
user    0m10.630s
sys     0m0.460s
Koichi Kobayashi koichik referenced this issue from a commit in koichik/node
koichik Add an optional length argument to Buffer.write()
Fixes #243.
Fixes #1361.
bc42d04
Koichi Kobayashi
Collaborator

Please review (2).

Ben Noordhuis

@koichik bc42d04 fails to apply on master.

$ curl -s https://github.com/koichik/node/commit/bc42d04.patch | git am --whitespace=fix
Applying: Add an optional length argument to Buffer.write()
error: patch failed: lib/buffer.js:92
error: lib/buffer.js: patch does not apply
Patch failed at 0001 Add an optional length argument to Buffer.write()
Koichi Kobayashi
Collaborator

I hadn't squashed commits, please apply f2ee5aa first.

Ben Noordhuis

Thanks, much better. Still 2% slower but I suppose that's acceptable if it solves a real problem.

Koichi Kobayashi
Collaborator

Thanks!

Koichi Kobayashi koichik closed this in 50e147b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.