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
Fix for incorrect integer value conversion on Windows #1126
Fix for incorrect integer value conversion on Windows #1126
Conversation
- Replace srtol(..) to the strtoll(..) Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change, this otherwise looks good to me.
Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me with green CI.
Gist: https://gist.githubusercontent.com/MichaelOrlov/2b6b58cb449fe2da0a220b18bca964cd/raw/df8f73acff15d43f9ecad2ac4c5d1920b013a47a/ros2.repos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, @MichaelOrlov thanks!
Note we would want to backport this to iron and humble. |
@Mergifyio backport humble iron |
✅ Backports have been created
|
* Fix for incorrect integer value conversion on Windows - Replace srtol(..) to the strtoll(..) Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Chris Lalancette <clalancette@gmail.com> (cherry picked from commit 7bcc293)
* Fix for incorrect integer value conversion on Windows - Replace srtol(..) to the strtoll(..) Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Chris Lalancette <clalancette@gmail.com> (cherry picked from commit 7bcc293)
* Fix for incorrect integer value conversion on Windows - Replace srtol(..) to the strtoll(..) Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Chris Lalancette <clalancette@gmail.com> (cherry picked from commit 7bcc293) Co-authored-by: Michael Orlov <morlovmr@gmail.com>
* Fix for incorrect integer value conversion on Windows - Replace srtol(..) to the strtoll(..) Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Chris Lalancette <clalancette@gmail.com> (cherry picked from commit 7bcc293) Co-authored-by: Michael Orlov <morlovmr@gmail.com>
RCA
It turns out that we are using
ival = strtol(value, &endptr, 0);
function when trying to parse integer value.On Linux and 64-bit, it works correctly for values bigger than
LONG_MAX = 2147483647
becausesizeof(long)
returns8 bytes. However on Windows, it is not true and
sizeof(long)
is 4 bytes andstrtol(value, &endptr, 0)
can't parse values bigger than 2147483647 and we fallback to the parsing value as a floating point value withstrtod(..)
and it succeeds.FIX
strtol(..)
withstrtoll(..)