-
Notifications
You must be signed in to change notification settings - Fork 30.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
url: fix off-by-one error in loop handling dots #8420
Conversation
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: nodejs#8420
68bda10
to
784b367
Compare
/cc @mscdex ? |
This needs a test. |
Hmm any suggestion? How do you test that an internal loop runs the correct amount of times. Right now it works because the first iteration is basically a noop. |
I assumed that with the use of "error" in the PR description/title that you already had some kind of input that triggered an actual incorrect output? Or did you just find this by perusing the source code or ? |
The latter. I wanted to see the algorithm behind |
Interesting, this has actually been this way since 2011 (af15b4e). This change LGTM though. |
LGTM given that CI is green-ish (other than the unfortunately expected arm failures) |
Another CI, just to be safe: https://ci.nodejs.org/job/node-test-pull-request/3987/ |
LGTM |
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: #8420 Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Landed in 63493e1 |
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: #8420 Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: #8420 Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: #8420 Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: #8420 Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Fixes an error where a loop, used to traverse an array of length `n`, ran `n + 1` times instead of `n`. PR-URL: #8420 Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
url
Description of change
Fixes an error where a loop, used to traverse an array of length
n
, rann + 1
times instead ofn
.