From 64eab21558f56d6a8ee12561748ace3054eba2c0 Mon Sep 17 00:00:00 2001 From: yi chen <87560781+94xhn@users.noreply.github.com> Date: Sat, 18 Jul 2026 01:08:03 +0800 Subject: [PATCH 1/2] Fix binarySearch() missing return statement (UB) binarySearch() in src/embedDB/embedDB.c fell off the end of a non-void function without returning on its normal termination path (first >= last), which is control-reaches-end-of-non-void-function UB per C11 6.9.1p12 and reproduces with -Wreturn-type. The unconditional pre-check "if (first >= last) break;" exited the loop before the buffered page min/max key was ever compared against the target key, so the function returned an indeterminate value on every normal binary-search convergence, not just a rare edge case. embedDBGet() uses this return value directly to decide whether a record exists, so an existing record could be misreported as missing (false negative) or an out-of-range key falsely reported as found, whenever the caller enables EMBEDDB_USE_BINARY_SEARCH. Fix mirrors the sibling linearSearch() function in the same file: move the "no more room to narrow" exit to after the compareKey check on each branch, and make it an explicit return -1 instead of an unconditional break with no trailing return. Verified with a standalone extraction of the exact function: before the fix, -Wall -Wextra -Wreturn-type reports control-reaches-end-of- non-void-function at -O0 and -O2, and the garbage return value is uncorrelated with actual key membership. After the fix, zero warnings and an 8-case test matrix (single-page hit/miss, multi-page middle/ boundary hits, below-all/above-all misses) all pass at -O0 and -O2. --- src/embedDB/embedDB.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/embedDB/embedDB.c b/src/embedDB/embedDB.c index 8a4ffec..559d721 100644 --- a/src/embedDB/embedDB.c +++ b/src/embedDB/embedDB.c @@ -1389,15 +1389,16 @@ int8_t binarySearch(embedDBState *state, void *buffer, void *key) { if (readPage(state, pageId % state->numDataPages) != 0) return -1; - if (first >= last) - break; - if (state->compareKey(key, embedDBGetMinKey(state, buffer)) < 0) { /* Key is less than smallest record in block. */ + if (first >= last) + return -1; last = pageId - 1; pageId = (first + last) / 2; } else if (state->compareKey(key, embedDBGetMaxKey(state, buffer)) > 0) { /* Key is larger than largest record in block. */ + if (first >= last) + return -1; first = pageId + 1; pageId = (first + last) / 2; } else { From b2971cc0a7083559be5f96490be92b3e3a20199b Mon Sep 17 00:00:00 2001 From: yi chen <94xhn1@gmail.com> Date: Sat, 18 Jul 2026 05:33:28 +0800 Subject: [PATCH 2/2] fix(search): guard binary page bounds A converged interval check does not prevent pageId - 1 from underflowing when the midpoint is already the first page. Guard the actual midpoint boundaries and cover the two-page lower-key case with the real file interface. Constraint: preserve the existing binary-search page selection API Confidence: high Scope-risk: narrow Tested: Linux official test 8/8; GCC 13 ASan/UBSan 8/8; MinGW GCC 8 8/8 Not-tested: Arduino targets --- src/embedDB/embedDB.c | 4 ++-- test/test_embedDB/test_embedDB.cpp | 32 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/embedDB/embedDB.c b/src/embedDB/embedDB.c index 559d721..0ae2583 100644 --- a/src/embedDB/embedDB.c +++ b/src/embedDB/embedDB.c @@ -1391,13 +1391,13 @@ int8_t binarySearch(embedDBState *state, void *buffer, void *key) { if (state->compareKey(key, embedDBGetMinKey(state, buffer)) < 0) { /* Key is less than smallest record in block. */ - if (first >= last) + if (pageId == first) return -1; last = pageId - 1; pageId = (first + last) / 2; } else if (state->compareKey(key, embedDBGetMaxKey(state, buffer)) > 0) { /* Key is larger than largest record in block. */ - if (first >= last) + if (pageId == last) return -1; first = pageId + 1; pageId = (first + last) / 2; diff --git a/test/test_embedDB/test_embedDB.cpp b/test/test_embedDB/test_embedDB.cpp index 20fdd80..aa925c2 100644 --- a/test/test_embedDB/test_embedDB.cpp +++ b/test/test_embedDB/test_embedDB.cpp @@ -67,7 +67,16 @@ #include "unity.h" +extern "C" int8_t binarySearch(embedDBState *state, void *buffer, void *key); + embedDBState *state; +static int8_t (*originalFileRead)(void *, uint32_t, uint32_t, void *); +static uint32_t lastReadPage; + +static int8_t trackFileRead(void *buffer, uint32_t pageNum, uint32_t pageSize, void *file) { + lastReadPage = pageNum; + return originalFileRead(buffer, pageNum, pageSize, file); +} void setupEmbedDB() { state = (embedDBState *)malloc(sizeof(embedDBState)); @@ -241,6 +250,28 @@ void embedDBFlush_does_not_write_when_nothing_in_buffer() { TEST_ASSERT_EQUAL_UINT32_MESSAGE(1000, state->numAvailDataPages, "embedDBFlush should not change numAvailDataPages when no records in buffer."); } +void binarySearch_stops_at_first_page_for_lower_key() { + for (uint32_t key = 10; key < 136; key++) { + uint32_t data = key; + TEST_ASSERT_EQUAL_INT8(0, embedDBPut(state, &key, &data)); + } + TEST_ASSERT_EQUAL_INT8(0, embedDBFlush(state)); + TEST_ASSERT_EQUAL_UINT32(2, state->nextDataPageId); + + state->bufferedPageId = UINT32_MAX; + originalFileRead = state->fileInterface->read; + state->fileInterface->read = trackFileRead; + lastReadPage = UINT32_MAX; + + uint32_t key = 9; + void *readBuffer = (int8_t *)state->buffer + state->pageSize; + int8_t result = binarySearch(state, readBuffer, &key); + + state->fileInterface->read = originalFileRead; + TEST_ASSERT_EQUAL_INT8(-1, result); + TEST_ASSERT_EQUAL_UINT32(0, lastReadPage); +} + int runUnityTests(void) { UNITY_BEGIN(); RUN_TEST(embedDB_initial_configuration_is_correct); @@ -250,6 +281,7 @@ int runUnityTests(void) { RUN_TEST(embedDB_put_inserts_one_more_than_one_page_of_records_correctly); RUN_TEST(iteratorReturnsCorrectRecords); RUN_TEST(embedDBFlush_does_not_write_when_nothing_in_buffer); + RUN_TEST(binarySearch_stops_at_first_page_for_lower_key); return UNITY_END(); }