Skip to content
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

buffer: fix writeInt{B,L}E for some negative values #3994

Closed
wants to merge 1 commit into from

Conversation

pabigot
Copy link
Contributor

@pabigot pabigot commented Nov 23, 2015

The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes #3992

Signed-off-by: Peter A. Bigot pab@pabigot.com

The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes nodejs#3992

Signed-off-by: Peter A. Bigot <pab@pabigot.com>
@evanlucas
Copy link
Contributor

/cc @trevnorris

@mscdex mscdex added the buffer Issues and PRs related to the buffer subsystem. label Nov 23, 2015
@trevnorris
Copy link
Contributor

CI https://ci.nodejs.org/job/node-test-pull-request/817/

If all's good then LGTM

@pabigot
Copy link
Contributor Author

pabigot commented Nov 23, 2015

@trevnorris From what I can see the CI build failed, but I'm at a loss as to why. At least part of it seems to be due to git failures. If you can see something that's due to my change please point it out and I'll work to fix it.

@Fishrock123
Copy link
Contributor

Hmm, cc @nodejs/build that doesn't seem to have run all of the test jobs?

@trevnorris
Copy link
Contributor

@orangemocha
Copy link
Contributor

Jenkins is currently messed up again. See nodejs/build#263

@orangemocha
Copy link
Contributor

Jenkins is working again. New CI run: https://ci.nodejs.org/job/node-test-pull-request/842/

@pabigot
Copy link
Contributor Author

pabigot commented Nov 25, 2015

@orangemocha thanks for resubmitting that. AFAICT the two new failures are due to networking errors. If these aren't expected intermittent problems with the CI infrastructure and you think they could be due to my patch let me know; otherwise I'm going to claim the results are "pass" and move on.

@orangemocha
Copy link
Contributor

I can confirm that those failures are unrelated to this PR.

trevnorris pushed a commit that referenced this pull request Dec 1, 2015
The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes: #3992
PR-URL: #3994
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
@trevnorris
Copy link
Contributor

Thanks much! Landed in bea6742.

@trevnorris trevnorris closed this Dec 1, 2015
rvagg pushed a commit that referenced this pull request Dec 5, 2015
The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes: #3992
PR-URL: #3994
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
MylesBorins pushed a commit that referenced this pull request Dec 15, 2015
The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes: #3992
PR-URL: #3994
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes: #3992
PR-URL: #3994
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes: #3992
PR-URL: #3994
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
The algorithm used to convert negative values to hex generates incorrect
values when the low byte(s) of the value are zero because a carried
subtraction is applied prematurely.

Fixes: nodejs#3992
PR-URL: nodejs#3994
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants