-
Notifications
You must be signed in to change notification settings - Fork 103
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
Remove dst_size from strnlen usage #353
Conversation
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
af58772
to
99a038e
Compare
Let me know if you think this fix is OK before running full CI @clalancette |
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.
This seems reasonable to me; there is really no reason that we should use strnlen(src, dst_size)
, since src
and dst_size
are not correlated at all.
That said, I'll be interested to see what happens when we run CI on this on all platforms, because some of them may complain about using a "bare" strlen. Besides the normal CI (Linux, Linux aarch64, Windows), I'll suggest also running CI with Windows Debug, the clang job, and RHEL to see if anything complains.
All right, running through CI results here:
So with all of that, the only "new" thing here is the one failure on Windows Debug. I'm going to rerun that one to see if it is a flake: |
All right, Windows Debug is much happier on the latest round. I'm going to go ahead and merge this one in, thanks @Blast545 ! |
Attempt to address #352
There's already a comparison to get the min value from
dest_size
andsrc_length
here:rcutils/src/error_handling_helpers.h
Line 60 in befc608
So I just removed the dst_size from the calculation of the source size.
Signed-off-by: Jorge Perez jjperez@ekumenlabs.com