-
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
src: audit and fix shift-into-sign-bit bugs #34827
Labels
c++
Issues and PRs that require attention from people who are familiar with C++.
good first issue
Issues that are suitable for first-time contributors.
Comments
4 tasks
shrish-pathak
pushed a commit
to shrish-pathak/node
that referenced
this issue
Aug 18, 2020
Quick question, why can't
|
@juanarbol It's okay to rename and move that function to e.g. |
Ok, I'll be working on this. |
I don't find a way to inline static int8_t unbase64(uint8_t x);
// Send all this as a const unsigned char pointer
// Or convert all this elements into a const unsigned char pointer
unbase64(src[i + 0]);
unbase64(src[i + 1]);
unbase64(src[i + 2]);
unbase64(src[i + 3]);
// and send it to our beautiful method
cares_get_32bit(&mentioned_above_const_char); |
2 tasks
juanarbol
added a commit
to juanarbol/node
that referenced
this issue
Sep 9, 2020
juanarbol
added a commit
to juanarbol/node
that referenced
this issue
Oct 18, 2020
Fixes: nodejs#34827 Backport-PR-URL: nodejs#35701
joesepi
pushed a commit
to joesepi/node
that referenced
this issue
Jan 8, 2021
Fixes: nodejs#34827 PR-URL: nodejs#34944 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
c++
Issues and PRs that require attention from people who are familiar with C++.
good first issue
Issues that are suitable for first-time contributors.
The LHS here is usually of type
unsigned char
but gets promoted to int (because 24 is an int) and is then left-shifted.If the LHS >= 128, that ends up shifting into the sign bit and that's implementation-defined behavior (i.e., bad - although it probably works okay with most compilers.)
Replace
24
with24u
and all is good. But! It's probably best to abstract it away into a helper function.cares_wrap.cc already has one -
cares_get_32bit()
- that handles it correctly.The text was updated successfully, but these errors were encountered: