Conversation
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
d1a0f65 to
04a1fd7
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #66325 +/- ##
==========================================
- Coverage 90.37% 90.36% -0.02%
==========================================
Files 790 792 +2
Lines 274514 275324 +810
Branches 52572 52768 +196
==========================================
+ Hits 248102 248785 +683
- Misses 16889 16964 +75
- Partials 9523 9575 +52
🚀 New features to boost your workflow:
|
panva
left a comment
There was a problem hiding this comment.
Use int64_t instead of double for the casts and return types. The result is an integer offset or -1, and int64_t fixes the overflow while still exposing a JavaScript Number.
double also disables this Fast API signature on PPC64 and s390x. int64_t keeps it eligible.
| assert.strictEqual(buffer.indexOf(0x0a, match), match); | ||
| assert.strictEqual(buffer.indexOf(Buffer.from([0x0a]), match), match); | ||
| assert.strictEqual(buffer.indexOf('\n', match), match); |
There was a problem hiding this comment.
| assert.strictEqual(buffer.indexOf(0x0a, match), match); | |
| assert.strictEqual(buffer.indexOf(Buffer.from([0x0a]), match), match); | |
| assert.strictEqual(buffer.indexOf('\n', match), match); | |
| assert.strictEqual(buffer.indexOf(0x0a, -1), match); | |
| assert.strictEqual(buffer.indexOf(Buffer.from([0x0a]), -1), match); | |
| assert.strictEqual(buffer.indexOf('\n', -1), match); | |
| assert.strictEqual(buffer.lastIndexOf(0x0a, -1), match); | |
| assert.strictEqual(buffer.lastIndexOf(Buffer.from([0x0a]), -1), match); | |
| assert.strictEqual(buffer.lastIndexOf('\n', -1), match); |
There was a problem hiding this comment.
I appreciate your review.
I’ve switched the return types and casts to int64_t, moved the test to test/pummel and added the suggested indexOf() and lastIndexOf() assertions.
Fix incorrect negative index results for large buffers. Add tests for number, Buffer and string searches beyond 2 GiB. Fixes: nodejs#66294 Signed-off-by: dansatch <dansatch98@gmail.com>
04a1fd7 to
7580065
Compare
Signed-off-by: dansatch <dansatch98@gmail.com>
Fixes #66294.
Buffer.prototype.indexOf()returned a negative position when a match occurred beyond 2 GiB because the result was converted to a signed 32-bit integer.This change returns the position as a JavaScript number and adds regression tests for number, Buffer, and string searches.