Mavgen C: Use strncpy instead of memcpy to pack char[n] fields #922
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The aim of this PR is to fix #725 and fix mavlink/mavlink#1946
Current situation:
At present when a user calls the
mavlink_msg_<message>_pack
function on a message containing char[n] fields, these fields will be copied into the destination buffer using memcpy. This can go wrong in a couple of ways, if end users are not careful, for example....These extra bytes will stop the Mavlink 2 message-truncation from being as efficient as it can be, and may confuse some receivers.
Proposal:
This PR proposes to use the strncpy function when packing char arrays, as I believe that this aligns with the way in which Mavlink char array fields are specified and used:
This should both protect against reading bad memory, and also ensure bytes beyond the length of short strings are zero, allowing the Mavlink 2 message truncation to work properly.
For little-endian and struct-aligned cases:
This is done by replacing the calls to
mav_array_memcpy
in mavgen_c.py with type-specific array assignment macrosmavlink_array_assign_[type]
.For char arrays, this uses
strncpy
(after check that input is not null pointer).For other arrays, this is a thin-macro which calls
mav_array_memcpy
, as per current behaviour.Big big-endian or non-struct-aligned cases:
This is done by an update to the existing
_mav_put_char_array
function to usestrncpy
(after check that input is not null pointer).Testing
I've tested this update (in particular using the PARAM_REQUEST_READ and PLAY_TUNE messages) on my system.
I also checked compilation with MAVLINK_ALIGNED_FIELDS=0.