From 381f9c196d19c940a9fc41d36004e984b6231f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 16 Jul 2026 21:48:25 +0200 Subject: [PATCH] NodeDB: clear only the slots removeNodeByNum() vacates (#11022) --- src/mesh/NodeDB.cpp | 14 ++++++++--- test/test_nodedb_blocked/test_main.cpp | 35 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 9c736312e..f9f04f487 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -1597,10 +1597,16 @@ void NodeDB::removeNodeByNum(NodeNum nodeNum) removed++; } numMeshNodes -= removed; - std::fill(nodeDatabase.nodes.begin() + numMeshNodes, nodeDatabase.nodes.begin() + numMeshNodes + 1, - meshtastic_NodeInfoLite()); - if (removed) - eraseNodeSatellites(nodeNum); + if (removed) { + // Clear exactly the slots compaction vacated. Sizing this from `removed` (rather than a + // fixed one) keeps it inside the vector when nothing matched and the store is full. + const size_t first = numMeshNodes; + const size_t last = std::min(first + removed, nodeDatabase.nodes.size()); + std::fill(nodeDatabase.nodes.begin() + first, nodeDatabase.nodes.begin() + last, meshtastic_NodeInfoLite()); + } + // Drop the node's satellite stores and warm-tier copy regardless of which tier it lived in, so + // an explicit removal fully forgets it. + eraseNodeSatellites(nodeNum); #if WARM_NODE_COUNT > 0 // Explicit user removal: don't let the warm tier resurrect the node warmStore.remove(nodeNum); diff --git a/test/test_nodedb_blocked/test_main.cpp b/test/test_nodedb_blocked/test_main.cpp index 04c1933de..88d7f0259 100644 --- a/test/test_nodedb_blocked/test_main.cpp +++ b/test/test_nodedb_blocked/test_main.cpp @@ -225,6 +225,39 @@ static void test_protectedCap_refusesBeyondLimit(void) TEST_ASSERT_TRUE(db->setProtectedFlag(already, NODEINFO_BITFIELD_IS_IGNORED_MASK, true)); } +// removeNodeByNum() compacts survivors down and clears the slots that leaves free. A full +// store with no matching node frees none, so there is nothing past the last node to clear. +static void test_removeNodeByNum_absentNodeOnFullDb(void) +{ + db->seedSelf(); + for (int i = 1; i < MAX_NUM_NODES; i++) // fill to MAX_NUM_NODES total (incl. self) + db->push(8000 + i, /*last_heard=*/i, false, false, /*withUser=*/true, /*withKey=*/true); + TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes()); + + db->removeNodeByNum(0xDEADBEEF); // absent; ASan flags a write past the last slot + + TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes()); // nothing removed + TEST_ASSERT_NOT_NULL(db->getMeshNode(0x0BADF00D)); // self intact + TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + 1)); + TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + MAX_NUM_NODES - 1)); // last slot intact +} + +// Control for the above: a matching node on a full store is still removed, the survivors +// compact down, and the freed tail slot is cleared. +static void test_removeNodeByNum_presentNodeOnFullDb(void) +{ + db->seedSelf(); + for (int i = 1; i < MAX_NUM_NODES; i++) + db->push(8000 + i, /*last_heard=*/i, false, false, /*withUser=*/true, /*withKey=*/true); + + db->removeNodeByNum(8000 + 5); + + TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES - 1, (int)db->getNumMeshNodes()); + TEST_ASSERT_NULL(db->getMeshNode(8000 + 5)); + TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + 4)); + TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + MAX_NUM_NODES - 1)); // survivors kept +} + NDB_TEST_ENTRY void setup() { initializeTestEnvironment(); @@ -238,6 +271,8 @@ NDB_TEST_ENTRY void setup() RUN_TEST(test_eviction_preservesFavorite); RUN_TEST(test_ignored_survivesEvictionAndCleanup); RUN_TEST(test_protectedCap_refusesBeyondLimit); + RUN_TEST(test_removeNodeByNum_absentNodeOnFullDb); + RUN_TEST(test_removeNodeByNum_presentNodeOnFullDb); exit(UNITY_END()); } NDB_TEST_ENTRY void loop() {}