From 40e43004eb2acef5e25fe0d0a02a2bf339eeb108 Mon Sep 17 00:00:00 2001 From: Olivier Martin Date: Wed, 21 Nov 2018 13:37:45 +0100 Subject: [PATCH 1/3] tests: settings: Fix typos Fix typos in `settings_test_fcb_init.c`. Signed-off-by: Olivier Martin --- tests/subsys/settings/fcb_init/src/settings_test_fcb_init.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/subsys/settings/fcb_init/src/settings_test_fcb_init.c b/tests/subsys/settings/fcb_init/src/settings_test_fcb_init.c index 20eefd79e0ff9..963a88d966bf0 100644 --- a/tests/subsys/settings/fcb_init/src/settings_test_fcb_init.c +++ b/tests/subsys/settings/fcb_init/src/settings_test_fcb_init.c @@ -83,7 +83,7 @@ void test_init_setup(void) settings_subsys_init(); err = settings_register(&c1_settings); - zassert_true(err == 0, "can't regsister the settings handler"); + zassert_true(err == 0, "can't register the settings handler"); err = settings_load(); zassert_true(err == 0, "can't load settings"); @@ -100,8 +100,7 @@ void test_init_setup(void) void test_main(void) { /* Bellow call is not used as a test setup intentionally. */ - /* It causes device reboota at the first device run after it */ - /* was flashed. */ + /* It causes device reboot a first time after it was flashed. */ test_init_setup(); ztest_test_suite(test_initialization, From 0c1066cdf9b61a26990771fa7792b57bfc92cf30 Mon Sep 17 00:00:00 2001 From: Olivier Martin Date: Wed, 21 Nov 2018 13:49:43 +0100 Subject: [PATCH 2/3] include: Fix typo in settings.h Fix a typo in a comment. Signed-off-by: Olivier Martin --- include/settings/settings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/settings/settings.h b/include/settings/settings.h index 9119f9181bd95..4f1da5a4d6c90 100644 --- a/include/settings/settings.h +++ b/include/settings/settings.h @@ -26,7 +26,7 @@ extern "C" { #define SETTINGS_MAX_VAL_LEN 256 #define SETTINGS_NAME_SEPARATOR "/" -/* pleace for settings additions: +/* place for settings additions: * up to 7 separators, '=', '\0' */ #define SETTINGS_EXTRA_LEN ((SETTINGS_MAX_DIR_DEPTH - 1) + 2) From a56b6f58f3c35eb476112e6e22324e851b43cffc Mon Sep 17 00:00:00 2001 From: Olivier Martin Date: Wed, 21 Nov 2018 13:51:58 +0100 Subject: [PATCH 3/3] subsys: settings: Invalidate entry with the same key in FCB Without this commit, entries with the same key name are present in FCB setting partition as many time as they have been written. All these duplicated keys are loaded at the initialisation of the settings susbystem. The consumer of these settings will be notified for each of these key. It could lead to issues in the caller logic. Signed-off-by: Olivier Martin --- subsys/settings/src/settings_fcb.c | 108 +++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 13 deletions(-) diff --git a/subsys/settings/src/settings_fcb.c b/subsys/settings/src/settings_fcb.c index 890d58716597b..2c92505cfe900 100644 --- a/subsys/settings/src/settings_fcb.c +++ b/subsys/settings/src/settings_fcb.c @@ -13,7 +13,17 @@ #include "settings/settings_fcb.h" #include "settings_priv.h" -#define SETTINGS_FCB_VERS 1 +/* Version 2: Add a status byte to tell if the settings is valid */ +#define SETTINGS_FCB_VERS 2 + +/* Add an extra byte for the status of the settings entry */ +#define FCB_SETTINGS_EXTRA_LEN 1 + +#define FCB_SETTINGS_BUFFER_LEN (FCB_SETTINGS_EXTRA_LEN + \ + SETTINGS_MAX_NAME_LEN + SETTINGS_MAX_VAL_LEN + SETTINGS_EXTRA_LEN) + +#define SETTINGS_ENTRY_VALID 0xFF +#define SETTINGS_ENTRY_INVALID 0x00 struct settings_fcb_load_cb_arg { load_cb cb; @@ -79,8 +89,7 @@ int settings_fcb_dst(struct settings_fcb *cf) static int settings_fcb_load_cb(struct fcb_entry_ctx *entry_ctx, void *arg) { struct settings_fcb_load_cb_arg *argp; - char buf[SETTINGS_MAX_NAME_LEN + SETTINGS_MAX_VAL_LEN + - SETTINGS_EXTRA_LEN]; + char buf[FCB_SETTINGS_BUFFER_LEN]; char *name_str; char *val_str; int rc; @@ -99,9 +108,16 @@ static int settings_fcb_load_cb(struct fcb_entry_ctx *entry_ctx, void *arg) if (rc) { return 0; } + + /* Check if the setting entry is valid */ + if (buf[0] == SETTINGS_ENTRY_INVALID) { + return 0; + } + buf[len] = '\0'; - rc = settings_line_parse(buf, &name_str, &val_str); + rc = settings_line_parse(buf + FCB_SETTINGS_EXTRA_LEN, &name_str, + &val_str); if (rc) { return 0; } @@ -136,18 +152,23 @@ static int settings_fcb_var_read(struct fcb_entry_ctx *entry_ctx, char *buf, if (rc) { return rc; } + + /* Check if the setting entry is valid */ + if (buf[0] == SETTINGS_ENTRY_INVALID) { + return 0; + } + buf[entry_ctx->loc.fe_data_len] = '\0'; - rc = settings_line_parse(buf, name, val); + + rc = settings_line_parse(buf + FCB_SETTINGS_EXTRA_LEN, name, val); return rc; } static void settings_fcb_compress(struct settings_fcb *cf) { int rc; - char buf1[SETTINGS_MAX_NAME_LEN + SETTINGS_MAX_VAL_LEN + - SETTINGS_EXTRA_LEN]; - char buf2[SETTINGS_MAX_NAME_LEN + SETTINGS_MAX_VAL_LEN + - SETTINGS_EXTRA_LEN]; + char buf1[FCB_SETTINGS_BUFFER_LEN]; + char buf2[FCB_SETTINGS_BUFFER_LEN]; struct fcb_entry_ctx loc1; struct fcb_entry_ctx loc2; char *name1, *val1; @@ -216,14 +237,61 @@ static void settings_fcb_compress(struct settings_fcb *cf) __ASSERT(rc == 0, "Failed to fcb rotate.\n"); } +static int settings_fcb_invalid_entry(struct fcb_entry_ctx *entry_ctx, + void *arg) +{ + const char *entry_name = arg; + char buf[FCB_SETTINGS_BUFFER_LEN]; + size_t len; + char *name_str; + char *val_str; + int rc; + + len = entry_ctx->loc.fe_data_len; + if (len >= sizeof(buf)) { + len = sizeof(buf) - 1; + } + + rc = flash_area_read(entry_ctx->fap, + FCB_ENTRY_FA_DATA_OFF(entry_ctx->loc), + buf, len); + if (rc) { + return rc; + } + + /* Check if the setting entry is valid */ + if (buf[0] == SETTINGS_ENTRY_INVALID) { + return 0; + } + + buf[len] = '\0'; + + rc = settings_line_parse(buf + FCB_SETTINGS_EXTRA_LEN, &name_str, + &val_str); + if (rc) { + return 0; + } + + if (strcmp(entry_name, name_str)) { + return 0; + } + + buf[0] = SETTINGS_ENTRY_INVALID; + return flash_area_write(entry_ctx->fap, + FCB_ENTRY_FA_DATA_OFF(entry_ctx->loc), + buf, FCB_SETTINGS_EXTRA_LEN); +} + static int settings_fcb_append(struct settings_fcb *cf, char *buf, int len) { int rc; int i; struct fcb_entry loc; + u8_t entry_status = SETTINGS_ENTRY_VALID; for (i = 0; i < 10; i++) { - rc = fcb_append(&cf->cf_fcb, len, &loc); + rc = fcb_append(&cf->cf_fcb, len + FCB_SETTINGS_EXTRA_LEN, + &loc); if (rc != FCB_ERR_NOSPACE) { break; } @@ -234,7 +302,14 @@ static int settings_fcb_append(struct settings_fcb *cf, char *buf, int len) } rc = flash_area_write(cf->cf_fcb.fap, FCB_ENTRY_FA_DATA_OFF(loc), - buf, len); + &entry_status, sizeof(entry_status)); + if (rc) { + return -EINVAL; + } + + rc = flash_area_write(cf->cf_fcb.fap, + FCB_ENTRY_FA_DATA_OFF(loc) + FCB_SETTINGS_EXTRA_LEN, + buf, len); if (rc) { return -EINVAL; } @@ -245,14 +320,21 @@ static int settings_fcb_save(struct settings_store *cs, const char *name, const char *value) { struct settings_fcb *cf = (struct settings_fcb *)cs; - char buf[SETTINGS_MAX_NAME_LEN + SETTINGS_MAX_VAL_LEN + - SETTINGS_EXTRA_LEN]; + char buf[FCB_SETTINGS_BUFFER_LEN]; int len; + int rc; if (!name) { return -EINVAL; } + /* Invalid entry with the same key */ + rc = fcb_walk(&cf->cf_fcb, 0, settings_fcb_invalid_entry, + (char *)name); + if (rc) { + return rc; + } + len = settings_line_make(buf, sizeof(buf), name, value); if (len < 0 || len + 2 > sizeof(buf)) { return -EINVAL;