diff --git a/n_cobs.c b/n_cobs.c index e7558e77..adaa9577 100644 --- a/n_cobs.c +++ b/n_cobs.c @@ -244,20 +244,26 @@ uint32_t _cobsEncodedLength(const uint8_t *ptr, uint32_t length) @param length Length of the data to encode - @return The max length required to encode the data + @return The max length required to encode the data (including EOP byte) + + @note Worst case is input with no zero bytes (no EOP markers to replace). + The COBS encoder always emits 1 initial code byte, then 1 additional + code byte for every 254 data bytes processed: + + codeBytes = floor(length / 254) + 1 + + IMPORTANT: This is NOT ceil(length / 254). They differ at exact + multiples of 254. Example: encoding 254 non-zero bytes produces + code byte 0xFF, 254 data bytes, then a final code byte 0x01. + That's 2 code bytes, but ceil(254/254) = 1 (wrong). - @note Since the contents of the buffer are unknown, then we must assume - that the entire buffer has no end-of-packet markers. This would - require the injection of overhead bytes (as opposed to the - replacement of end-of-packet markers with overhead bytes) at - intervals of 255, thus producing the worst case scenario. @note An additional byte is added for the EOP (end-of-packet) marker. */ /**************************************************************************/ uint32_t _cobsEncodedMaxLength(uint32_t length) { - const uint32_t overheadBytes = (length == 0) + ((length != 0) * ((length / COBS_MAX_PACKET_SIZE) + ((length % COBS_MAX_PACKET_SIZE) > 0))); - return (length + overheadBytes + COBS_EOP_OVERHEAD); + const uint32_t codeBytes = (length / COBS_MAX_PACKET_SIZE) + 1; + return (length + codeBytes + COBS_EOP_OVERHEAD); } //**************************************************************************/ @@ -267,36 +273,40 @@ uint32_t _cobsEncodedMaxLength(uint32_t length) @param bufLen Length of the buffer in bytes - @return the length of unencoded data + @return the length of unencoded data that is guaranteed to fit when + COBS-encoded into bufLen bytes (including EOP) + + @note The COBS encoder always emits 1 initial code byte, then 1 additional + code byte for every 254 data bytes. Therefore: + + codeBytes(u) = floor(u / 254) + 1 + + IMPORTANT: This is NOT ceil(u / 254). They differ at exact multiples + of 254. Example: encoding 254 non-zero bytes produces code 0xFF, + 254 data bytes, then final code 0x01 — that's 2 code bytes, but + ceil(254/254) = 1 (wrong). - @note An additional byte for the EOP (end-of-packet) marker is assumed. + Buffer requirement: bufLen >= u + floor(u/254) + 1 + 1(EOP) + = u + floor(u/254) + 2 + + Inversion: find max u where u + floor(u/254) <= t, with t = bufLen-2. + Substituting u = 254q + r (0 <= r <= 253): 255q + r <= t. + + Closed form: u = t - floor((t + 1) / 255) + + Proof: Let t+1 = 255k + j (0 <= j <= 254), so u = 254k + j - 1. + j >= 1: floor(u/254)=k, u+floor(u/254) = 255k+j-1 = t. Exact fit. + j = 0: u=254(k-1)+253, floor(u/254)=k-1, sum = t-1. One byte slack. + In both cases u+1 would exceed t, confirming u is the maximum. + + @see _cobsEncodedMaxLength() */ /**************************************************************************/ uint32_t _cobsGuaranteedFit(uint32_t bufLen) { - // encodedLen = unencodedLen + codeBytesLen - // e = u + c (e = encoded (sorry Euler), and c = code bytes (sorry Einstein)) - // u = e - c - // c = ⌈u / 254⌉ (the ceiling of u divided by 254) - // - // Rearranging the ceiling equation: - // (c - 1) < u / 254 <= c - // 254(c - 1) < u <= 254c - // - // Substitute u from first equation: - // 254(c - 1) < e - c <= 254c - // 254c - 254 < e - c <= 254c - // 255c < e + 254 AND e <= 255c - // - // Thus: - // e <= 255c < e + 254 - // e / 255 <= c < (e + 254) / 255 - // - // Knowing that c is an integer, we can express c as: - // c = ⌊(e + 254) / 255⌋ (the floor of (e + 254) divided by 255) - // - // Substitute c back into the original equation for u: - // u = e - ⌊(e + 254) / 255⌋ - const uint32_t encodedLen = (bufLen == 0) + ((bufLen != 0) * (bufLen - COBS_EOP_OVERHEAD)); - return (encodedLen - ((encodedLen + COBS_MAX_PACKET_SIZE) / 255)); + if (bufLen <= 2) { + return 0; + } + const uint32_t t = bufLen - 2; + return (t - ((t + 1) / 255)); } diff --git a/test/src/_cobsEncodedMaxLength_test.cpp b/test/src/_cobsEncodedMaxLength_test.cpp index 47244c9e..a4c3a66a 100644 --- a/test/src/_cobsEncodedMaxLength_test.cpp +++ b/test/src/_cobsEncodedMaxLength_test.cpp @@ -74,6 +74,83 @@ SCENARIO("_cobsEncodedMaxLength") } } } + + // The following tests verify the fix for the ceil(u/254) vs floor(u/254)+1 + // bug. At exact multiples of 254, the encoder emits an extra code byte + // that ceil() misses. Example: 254 non-zero bytes encode as [0xFF, data + // x254, 0x01] — that's 2 code bytes, not ceil(254/254)=1. + + GIVEN("A buffer length of exactly 253 bytes (just under one full COBS block)") { + const uint32_t bufLen = 253; + + WHEN("The max encoded length is calculated") { + const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen); + + THEN("The encoded length is 253 + 1 code byte + 1 EOP = 255") { + CHECK(encodedLen == 255); + } + } + } + + GIVEN("A buffer length of exactly 254 bytes (one full COBS block)") { + // BUG FIX: Old code used ceil(254/254)=1 code byte, but the encoder + // actually emits 2: one 0xFF at the start and one 0x01 at the end. + const uint32_t bufLen = 254; + + WHEN("The max encoded length is calculated") { + const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen); + + THEN("The encoded length is 254 + 2 code bytes + 1 EOP = 257") { + CHECK(encodedLen == 257); + } + } + } + + GIVEN("A buffer length of exactly 508 bytes (two full COBS blocks)") { + // BUG FIX: Old code used ceil(508/254)=2 code bytes, but the encoder + // emits 3: one per 254-byte block plus one final. + const uint32_t bufLen = 508; + + WHEN("The max encoded length is calculated") { + const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen); + + THEN("The encoded length is 508 + 3 code bytes + 1 EOP = 512") { + CHECK(encodedLen == 512); + } + } + } + + GIVEN("A buffer length of 255 bytes (one byte past a full COBS block)") { + const uint32_t bufLen = 255; + + WHEN("The max encoded length is calculated") { + const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen); + + THEN("The encoded length is 255 + 2 code bytes + 1 EOP = 258") { + CHECK(encodedLen == 258); + } + } + } +} + +SCENARIO("_cobsEncodedMaxLength round-trip with _cobsGuaranteedFit") +{ + GIVEN("Various buffer sizes") { + // For any buffer size B, encoding cobsGuaranteedFit(B) bytes must + // produce at most B bytes (including the EOP byte). + uint32_t testSizes[] = {3, 8, 255, 256, 257, 510, 511, 512, 65536, 65796, 262144}; + + WHEN("_cobsGuaranteedFit is computed and then _cobsEncodedMaxLength is applied") { + THEN("The encoded max length never exceeds the original buffer") { + for (size_t i = 0; i < sizeof(testSizes)/sizeof(testSizes[0]); i++) { + uint32_t bufLen = testSizes[i]; + uint32_t maxUnencoded = _cobsGuaranteedFit(bufLen); + uint32_t encodedLen = _cobsEncodedMaxLength(maxUnencoded); + CHECK(encodedLen <= bufLen); + } + } + } + } } } diff --git a/test/src/_cobsGuaranteedFit_test.cpp b/test/src/_cobsGuaranteedFit_test.cpp index 4e75e4dc..b2a06d2e 100644 --- a/test/src/_cobsGuaranteedFit_test.cpp +++ b/test/src/_cobsGuaranteedFit_test.cpp @@ -100,6 +100,106 @@ SCENARIO("_cobsGuaranteedFit") } } } + + // The following tests verify the fix for the ceil(u/254) vs floor(u/254)+1 + // bug. At bufLen = 255k+1, the old formula overestimated by 1, promising a + // fit that would actually overflow the buffer after COBS encoding. + + GIVEN("A buffer length of 256 bytes (255*1 + 1, boundary case)") { + // BUG FIX: Old code returned 254. But encoding 254 non-zero bytes + // produces 254 + 2 code bytes = 256, which with +1 EOP = 257 > 256. + // Correct answer: 253 (encodes to 253 + 1 code + 1 EOP = 255 ≤ 256). + const uint32_t bufLen = 256; + + WHEN("The max unencoded length is calculated") { + const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen); + + THEN("The unencoded length is 253, not 254") { + CHECK(unencodedLen == 253); + } + } + } + + GIVEN("A buffer length of 511 bytes (255*2 + 1, boundary case)") { + // BUG FIX: Old code returned 508. But encoding 508 non-zero bytes + // produces 508 + 3 code bytes = 511, plus 1 EOP = 512 > 511. + // Correct answer: 507. + const uint32_t bufLen = 511; + + WHEN("The max unencoded length is calculated") { + const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen); + + THEN("The unencoded length is 507, not 508") { + CHECK(unencodedLen == 507); + } + } + } + + GIVEN("A buffer length of 766 bytes (255*3 + 1, boundary case)") { + const uint32_t bufLen = 766; + + WHEN("The max unencoded length is calculated") { + const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen); + + THEN("The unencoded length is 761") { + CHECK(unencodedLen == 761); + } + } + } + + GIVEN("A buffer length of 257 bytes (non-boundary, one past 256)") { + const uint32_t bufLen = 257; + + WHEN("The max unencoded length is calculated") { + const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen); + + THEN("The unencoded length is 254") { + CHECK(unencodedLen == 254); + } + } + } + + GIVEN("A buffer length of 262144 bytes (customer-reported capacity)") { + const uint32_t bufLen = 262144; + + WHEN("The max unencoded length is calculated") { + const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen); + + THEN("The unencoded length is 261114") { + CHECK(unencodedLen == 261114); + } + } + } +} + +SCENARIO("_cobsGuaranteedFit round-trip with _cobsEncodedMaxLength") +{ + GIVEN("Buffer sizes at every 255k+1 boundary from 1 to 2551") { + WHEN("_cobsGuaranteedFit is computed and then _cobsEncodedMaxLength is applied") { + THEN("The encoded max length never exceeds the original buffer") { + for (uint32_t k = 0; k <= 10; k++) { + uint32_t bufLen = 255 * k + 1; + if (bufLen <= 2) continue; + uint32_t maxUnencoded = _cobsGuaranteedFit(bufLen); + uint32_t encodedLen = _cobsEncodedMaxLength(maxUnencoded); + CHECK(encodedLen <= bufLen); + } + } + } + } + + GIVEN("Buffer sizes at every 255k boundary from 255 to 2550") { + WHEN("The result is checked against _cobsEncodedMaxLength") { + THEN("The encoded max length never exceeds the original buffer") { + for (uint32_t k = 1; k <= 10; k++) { + uint32_t bufLen = 255 * k; + uint32_t maxUnencoded = _cobsGuaranteedFit(bufLen); + uint32_t encodedLen = _cobsEncodedMaxLength(maxUnencoded); + CHECK(encodedLen <= bufLen); + } + } + } + } } }