Add sized overload for Layer::copyData#2141
Conversation
…ze for bounded copy. Improve documentation of `copyData`.
|
|
||
| size_t Layer::copyData(uint8_t* dest, size_t destSize) const | ||
| { | ||
| size_t bytesToCopy = (std::min)(destSize, m_DataLen); |
There was a problem hiding this comment.
(std::min)(destSize, m_DataLen) this is written as such because Windows.h is fun and has MIN(a, b) macro, that normally conflicts with std::min. 🙂
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #2141 +/- ##
========================================
Coverage 82.72% 82.73%
========================================
Files 332 332
Lines 59807 59835 +28
Branches 12591 12587 -4
========================================
+ Hits 49475 49504 +29
- Misses 8947 9462 +515
+ Partials 1385 869 -516
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| /// @param[out] dest The destination byte array | ||
| /// @param[in] destSize The maximum number of bytes to copy | ||
| /// @return The number of bytes copied to the destination array. | ||
| size_t copyData(uint8_t* dest, size_t destSize) const; |
There was a problem hiding this comment.
Can we add tests for this new method?
There was a problem hiding this comment.
Perhaps we should add it in PacketTests.cpp?
We already have CopyLayerAndPacketTest that kind of fits, or we can add another test in this file
There was a problem hiding this comment.
Moved the test to PacketTests.cpp 9f5e360
Kept the test separate because Ctrl+C, Ctrl+V was easier and CopyLayerAndPacketTest is very large as is.
2 participants
The PR adds a overload of
Layer::copyDatathat offers improved bounding checks against buffer overruns. It also improves the documentation of the methods.