Skip to content

Commit 765b483

Browse files
author
David Saada
committed
NVStore: fix area calculation function
Don't allocate the sector map array in this function, as it was buggy and redundant. Separate user config vs. automatic allocation cases instead (which was essentially the case anyway). In addition, fix tests to get over failures in low end boards
1 parent 8f48104 commit 765b483

File tree

2 files changed

+26
-41
lines changed

2 files changed

+26
-41
lines changed

features/storage/nvstore/TESTS/nvstore/functionality/main.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,12 @@ static void nvstore_basic_functionality_test()
109109
TEST_SKIP_UNLESS_MESSAGE(max_possible_keys >= max_possible_keys_threshold,
110110
"Max possible keys below threshold. Test skipped.");
111111

112-
nvstore.set_max_keys(max_test_keys);
113-
TEST_ASSERT_EQUAL(max_test_keys, nvstore.get_max_keys());
114-
115112
result = nvstore.reset();
116113
TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);
117114

115+
nvstore.set_max_keys(max_test_keys);
116+
TEST_ASSERT_EQUAL(max_test_keys, nvstore.get_max_keys());
117+
118118
printf("Max keys %d (out of %d possible ones)\n", nvstore.get_max_keys(), max_possible_keys);
119119

120120
result = nvstore.set(5, 18, nvstore_testing_buf_set);
@@ -503,16 +503,12 @@ static void nvstore_multi_thread_test()
503503
TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, ret);
504504
}
505505

506-
dummy = new (std::nothrow) char[thr_test_num_threads * thr_test_stack_size];
507-
delete[] dummy;
508-
if (!dummy) {
509-
goto mem_fail;
510-
}
511-
512506
for (i = 0; i < thr_test_num_threads; i++) {
513507
threads[i] = new (std::nothrow) rtos::Thread((osPriority_t)((int)osPriorityBelowNormal - thr_test_num_threads + i),
514508
thr_test_stack_size);
515-
if (!threads[i]) {
509+
dummy = new (std::nothrow) char[thr_test_stack_size];
510+
delete[] dummy;
511+
if (!threads[i] || !dummy) {
516512
goto mem_fail;
517513
}
518514
threads[i]->start(mbed::callback(thread_test_worker));

features/storage/nvstore/source/nvstore.cpp

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -264,32 +264,29 @@ int NVStore::flash_erase_area(uint8_t area)
264264

265265
void NVStore::calc_validate_area_params()
266266
{
267-
int num_sectors = 0;
268-
269-
size_t flash_addr = _flash->get_flash_start();
267+
size_t flash_start_addr = _flash->get_flash_start();
270268
size_t flash_size = _flash->get_flash_size();
269+
size_t flash_addr;
271270
size_t sector_size;
272-
int max_sectors = flash_size / _flash->get_sector_size(flash_addr) + 1;
273-
size_t *sector_map = new size_t[max_sectors];
274271

275272
int area = 0;
276273
size_t left_size = flash_size;
277274

278275
memcpy(_flash_area_params, initial_area_params, sizeof(_flash_area_params));
279-
int user_config = (_flash_area_params[0].size != 0);
280-
int in_area = 0;
276+
bool user_config = (_flash_area_params[0].size != 0);
277+
bool in_area = false;
281278
size_t area_size = 0;
282279

283-
while (left_size) {
284-
sector_size = _flash->get_sector_size(flash_addr);
285-
sector_map[num_sectors++] = flash_addr;
280+
if (user_config) {
281+
flash_addr = flash_start_addr;
282+
while (left_size) {
283+
sector_size = _flash->get_sector_size(flash_addr);
286284

287-
if (user_config) {
288285
// User configuration - here we validate it
289286
// Check that address is on a sector boundary, that size covers complete sector sizes,
290287
// and that areas don't overlap.
291288
if (_flash_area_params[area].address == flash_addr) {
292-
in_area = 1;
289+
in_area = true;
293290
}
294291
if (in_area) {
295292
area_size += sector_size;
@@ -298,42 +295,34 @@ void NVStore::calc_validate_area_params()
298295
if (area == NVSTORE_NUM_AREAS) {
299296
break;
300297
}
301-
in_area = 0;
298+
in_area = false;
302299
area_size = 0;
303300
}
304301
}
302+
flash_addr += sector_size;
303+
left_size -= sector_size;
305304
}
306-
307-
flash_addr += sector_size;
308-
left_size -= sector_size;
309-
}
310-
sector_map[num_sectors] = flash_addr;
311-
312-
if (user_config) {
313305
// Valid areas were counted. Assert if not the expected number.
314306
MBED_ASSERT(area == NVSTORE_NUM_AREAS);
315307
} else {
316308
// Not user configuration - calculate area parameters.
317309
// Take last two sectors by default. If their sizes aren't big enough, take
318310
// a few consecutive ones.
319311
area = 1;
320-
_flash_area_params[area].size = 0;
321-
int i;
322-
for (i = num_sectors - 1; i >= 0; i--) {
323-
sector_size = sector_map[i + 1] - sector_map[i];
312+
flash_addr = flash_start_addr + flash_size;
313+
_flash_area_params[0].size = 0;
314+
_flash_area_params[1].size = 0;
315+
while (area >= 0) {
316+
MBED_ASSERT(flash_addr > flash_start_addr);
317+
sector_size = _flash->get_sector_size(flash_addr - 1);
318+
flash_addr -= sector_size;
324319
_flash_area_params[area].size += sector_size;
325320
if (_flash_area_params[area].size >= min_area_size) {
326-
_flash_area_params[area].address = sector_map[i];
321+
_flash_area_params[area].address = flash_addr;
327322
area--;
328-
if (area < 0) {
329-
break;
330-
}
331-
_flash_area_params[area].size = 0;
332323
}
333324
}
334325
}
335-
336-
delete[] sector_map;
337326
}
338327

339328

0 commit comments

Comments
 (0)