From 1b0fb2caaa58240c4571fe1f41ce35b3ef0f56cf Mon Sep 17 00:00:00 2001 From: Laczen JMS Date: Sat, 8 Jun 2019 21:24:42 +0200 Subject: [PATCH 1/2] subsys/settings: change processing to const char The settings module processes the variable name by splitting it up in a set of variables. This PR removes the splitting up and keeps the variable name as one string. It is an alternative to #16609 The possibility is introduced to register handler including a separator, or to register a handler for each variable. The ability is introduced to load a subtree from flash or even to load a single item. Two ways to operate on variable and settings_handler names are provided: settings_name_steq(const char *name, const char *key, const char **next) which checks if name starts with key, returns 1/0 if it does/does not the remaining part of name is in next. settings_name_split(const char *name, char *argv, const char **next) which splits up name in a part before "/"" that is found in argv and the remaining part that is in next. A mutex is added to make settings thread-safe The settings_handlers list is stored in reverse alphabetical order, this allows registration of e.g. bt and bt/mesh in separate handlers, the bt handler itself should not contain any handling of bt/mesh. A settings_deregister() method is added. Settings_handlers can now be added/removed when required. This saves RAM when settings_handlers are not needed. Tests have been updated to reflect changes in the settings api. Updates after meeting: 1. Removed settings_deregister 2. Changed settings_name_split() in settings_name_next: int settings_name_next(const char *name, const char **next): returns the number of characters before the first separator. This can then be used to read the correct number of characters from name using strncpy for processing. 3. New functional test added Update in settings.h: settings_name_next() changed position -> index Added some comments in settings.h (settings_name_steq()) Updated tests to reflect change to settings_name_next() and pointer value comparison. The functional test can now also run on qemu_x86. Corrected some documentation in header. Changed registration of handlers to be non ordered. Changed handler lookup to handle non ordered list of handlers, this improves handler matching in case different length names are compared and also makes it easier to add rom based handlers as they will not be ordered. Signed-off-by: Laczen JMS --- include/settings/settings.h | 104 ++++-- subsys/settings/src/settings.c | 158 ++++++--- subsys/settings/src/settings_fcb.c | 9 +- subsys/settings/src/settings_file.c | 9 +- subsys/settings/src/settings_line.c | 15 +- subsys/settings/src/settings_priv.h | 12 +- subsys/settings/src/settings_runtime.c | 33 +- subsys/settings/src/settings_store.c | 24 +- .../settings/fcb/src/settings_test_fcb.c | 65 ++-- .../fcb_init/src/settings_test_fcb_init.c | 5 +- .../subsys/settings/functional/CMakeLists.txt | 18 + .../settings/functional/native_posix.overlay | 25 ++ .../functional/nrf52840_pca10056.overlay | 25 ++ .../functional/nrf52_pca10040.overlay | 25 ++ tests/subsys/settings/functional/prj.conf | 12 + .../settings/functional/prj_native_posix.conf | 11 + .../settings/functional/prj_qemu_x86.conf | 11 + .../functional/src/settings_basic_test.c | 312 ++++++++++++++++++ .../subsys/settings/functional/testcase.yaml | 4 + .../settings/nffs/src/settings_test_nffs.c | 22 +- 20 files changed, 749 insertions(+), 150 deletions(-) create mode 100644 tests/subsys/settings/functional/CMakeLists.txt create mode 100644 tests/subsys/settings/functional/native_posix.overlay create mode 100644 tests/subsys/settings/functional/nrf52840_pca10056.overlay create mode 100644 tests/subsys/settings/functional/nrf52_pca10040.overlay create mode 100644 tests/subsys/settings/functional/prj.conf create mode 100644 tests/subsys/settings/functional/prj_native_posix.conf create mode 100644 tests/subsys/settings/functional/prj_qemu_x86.conf create mode 100644 tests/subsys/settings/functional/src/settings_basic_test.c create mode 100644 tests/subsys/settings/functional/testcase.yaml diff --git a/include/settings/settings.h b/include/settings/settings.h index 6379597b171d0..e64f047b325e0 100644 --- a/include/settings/settings.h +++ b/include/settings/settings.h @@ -26,7 +26,8 @@ extern "C" { #define SETTINGS_MAX_DIR_DEPTH 8 /* max depth of settings tree */ #define SETTINGS_MAX_NAME_LEN (8 * SETTINGS_MAX_DIR_DEPTH) #define SETTINGS_MAX_VAL_LEN 256 -#define SETTINGS_NAME_SEPARATOR "/" +#define SETTINGS_NAME_SEPARATOR '/' +#define SETTINGS_NAME_END '=' /* pleace for settings additions: * up to 7 separators, '=', '\0' @@ -47,26 +48,27 @@ struct settings_handler { char *name; /**< Name of subtree. */ - int (*h_get)(int argc, char **argv, char *val, int val_len_max); + int (*h_get)(const char *key, char *val, int val_len_max); /**< Get values handler of settings items identified by keyword names. * * Parameters: - * - argc - count of item in argv. - * - argv - array of pointers to keyword names. - * - val - buffer for a value. - * - val_len_max - size of that buffer. + * - key[in] the name with skipped part that was used as name in + * handler registration + * - val[out] buffer to receive value. + * - val_len_max[in] size of that buffer. */ - int (*h_set)(int argc, char **argv, size_t len, - settings_read_cb read_cb, void *cb_arg); + int (*h_set)(const char *key, size_t len, settings_read_cb read_cb, + void *cb_arg); /**< Set value handler of settings items identified by keyword names. * * Parameters: - * - argc - count of item in argv. - * - argv - array of pointers to keyword names. - * - len - the size of the data found in the backend. - * - read_cb - function provided to read the data from the backend. - * - cb_arg - arguments for the read function provided by the backend. + * - key[in] the name with skipped part that was used as name in + * handler registration + * - len[in] the size of the data found in the backend. + * - read_cb[in] function provided to read the data from the backend. + * - cb_arg[in] arguments for the read function provided by the + * backend. */ int (*h_commit)(void); @@ -121,8 +123,18 @@ int settings_register(struct settings_handler *cf); int settings_load(void); /** - * Save currently running serialized items. All serialized items which are different - * from currently persisted values will be saved. + * Load limited set of serialized items from registered persistence sources. + * Handlers for serialized item subtrees registered earlier will be called for + * encountered values that belong to the subtree. + * + * @param[in] subtree name of the subtree to be loaded. + * @return 0 on success, non-zero on failure. + */ +int settings_load_subtree(const char *subtree); + +/** + * Save currently running serialized items. All serialized items which are + * different from currently persisted values will be saved. * * @return 0 on success, non-zero on failure. */ @@ -161,6 +173,16 @@ int settings_delete(const char *name); */ int settings_commit(void); +/** + * Call commit for settings handler that belong to subtree. + * This should apply all settings which has been set, but not applied yet. + * + * @param[in] subtree name of the subtree to be committed. + * + * @return 0 on success, non-zero on failure. + */ +int settings_commit_subtree(const char *subtree); + /** * @} settings */ @@ -191,8 +213,9 @@ struct settings_store { * Destinations are registered using a call to @ref settings_dst_register. */ struct settings_store_itf { - int (*csi_load)(struct settings_store *cs); - /**< Loads all values from storage. + int (*csi_load)(struct settings_store *cs, const char *subtree); + /**< Loads values from storage limited to subtree defined by subtree. If + * subtree = NULL loads all values. * * Parameters: * - cs - Corresponding backend handler node @@ -248,16 +271,53 @@ void settings_dst_register(struct settings_store *cs); /** * Parses a key to an array of elements and locate corresponding module handler. * - * @param name Key in string format - * @param name_argc Parsed number of elements. - * @param name_argv Parsed array of elements. + * @param[in] name in string format + * @param[out] next remaining of name after matched handler * * @return settings_handler node on success, NULL on failure. */ -struct settings_handler *settings_parse_and_lookup(char *name, int *name_argc, - char *name_argv[]); +struct settings_handler *settings_parse_and_lookup(const char *name, + const char **next); +/* + * API for const name processing + */ + +/** + * Compares the start of name with a key + * + * @param[in] name in string format + * @param[in] key comparison string + * @param[out] next pointer to remaining of name, when the remaining part + * starts with a separator the separator is removed from next + * + * Some examples: + * settings_name_steq("bt/btmesh/iv", "b", &next) returns 1, next="t/btmesh/iv" + * settings_name_steq("bt/btmesh/iv", "bt", &next) returns 1, next="btmesh/iv" + * settings_name_steq("bt/btmesh/iv", "bt/", &next) returns 0, next=NULL + * settings_name_steq("bt/btmesh/iv", "bta", &next) returns 0, next=NULL + * + * REMARK: This routine could be simplified if the settings_handler names + * would include a separator at the end. + * + * @return 0: no match + * 1: match, next can be used to check if match is full + */ +int settings_name_steq(const char *name, const char *key, const char **next); + +/** + * determine the number of characters before the first separator + * + * @param[in] name in string format + * @param[out] next pointer to remaining of name (excluding separator) + * + * @return index of the first separator, in case no separator was found this + * is the size of name + * + */ +int settings_name_next(const char *name, const char **next); + /* * API for runtime settings */ diff --git a/subsys/settings/src/settings.c b/subsys/settings/src/settings.c index 2c179255b12f8..51b70c3945454 100644 --- a/subsys/settings/src/settings.c +++ b/subsys/settings/src/settings.c @@ -18,10 +18,9 @@ LOG_MODULE_REGISTER(settings, CONFIG_SETTINGS_LOG_LEVEL); sys_slist_t settings_handlers; +struct k_mutex settings_lock; -static struct settings_handler *settings_handler_lookup(char *name); - void settings_store_init(void); void settings_init(void) @@ -32,80 +31,151 @@ void settings_init(void) int settings_register(struct settings_handler *handler) { - if (settings_handler_lookup(handler->name)) { - return -EEXIST; - } - sys_slist_prepend(&settings_handlers, &handler->node); + int rc; + struct settings_handler *ch; - return 0; + k_mutex_lock(&settings_lock, K_FOREVER); + + SYS_SLIST_FOR_EACH_CONTAINER(&settings_handlers, ch, node) { + if (strcmp(handler->name, ch->name) == 0) { + rc = -EEXIST; + goto end; + } + } + sys_slist_append(&settings_handlers, &handler->node); + rc = 0; +end: + k_mutex_unlock(&settings_lock); + return rc; } -/* - * Find settings_handler based on name. - */ -static struct settings_handler *settings_handler_lookup(char *name) +int settings_name_steq(const char *name, const char *key, const char **next) { - struct settings_handler *ch; + if (next) { + *next = NULL; + } - SYS_SLIST_FOR_EACH_CONTAINER(&settings_handlers, ch, node) { - if (!strcmp(name, ch->name)) { - return ch; + if ((!name) || (!key)) { + return 0; + } + + /* name might come from flash directly, in flash the name would end + * with '=' or '\0' depending how storage is done. Flash reading is + * limited to what can be read + */ + + while ((*key != '\0') && (*key == *name) && + (*name != '\0') && (*name != SETTINGS_NAME_END)) { + key++; + name++; + } + + if (*key != '\0') { + return 0; + } + + if (*name == SETTINGS_NAME_SEPARATOR) { + if (next) { + *next = name + 1; } + return 1; } - return NULL; + + if ((*name == SETTINGS_NAME_END) || (*name == '\0')) { + return 1; + } + + return 0; } -/* - * Separate string into argv array. - */ -int settings_parse_name(char *name, int *name_argc, char *name_argv[]) +int settings_name_next(const char *name, const char **next) { - int i = 0; + int rc = 0; - while (name) { - name_argv[i++] = name; + if (next) { + *next = NULL; + } - while (1) { - if (*name == '\0') { - name = NULL; - break; - } + if (!name) { + return 0; + } - if (*name == *SETTINGS_NAME_SEPARATOR) { - *name = '\0'; - name++; - break; - } - name++; + /* name might come from flash directly, in flash the name would end + * with '=' or '\0' depending how storage is done. Flash reading is + * limited to what can be read + */ + while ((*name != '\0') && (*name != SETTINGS_NAME_END) && + (*name != SETTINGS_NAME_SEPARATOR)) { + rc++; + name++; + } + + if (*name == SETTINGS_NAME_SEPARATOR) { + if (next) { + *next = name + 1; } + return rc; } - *name_argc = i; + return rc; +} - return 0; +struct settings_handler *settings_parse_and_lookup(const char *name, + const char **next) +{ + struct settings_handler *ch, *bestmatch; + const char *tmpnext; + + bestmatch = NULL; + if (next) { + *next = NULL; + } + + SYS_SLIST_FOR_EACH_CONTAINER(&settings_handlers, ch, node) { + if (settings_name_steq(name, ch->name, &tmpnext)) { + if ((!bestmatch) || + (settings_name_steq(ch->name, + bestmatch->name, NULL))) { + bestmatch = ch; + if (next) { + *next = tmpnext; + } + } + } + } + return bestmatch; } -struct settings_handler *settings_parse_and_lookup(char *name, - int *name_argc, - char *name_argv[]) +int settings_commit(void) { + struct settings_handler *ch; int rc; + int rc2; - rc = settings_parse_name(name, name_argc, name_argv); - if (rc) { - return NULL; + rc = 0; + SYS_SLIST_FOR_EACH_CONTAINER(&settings_handlers, ch, node) { + if (ch->h_commit) { + rc2 = ch->h_commit(); + if (!rc) { + rc = rc2; + } + } } - return settings_handler_lookup(name_argv[0]); + return rc; } -int settings_commit(void) +int settings_commit_subtree(const char *subtree) { struct settings_handler *ch; + int rc; int rc2; rc = 0; SYS_SLIST_FOR_EACH_CONTAINER(&settings_handlers, ch, node) { + if (subtree && !settings_name_steq(ch->name, subtree, NULL)) { + continue; + } if (ch->h_commit) { rc2 = ch->h_commit(); if (!rc) { diff --git a/subsys/settings/src/settings_fcb.c b/subsys/settings/src/settings_fcb.c index 0622aaf4963be..5824e30d0b78d 100644 --- a/subsys/settings/src/settings_fcb.c +++ b/subsys/settings/src/settings_fcb.c @@ -23,11 +23,11 @@ struct settings_fcb_load_cb_arg { void *cb_arg; }; -static int settings_fcb_load(struct settings_store *cs); +static int settings_fcb_load(struct settings_store *cs, const char *subtree); static int settings_fcb_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); -static struct settings_store_itf settings_fcb_itf = { +static const struct settings_store_itf settings_fcb_itf = { .csi_load = settings_fcb_load, .csi_save = settings_fcb_save, }; @@ -117,9 +117,10 @@ static int settings_fcb_load_priv(struct settings_store *cs, line_load_cb cb, return 0; } -static int settings_fcb_load(struct settings_store *cs) +static int settings_fcb_load(struct settings_store *cs, const char *subtree) { - return settings_fcb_load_priv(cs, settings_line_load_cb, NULL); + return settings_fcb_load_priv(cs, settings_line_load_cb, + (void *)subtree); } diff --git a/subsys/settings/src/settings_file.c b/subsys/settings/src/settings_file.c index 79faf46563111..b93c9298847a3 100644 --- a/subsys/settings/src/settings_file.c +++ b/subsys/settings/src/settings_file.c @@ -14,11 +14,11 @@ #include "settings/settings_file.h" #include "settings_priv.h" -static int settings_file_load(struct settings_store *cs); +static int settings_file_load(struct settings_store *cs, const char *subtree); static int settings_file_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); -static struct settings_store_itf settings_file_itf = { +static const struct settings_store_itf settings_file_itf = { .csi_load = settings_file_load, .csi_save = settings_file_save, }; @@ -109,9 +109,10 @@ static int settings_file_load_priv(struct settings_store *cs, line_load_cb cb, /* * Called to load configuration items. */ -static int settings_file_load(struct settings_store *cs) +static int settings_file_load(struct settings_store *cs, const char *subtree) { - return settings_file_load_priv(cs, settings_line_load_cb, NULL); + return settings_file_load_priv(cs, settings_line_load_cb, + (void *)subtree); } static void settings_tmpfile(char *dst, const char *src, char *pfx) diff --git a/subsys/settings/src/settings_line.c b/subsys/settings/src/settings_line.c index f773fa82a0300..34d560459d9d9 100644 --- a/subsys/settings/src/settings_line.c +++ b/subsys/settings/src/settings_line.c @@ -487,7 +487,7 @@ static int settings_line_cmp(char const *val, size_t val_len, return rc; } -void settings_line_dup_check_cb(char *name, void *val_read_cb_ctx, +void settings_line_dup_check_cb(const char *name, void *val_read_cb_ctx, off_t off, void *cb_arg) { struct settings_line_dup_check_arg *cdca; @@ -530,17 +530,20 @@ static ssize_t settings_line_read_cb(void *cb_arg, void *data, size_t len) return -1; } -void settings_line_load_cb(char *name, void *val_read_cb_ctx, off_t off, +void settings_line_load_cb(const char *name, void *val_read_cb_ctx, off_t off, void *cb_arg) { - int name_argc; - char *name_argv[SETTINGS_MAX_DIR_DEPTH]; + const char *name_key; struct settings_handler *ch; struct settings_line_read_value_cb_ctx value_ctx; int rc; size_t len; - ch = settings_parse_and_lookup(name, &name_argc, name_argv); + if (cb_arg && !settings_name_steq(name, cb_arg, NULL)) { + return; + } + + ch = settings_parse_and_lookup(name, &name_key); if (!ch) { return; } @@ -550,7 +553,7 @@ void settings_line_load_cb(char *name, void *val_read_cb_ctx, off_t off, len = settings_line_val_get_len(off, val_read_cb_ctx); - rc = ch->h_set(name_argc - 1, &name_argv[1], len, settings_line_read_cb, + rc = ch->h_set(name_key, len, settings_line_read_cb, (void *)&value_ctx); if (rc != 0) { diff --git a/subsys/settings/src/settings_priv.h b/subsys/settings/src/settings_priv.h index 8fbe452e649f9..f228e96f3a98f 100644 --- a/subsys/settings/src/settings_priv.h +++ b/subsys/settings/src/settings_priv.h @@ -35,14 +35,14 @@ int settings_line_write(const char *name, const char *value, size_t val_len, /* Get len of record without alignment to write-block-size */ int settings_line_len_calc(const char *name, size_t val_len); -void settings_line_dup_check_cb(char *name, void *val_read_cb_ctx, off_t off, - void *cb_arg); +void settings_line_dup_check_cb(const char *name, void *val_read_cb_ctx, + off_t off, void *cb_arg); -void settings_line_load_cb(char *name, void *val_read_cb_ctx, off_t off, - void *cb_arg); +void settings_line_load_cb(const char *name, void *val_read_cb_ctx, + off_t off, void *cb_arg); -typedef void (*line_load_cb)(char *name, void *val_read_cb_ctx, off_t off, - void *cb_arg); +typedef void (*line_load_cb)(const char *name, void *val_read_cb_ctx, + off_t off, void *cb_arg); struct settings_line_read_value_cb_ctx { void *read_cb_ctx; diff --git a/subsys/settings/src/settings_runtime.c b/subsys/settings/src/settings_runtime.c index 0401f8d5712c6..288f69caa7645 100644 --- a/subsys/settings/src/settings_runtime.c +++ b/subsys/settings/src/settings_runtime.c @@ -18,54 +18,39 @@ static ssize_t settings_runtime_read_cb(void *cb_arg, void *data, size_t len) int settings_runtime_set(const char *name, void *data, size_t len) { struct settings_handler *ch; - char name1[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN]; - char *name_argv[SETTINGS_MAX_DIR_DEPTH]; - int name_argc; + const char *name_key; - strncpy(name1, name, sizeof(name1) - 1); - name1[sizeof(name1) - 1] = '\0'; - - ch = settings_parse_and_lookup(name1, &name_argc, name_argv); + ch = settings_parse_and_lookup(name, &name_key); if (!ch) { return -EINVAL; } - return ch->h_set(name_argc - 1, &name_argv[1], len, - settings_runtime_read_cb, data); + return ch->h_set(name_key, len, settings_runtime_read_cb, data); } int settings_runtime_get(const char *name, void *data, size_t len) { struct settings_handler *ch; - char name1[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN]; - char *name_argv[SETTINGS_MAX_DIR_DEPTH]; - int name_argc; - - strncpy(name1, name, sizeof(name1) - 1); - name1[sizeof(name1) - 1] = '\0'; + const char *name_key; - ch = settings_parse_and_lookup(name1, &name_argc, name_argv); + ch = settings_parse_and_lookup(name, &name_key); if (!ch) { return -EINVAL; } - return ch->h_get(name_argc - 1, &name_argv[1], data, len); + return ch->h_get(name_key, data, len); } int settings_runtime_commit(const char *name) { struct settings_handler *ch; - char name1[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN]; - char *name_argv[SETTINGS_MAX_DIR_DEPTH]; - int name_argc; + const char *name_key; - strncpy(name1, name, sizeof(name1) - 1); - name1[sizeof(name1) - 1] = '\0'; - - ch = settings_parse_and_lookup(name1, &name_argc, name_argv); + ch = settings_parse_and_lookup(name, &name_key); if (!ch) { return -EINVAL; } + if (ch->h_commit) { return ch->h_commit(); } else { diff --git a/subsys/settings/src/settings_store.c b/subsys/settings/src/settings_store.c index 9128ea60e9df1..ef43e50d8d813 100644 --- a/subsys/settings/src/settings_store.c +++ b/subsys/settings/src/settings_store.c @@ -21,6 +21,7 @@ LOG_MODULE_DECLARE(settings, CONFIG_SETTINGS_LOG_LEVEL); sys_slist_t settings_load_srcs; struct settings_store *settings_save_dst; +extern struct k_mutex settings_lock; void settings_src_register(struct settings_store *cs) { @@ -41,8 +42,14 @@ void settings_dst_register(struct settings_store *cs) } int settings_load(void) +{ + return settings_load_subtree(NULL); +} + +int settings_load_subtree(const char *subtree) { struct settings_store *cs; + int rc; /* * for every config store @@ -50,11 +57,13 @@ int settings_load(void) * apply config * commit all */ - + k_mutex_lock(&settings_lock, K_FOREVER); SYS_SLIST_FOR_EACH_CONTAINER(&settings_load_srcs, cs, cs_next) { - cs->cs_itf->csi_load(cs); + cs->cs_itf->csi_load(cs, subtree); } - return settings_commit(); + rc = settings_commit_subtree(subtree); + k_mutex_unlock(&settings_lock); + return rc; } /* @@ -62,6 +71,7 @@ int settings_load(void) */ int settings_save_one(const char *name, const void *value, size_t val_len) { + int rc; struct settings_store *cs; cs = settings_save_dst; @@ -69,7 +79,13 @@ int settings_save_one(const char *name, const void *value, size_t val_len) return -ENOENT; } - return cs->cs_itf->csi_save(cs, name, (char *)value, val_len); + k_mutex_lock(&settings_lock, K_FOREVER); + + rc = cs->cs_itf->csi_save(cs, name, (char *)value, val_len); + + k_mutex_unlock(&settings_lock); + + return rc; } int settings_delete(const char *name) diff --git a/tests/subsys/settings/fcb/src/settings_test_fcb.c b/tests/subsys/settings/fcb/src/settings_test_fcb.c index 33a8d65db89dd..c687cf311f0d0 100644 --- a/tests/subsys/settings/fcb/src/settings_test_fcb.c +++ b/tests/subsys/settings/fcb/src/settings_test_fcb.c @@ -23,21 +23,21 @@ int test_export_block; int c2_var_count = 1; -int c1_handle_get(int argc, char **argv, char *val, int val_len_max); -int c1_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c1_handle_get(const char *name, char *val, int val_len_max); +int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); int c1_handle_commit(void); int c1_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)); -int c2_handle_get(int argc, char **argv, char *val, int val_len_max); -int c2_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c2_handle_get(const char *name, char *val, int val_len_max); +int c2_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); int c2_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)); -int c3_handle_get(int argc, char **argv, char *val, int val_len_max); -int c3_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c3_handle_get(const char *name, char *val, int val_len_max); +int c3_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); int c3_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)); @@ -69,17 +69,19 @@ struct settings_handler c_test_handlers[] = { char val_string[SETTINGS_TEST_FCB_VAL_STR_CNT][SETTINGS_MAX_VAL_LEN]; char test_ref_value[SETTINGS_TEST_FCB_VAL_STR_CNT][SETTINGS_MAX_VAL_LEN]; -int c1_handle_get(int argc, char **argv, char *val, int val_len_max) +int c1_handle_get(const char *name, char *val, int val_len_max) { + const char *next; + test_get_called = 1; - if (argc == 1 && !strcmp(argv[0], "mybar")) { + if (settings_name_steq(name, "mybar", &next) && !next) { val_len_max = MIN(val_len_max, sizeof(val8)); memcpy(val, &val8, MIN(val_len_max, sizeof(val8))); return val_len_max; } - if (argc == 1 && !strcmp(argv[0], "mybar64")) { + if (settings_name_steq(name, "mybar64", &next) && !next) { val_len_max = MIN(val_len_max, sizeof(val64)); memcpy(val, &val64, MIN(val_len_max, sizeof(val64))); return val_len_max; @@ -88,26 +90,27 @@ int c1_handle_get(int argc, char **argv, char *val, int val_len_max) return -ENOENT; } -int c1_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) { size_t val_len; int rc; + const char *next; test_set_called = 1; - if (argc == 1 && !strcmp(argv[0], "mybar")) { + if (settings_name_steq(name, "mybar", &next) && !next) { rc = read_cb(cb_arg, &val8, sizeof(val8)); zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); return 0; } - if (argc == 1 && !strcmp(argv[0], "mybar64")) { + if (settings_name_steq(name, "mybar64", &next) && !next) { rc = read_cb(cb_arg, &val64, sizeof(val64)); zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); return 0; } - if (argc == 1 && !strcmp(argv[0], "unaligned")) { + if (settings_name_steq(name, "unaligned", &next) && !next) { val_len = len; zassert_equal(val_len, sizeof(val8_un), "value length: %d, ought equal 1", val_len); @@ -225,13 +228,18 @@ char *c2_var_find(char *name) return val_string[idx]; } -int c2_handle_get(int argc, char **argv, char *val, int val_len_max) +int c2_handle_get(const char *name, char *val, int val_len_max) { int len; char *valptr; - - if (argc == 1) { - valptr = c2_var_find(argv[0]); + const char *next; + char argv[32]; + + len = settings_name_next(name, &next); + if (len && !next) { + strncpy(argv, name, len); + argv[len] = '\0'; + valptr = c2_var_find(argv); if (!valptr) { return -ENOENT; } @@ -249,14 +257,20 @@ int c2_handle_get(int argc, char **argv, char *val, int val_len_max) return -ENOENT; } -int c2_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c2_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) { char *valptr; + const char *next; + char argv[32]; + int rc; - if (argc == 1) { - valptr = c2_var_find(argv[0]); + len = settings_name_next(name, &next); + if (len && !next) { + strncpy(argv, name, len); + argv[len] = '\0'; + valptr = c2_var_find(argv); if (!valptr) { return -ENOENT; } @@ -287,9 +301,11 @@ int c2_handle_export(int (*cb)(const char *name, return 0; } -int c3_handle_get(int argc, char **argv, char *val, int val_len_max) +int c3_handle_get(const char *name, char *val, int val_len_max) { - if (argc == 1 && !strcmp(argv[0], "v")) { + const char *next; + + if (settings_name_steq(name, "v", &next) && !next) { val_len_max = MIN(val_len_max, sizeof(val32)); memcpy(val, &val32, MIN(val_len_max, sizeof(val32))); return val_len_max; @@ -297,13 +313,14 @@ int c3_handle_get(int argc, char **argv, char *val, int val_len_max) return -EINVAL; } -int c3_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c3_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) { int rc; size_t val_len; + const char *next; - if (argc == 1 && !strcmp(argv[0], "v")) { + if (settings_name_steq(name, "v", &next) && !next) { val_len = len; zassert_true(val_len == 4, "bad set-value size"); 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 055b69e1e9f03..dee6ad2ac6ede 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 @@ -15,12 +15,13 @@ static u32_t val32; -static int c1_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +static int c1_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) { int rc; + const char *next; - if (argc == 1 && !strcmp(argv[0], "val32")) { + if (settings_name_steq(name, "val32", &next) && !next) { rc = read_cb(cb_arg, &val32, sizeof(val32)); zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); return 0; diff --git a/tests/subsys/settings/functional/CMakeLists.txt b/tests/subsys/settings/functional/CMakeLists.txt new file mode 100644 index 0000000000000..fb0087908c89e --- /dev/null +++ b/tests/subsys/settings/functional/CMakeLists.txt @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.13.1) +include($ENV{ZEPHYR_BASE}/cmake/app/boilerplate.cmake NO_POLICY_SCOPE) +project(NONE) + +FILE(GLOB app_sources src/*.c) +target_sources(app PRIVATE ${app_sources}) +zephyr_include_directories( + $ENV{ZEPHYR_BASE}/subsys/settings/include + $ENV{ZEPHYR_BASE}/subsys/settings/src + ) + +if(TEST) + target_compile_definitions(app PRIVATE + -DTEST_${TEST} + ) +endif() diff --git a/tests/subsys/settings/functional/native_posix.overlay b/tests/subsys/settings/functional/native_posix.overlay new file mode 100644 index 0000000000000..d936a341ab233 --- /dev/null +++ b/tests/subsys/settings/functional/native_posix.overlay @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2019 Jan Van Winkel + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/delete-node/ &storage_partition; +/delete-node/ &scratch_partition; + +&flash0 { + /* + * For more information, see: + * http://docs.zephyrproject.org/latest/guides/dts/index.html#flash-partitions + */ + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + storage_partition: partition@70000 { + label = "storage"; + reg = <0x00070000 0x10000>; + }; + }; +}; diff --git a/tests/subsys/settings/functional/nrf52840_pca10056.overlay b/tests/subsys/settings/functional/nrf52840_pca10056.overlay new file mode 100644 index 0000000000000..90200eaedc7b4 --- /dev/null +++ b/tests/subsys/settings/functional/nrf52840_pca10056.overlay @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/delete-node/ &storage_partition; +/delete-node/ &scratch_partition; + +&flash0 { + /* + * For more information, see: + * http://docs.zephyrproject.org/latest/guides/dts/index.html#flash-partitions + */ + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + storage_partition: partition@de000 { + label = "storage"; + reg = <0x000de000 0x00010000>; + }; + }; +}; diff --git a/tests/subsys/settings/functional/nrf52_pca10040.overlay b/tests/subsys/settings/functional/nrf52_pca10040.overlay new file mode 100644 index 0000000000000..b16c30e1ae455 --- /dev/null +++ b/tests/subsys/settings/functional/nrf52_pca10040.overlay @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2018 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/delete-node/ &storage_partition; +/delete-node/ &scratch_partition; + +&flash0 { + /* + * For more information, see: + * http://docs.zephyrproject.org/latest/guides/dts/index.html#flash-partitions + */ + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + storage_partition: partition@70000 { + label = "storage"; + reg = <0x00070000 0x10000>; + }; + }; +}; diff --git a/tests/subsys/settings/functional/prj.conf b/tests/subsys/settings/functional/prj.conf new file mode 100644 index 0000000000000..767305cd62145 --- /dev/null +++ b/tests/subsys/settings/functional/prj.conf @@ -0,0 +1,12 @@ +CONFIG_ZTEST=y +CONFIG_STDOUT_CONSOLE=y +CONFIG_FLASH=y +CONFIG_FLASH_PAGE_LAYOUT=y +CONFIG_FLASH_MAP=y +CONFIG_ARM_MPU=n +CONFIG_FCB=y + +CONFIG_SETTINGS=y +CONFIG_SETTINGS_RUNTIME=y +CONFIG_SETTINGS_FCB=y +CONFIG_SETTINGS_USE_BASE64=n diff --git a/tests/subsys/settings/functional/prj_native_posix.conf b/tests/subsys/settings/functional/prj_native_posix.conf new file mode 100644 index 0000000000000..07595f0fd6e87 --- /dev/null +++ b/tests/subsys/settings/functional/prj_native_posix.conf @@ -0,0 +1,11 @@ +CONFIG_ZTEST=y +CONFIG_STDOUT_CONSOLE=y +CONFIG_FLASH=y +CONFIG_FLASH_PAGE_LAYOUT=y +CONFIG_FLASH_MAP=y +CONFIG_FCB=y + +CONFIG_SETTINGS=y +CONFIG_SETTINGS_RUNTIME=y +CONFIG_SETTINGS_FCB=y +CONFIG_SETTINGS_USE_BASE64=n diff --git a/tests/subsys/settings/functional/prj_qemu_x86.conf b/tests/subsys/settings/functional/prj_qemu_x86.conf new file mode 100644 index 0000000000000..07595f0fd6e87 --- /dev/null +++ b/tests/subsys/settings/functional/prj_qemu_x86.conf @@ -0,0 +1,11 @@ +CONFIG_ZTEST=y +CONFIG_STDOUT_CONSOLE=y +CONFIG_FLASH=y +CONFIG_FLASH_PAGE_LAYOUT=y +CONFIG_FLASH_MAP=y +CONFIG_FCB=y + +CONFIG_SETTINGS=y +CONFIG_SETTINGS_RUNTIME=y +CONFIG_SETTINGS_FCB=y +CONFIG_SETTINGS_USE_BASE64=n diff --git a/tests/subsys/settings/functional/src/settings_basic_test.c b/tests/subsys/settings/functional/src/settings_basic_test.c new file mode 100644 index 0000000000000..d68d0c7a38989 --- /dev/null +++ b/tests/subsys/settings/functional/src/settings_basic_test.c @@ -0,0 +1,312 @@ +/* + * Copyright (c) 2018 Laczen + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** @file + * @brief Settings functional test suite + * + */ + +#include +#include +#include +#include +#include +LOG_MODULE_REGISTER(settings_basic_test); + +/* + * Test the two support routines that settings provides: + * + * settings_name_steq(name, key, next): compares the start of name with key + * settings_name_next(name, next): returns the location of the first + * separator + */ + +static void test_support_rtn(void) +{ + const char test1[] = "bt/a/b/c/d"; + const char test2[] = "bt/a/b/c/d="; + const char *next1, *next2; + int rc; + + /* complete match: return 1, next = NULL */ + rc = settings_name_steq(test1, "bt/a/b/c/d", &next1); + zassert_true(rc == 1, "_steq comparison failure"); + zassert_is_null(next1, "_steq comparison next error"); + rc = settings_name_steq(test2, "bt/a/b/c/d", &next2); + zassert_true(rc == 1, "_steq comparison failure"); + zassert_is_null(next2, "_steq comparison next error"); + + /* partial match: return 1, next <> NULL */ + rc = settings_name_steq(test1, "bt/a/b/c", &next1); + zassert_true(rc == 1, "_steq comparison failure"); + zassert_not_null(next1, "_steq comparison next error"); + zassert_equal_ptr(next1, test1+9, "next points to wrong location"); + rc = settings_name_steq(test2, "bt/a/b/c", &next2); + zassert_true(rc == 1, "_steq comparison failure"); + zassert_not_null(next2, "_steq comparison next error"); + zassert_equal_ptr(next2, test2+9, "next points to wrong location"); + + /* no match: return 0, next = NULL */ + rc = settings_name_steq(test1, "bta", &next1); + zassert_true(rc == 0, "_steq comparison failure"); + zassert_is_null(next1, "_steq comparison next error"); + rc = settings_name_steq(test2, "bta", &next2); + zassert_true(rc == 0, "_steq comparison failure"); + zassert_is_null(next2, "_steq comparison next error"); + + /* no match: return 0, next = NULL */ + rc = settings_name_steq(test1, "b", &next1); + zassert_true(rc == 0, "_steq comparison failure"); + zassert_is_null(next1, "_steq comparison next error"); + rc = settings_name_steq(test2, "b", &next2); + zassert_true(rc == 0, "_steq comparison failure"); + zassert_is_null(next2, "_steq comparison next error"); + + /* first separator: return 2, next <> NULL */ + rc = settings_name_next(test1, &next1); + zassert_true(rc == 2, "_next wrong return value"); + zassert_not_null(next1, "_next wrong next"); + zassert_equal_ptr(next1, test1+3, "next points to wrong location"); + rc = settings_name_next(test2, &next2); + zassert_true(rc == 2, "_next wrong return value"); + zassert_not_null(next2, "_next wrong next"); + zassert_equal_ptr(next2, test2+3, "next points to wrong location"); + + /* second separator: return 1, next <> NULL */ + rc = settings_name_next(next1, &next1); + zassert_true(rc == 1, "_next wrong return value"); + zassert_not_null(next1, "_next wrong next"); + zassert_equal_ptr(next1, test1+5, "next points to wrong location"); + rc = settings_name_next(next2, &next2); + zassert_true(rc == 1, "_next wrong return value"); + zassert_not_null(next2, "_next wrong next"); + zassert_equal_ptr(next2, test2+5, "next points to wrong location"); + + /* third separator: return 1, next <> NULL */ + rc = settings_name_next(next1, &next1); + zassert_true(rc == 1, "_next wrong return value"); + zassert_not_null(next1, "_next wrong next"); + rc = settings_name_next(next2, &next2); + zassert_true(rc == 1, "_next wrong return value"); + zassert_not_null(next2, "_next wrong next"); + + /* fourth separator: return 1, next <> NULL */ + rc = settings_name_next(next1, &next1); + zassert_true(rc == 1, "_next wrong return value"); + zassert_not_null(next1, "_next wrong next"); + rc = settings_name_next(next2, &next2); + zassert_true(rc == 1, "_next wrong return value"); + zassert_not_null(next2, "_next wrong next"); + + /* fifth separator: return 1, next == NULL */ + rc = settings_name_next(next1, &next1); + zassert_true(rc == 1, "_next wrong return value"); + zassert_is_null(next1, "_next wrong next"); + rc = settings_name_next(next2, &next2); + zassert_true(rc == 1, "_next wrong return value"); + zassert_is_null(next2, "_next wrong next"); + +} + +struct stored_data { + u8_t val1; + u8_t val2; + u8_t val3; + bool en1; + bool en2; + bool en3; +}; + +struct stored_data data; + +int val1_set(const char *key, size_t len, settings_read_cb read_cb, + void *cb_arg) +{ + data.val1 = 1; + return 0; +} +int val1_commit(void) +{ + data.en1 = true; + return 0; +} +static struct settings_handler val1_settings = { + .name = "ps", + .h_set = val1_set, + .h_commit = val1_commit, +}; + +int val2_set(const char *key, size_t len, settings_read_cb read_cb, + void *cb_arg) +{ + data.val2 = 2; + return 0; +} +int val2_commit(void) +{ + data.en2 = true; + return 0; +} +static struct settings_handler val2_settings = { + .name = "ps/ss/ss", + .h_set = val2_set, + .h_commit = val2_commit, +}; + +int val3_set(const char *key, size_t len, settings_read_cb read_cb, + void *cb_arg) +{ + data.val3 = 3; + return 0; +} +int val3_commit(void) +{ + data.en3 = true; + return 0; +} +static struct settings_handler val3_settings = { + .name = "ps/ss", + .h_set = val3_set, + .h_commit = val3_commit, +}; + +/* helper routine to remove a handler from settings */ +int settings_deregister(struct settings_handler *handler) +{ + extern sys_slist_t settings_handlers; + + return sys_slist_find_and_remove(&settings_handlers, &handler->node); +} + +static void test_register_and_loading(void) +{ + int rc, err; + u8_t val = 0; + + rc = settings_subsys_init(); + zassert_true(rc == 0, "subsys init failed"); + + + settings_save_one("ps/ss/ss/val2", &val, sizeof(u8_t)); + + memset(&data, 0, sizeof(struct stored_data)); + + rc = settings_register(&val1_settings); + zassert_true(rc == 0, "register of val1 settings failed"); + + /* when we load settings now data.val1 should receive the value*/ + rc = settings_load(); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 1) && (data.val2 == 0) && (data.val3 == 0); + zassert_true(err, "wrong data value found"); + /* commit is only called for val1_settings */ + err = (data.en1) && (!data.en2) && (!data.en3); + zassert_true(err, "wrong data enable found"); + + /* Next register should be ok */ + rc = settings_register(&val2_settings); + zassert_true(rc == 0, "register of val2 settings failed"); + + /* Next register should fail (same name) */ + rc = settings_register(&val2_settings); + zassert_true(rc == -EEXIST, "double register of val2 settings allowed"); + + memset(&data, 0, sizeof(struct stored_data)); + /* when we load settings now data.val2 should receive the value*/ + rc = settings_load(); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 0) && (data.val2 == 2) && (data.val3 == 0); + zassert_true(err, "wrong data value found"); + /* commit is called for val1_settings and val2_settings*/ + err = (data.en1) && (data.en2) && (!data.en3); + zassert_true(err, "wrong data enable found"); + + settings_save_one("ps/ss/val3", &val, sizeof(u8_t)); + memset(&data, 0, sizeof(struct stored_data)); + /* when we load settings now data.val2 and data.val1 should receive a + * value + */ + rc = settings_load(); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 1) && (data.val2 == 2) && (data.val3 == 0); + zassert_true(err, "wrong data value found"); + /* commit is called for val1_settings and val2_settings*/ + err = (data.en1) && (data.en2) && (!data.en3); + zassert_true(err, "wrong data enable found"); + + /* val3 settings should be inserted in between val1_settings and + * val2_settings + */ + rc = settings_register(&val3_settings); + zassert_true(rc == 0, "register of val3 settings failed"); + memset(&data, 0, sizeof(struct stored_data)); + /* when we load settings now data.val2 and data.val3 should receive a + * value + */ + rc = settings_load(); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 0) && (data.val2 == 2) && (data.val3 == 3); + zassert_true(err, "wrong data value found"); + /* commit is called for val1_settings, val2_settings and val3_settings + */ + err = (data.en1) && (data.en2) && (data.en3); + zassert_true(err, "wrong data enable found"); + + settings_save_one("ps/val1", &val, sizeof(u8_t)); + memset(&data, 0, sizeof(struct stored_data)); + /* when we load settings all data should receive a value loaded */ + rc = settings_load(); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 1) && (data.val2 == 2) && (data.val3 == 3); + zassert_true(err, "wrong data value found"); + /* commit is called for all settings*/ + err = (data.en1) && (data.en2) && (data.en3); + zassert_true(err, "wrong data enable found"); + + memset(&data, 0, sizeof(struct stored_data)); + /* test subtree loading: subtree "ps/ss" data.val2 and data.val3 should + * receive a value + */ + rc = settings_load_subtree("ps/ss"); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 0) && (data.val2 == 2) && (data.val3 == 3); + zassert_true(err, "wrong data value found"); + /* commit is called for val2_settings and val3_settings */ + err = (!data.en1) && (data.en2) && (data.en3); + zassert_true(err, "wrong data enable found"); + + memset(&data, 0, sizeof(struct stored_data)); + /* test subtree loading: subtree "ps/ss/ss" only data.val2 should + * receive a value + */ + rc = settings_load_subtree("ps/ss/ss"); + zassert_true(rc == 0, "settings_load failed"); + err = (data.val1 == 0) && (data.val2 == 2) && (data.val3 == 0); + zassert_true(err, "wrong data value found"); + /* commit is called only for val2_settings */ + err = (!data.en1) && (data.en2) && (!data.en3); + zassert_true(err, "wrong data enable found"); + + /* clean up by deregisterring settings_handler */ + rc = settings_deregister(&val1_settings); + zassert_true(rc, "deregistering val1_settings failed"); + + rc = settings_deregister(&val2_settings); + zassert_true(rc, "deregistering val1_settings failed"); + + rc = settings_deregister(&val3_settings); + zassert_true(rc, "deregistering val1_settings failed"); +} + +void test_main(void) +{ + ztest_test_suite(settings_test_suite, + ztest_unit_test(test_support_rtn), + ztest_unit_test(test_register_and_loading) + ); + + ztest_run_test_suite(settings_test_suite); +} diff --git a/tests/subsys/settings/functional/testcase.yaml b/tests/subsys/settings/functional/testcase.yaml new file mode 100644 index 0000000000000..42061f0047e4b --- /dev/null +++ b/tests/subsys/settings/functional/testcase.yaml @@ -0,0 +1,4 @@ +tests: + system.settings.fcb: + platform_whitelist: nrf52840_pca10056 nrf52_pca10040 native_posix + tags: settings_fcb diff --git a/tests/subsys/settings/nffs/src/settings_test_nffs.c b/tests/subsys/settings/nffs/src/settings_test_nffs.c index 0b8359fbea30c..0a16d8bc37c1c 100644 --- a/tests/subsys/settings/nffs/src/settings_test_nffs.c +++ b/tests/subsys/settings/nffs/src/settings_test_nffs.c @@ -22,8 +22,8 @@ int test_export_block; int c2_var_count = 1; -int c1_handle_get(int argc, char **argv, char *val, int val_len_max); -int c1_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c1_handle_get(const char *name, char *val, int val_len_max); +int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); int c1_handle_commit(void); int c1_handle_export(int (*cb)(const char *name, @@ -41,23 +41,24 @@ struct settings_handler c_test_handlers[] = { -int c1_handle_get(int argc, char **argv, char *val, int val_len_max) +int c1_handle_get(const char *name, char *val, int val_len_max) { test_get_called = 1; + const char *next; - if (argc == 1 && !strcmp(argv[0], "mybar")) { + if (settings_name_steq(name, "mybar", &next) && !next) { val_len_max = MIN(val_len_max, sizeof(val8)); memcpy(val, &val8, MIN(val_len_max, sizeof(val8))); return val_len_max; } - if (argc == 1 && !strcmp(argv[0], "mybar16")) { + if (settings_name_steq(name, "mybar16", &next) && !next) { val_len_max = MIN(val_len_max, sizeof(val16)); memcpy(val, &val16, MIN(val_len_max, sizeof(val16))); return val_len_max; } - if (argc == 1 && !strcmp(argv[0], "mybar64")) { + if (settings_name_steq(name, "mybar64", &next) && !next) { val_len_max = MIN(val_len_max, sizeof(val64)); memcpy(val, &val64, MIN(val_len_max, sizeof(val64))); return val_len_max; @@ -66,14 +67,15 @@ int c1_handle_get(int argc, char **argv, char *val, int val_len_max) return -ENOENT; } -int c1_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, +int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) { int rc; + const char *next; size_t val_len; test_set_called = 1; - if (argc == 1 && !strcmp(argv[0], "mybar")) { + if (settings_name_steq(name, "mybar", &next) && !next) { val_len = len; zassert_true(val_len == 1, "bad set-value size"); @@ -82,7 +84,7 @@ int c1_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, return 0; } - if (argc == 1 && !strcmp(argv[0], "mybar16")) { + if (settings_name_steq(name, "mybar16", &next) && !next) { val_len = len; zassert_true(val_len == 2, "bad set-value size"); @@ -91,7 +93,7 @@ int c1_handle_set(int argc, char **argv, size_t len, settings_read_cb read_cb, return 0; } - if (argc == 1 && !strcmp(argv[0], "mybar64")) { + if (settings_name_steq(name, "mybar64", &next) && !next) { val_len = len; zassert_true(val_len == 8, "bad set-value size"); From 791309a48570926a15d626d21b242aab1eb8e835 Mon Sep 17 00:00:00 2001 From: Laczen JMS Date: Fri, 14 Jun 2019 18:23:39 +0200 Subject: [PATCH 2/2] subsys/settings: Update client modules This updates all client modules to const char processing of setting names. Update of peripheral_dis sample Signed-off-by: Laczen JMS --- samples/bluetooth/peripheral_dis/src/main.c | 6 +- .../src/storage.c | 22 +++-- subsys/bluetooth/host/gatt.c | 29 +++--- subsys/bluetooth/host/keys.c | 19 ++-- subsys/bluetooth/host/mesh/settings.c | 96 +++++++++++-------- subsys/bluetooth/host/settings.c | 31 +++--- subsys/bluetooth/host/settings.h | 4 +- subsys/bluetooth/services/dis.c | 18 ++-- 8 files changed, 130 insertions(+), 95 deletions(-) diff --git a/samples/bluetooth/peripheral_dis/src/main.c b/samples/bluetooth/peripheral_dis/src/main.c index cb2191703bc87..e9397e3966de6 100644 --- a/samples/bluetooth/peripheral_dis/src/main.c +++ b/samples/bluetooth/peripheral_dis/src/main.c @@ -45,7 +45,8 @@ static struct bt_conn_cb conn_callbacks = { .disconnected = disconnected, }; -static int zephyr_settings_fw_load(struct settings_store *cs); +static int zephyr_settings_fw_load(struct settings_store *cs, + const char *subtree); static const struct settings_store_itf zephyr_settings_fw_itf = { .csi_load = zephyr_settings_fw_load, @@ -55,7 +56,8 @@ static struct settings_store zephyr_settings_fw_store = { .cs_itf = &zephyr_settings_fw_itf }; -static int zephyr_settings_fw_load(struct settings_store *cs) +static int zephyr_settings_fw_load(struct settings_store *cs, + const char *subtree) { #if defined(CONFIG_BT_GATT_DIS_SETTINGS) diff --git a/samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/src/storage.c b/samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/src/storage.c index 81809a6378316..560945cf013fa 100644 --- a/samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/src/storage.c +++ b/samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/src/storage.c @@ -114,48 +114,52 @@ void save_on_flash(u8_t id) k_work_submit(&storage_work); } -static int ps_set(int argc, char **argv, size_t len_rd, +static int ps_set(const char *key, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { ssize_t len = 0; + int key_len; + const char *next; - if (argc == 1) { - if (!strcmp(argv[0], "rc")) { + key_len = settings_name_next(key, &next); + + if (!next) { + if (!strncmp(key, "rc", key_len)) { len = read_cb(cb_arg, &reset_counter, sizeof(reset_counter)); } - if (!strcmp(argv[0], "gdtt")) { + if (!strncmp(key, "gdtt", key_len)) { len = read_cb(cb_arg, &gen_def_trans_time_srv_user_data.tt, sizeof(gen_def_trans_time_srv_user_data.tt)); } - if (!strcmp(argv[0], "gpo")) { + if (!strncmp(key, "gpo", key_len)) { len = read_cb(cb_arg, &gen_power_onoff_srv_user_data.onpowerup, sizeof(gen_power_onoff_srv_user_data.onpowerup)); } - if (!strcmp(argv[0], "ltd")) { + if (!strncmp(key, "ltd", key_len)) { len = read_cb(cb_arg, &light_ctl_srv_user_data.lightness_temp_def, sizeof(light_ctl_srv_user_data.lightness_temp_def)); } - if (!strcmp(argv[0], "ltl")) { + if (!strncmp(key, "ltl", key_len)) { len = read_cb(cb_arg, &light_ctl_srv_user_data.lightness_temp_last, sizeof(light_ctl_srv_user_data.lightness_temp_last)); } - if (!strcmp(argv[0], "lr")) { + if (!strncmp(key, "lr", key_len)) { len = read_cb(cb_arg, &light_lightness_srv_user_data.lightness_range, sizeof(light_lightness_srv_user_data.lightness_range)); } - if (!strcmp(argv[0], "tr")) { + if (!strncmp(key, "tr", key_len)) { len = read_cb(cb_arg, &light_ctl_srv_user_data.temperature_range, sizeof(light_ctl_srv_user_data.temperature_range)); diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index 09dca7854ad0e..2c6026bc585d2 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -3558,26 +3558,29 @@ static u8_t ccc_load(const struct bt_gatt_attr *attr, void *user_data) return load->count ? BT_GATT_ITER_CONTINUE : BT_GATT_ITER_STOP; } -static int ccc_set(int argc, char **argv, size_t len_rd, - settings_read_cb read_cb, void *cb_arg) +static int ccc_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) { struct ccc_store ccc_store[CCC_STORE_MAX]; struct ccc_load load; bt_addr_le_t addr; int len, err; + const char *next; - if (argc < 1) { + settings_name_next(name, &next); + + if (!name) { BT_ERR("Insufficient number of arguments"); return -EINVAL; - } else if (argc == 1) { + } else if (!next) { load.addr_with_id.id = BT_ID_DEFAULT; } else { - load.addr_with_id.id = strtol(argv[1], NULL, 10); + load.addr_with_id.id = strtol(next, NULL, 10); } - err = bt_settings_decode_key(argv[0], &addr); + err = bt_settings_decode_key(name, &addr); if (err) { - BT_ERR("Unable to decode address %s", argv[0]); + BT_ERR("Unable to decode address %s", name); return -EINVAL; } @@ -3614,21 +3617,21 @@ static int ccc_set(int argc, char **argv, size_t len_rd, BT_SETTINGS_DEFINE(ccc, ccc_set, NULL, NULL); #if defined(CONFIG_BT_GATT_CACHING) -static int cf_set(int argc, char **argv, size_t len_rd, - settings_read_cb read_cb, void *cb_arg) +static int cf_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) { struct gatt_cf_cfg *cfg; bt_addr_le_t addr; int len, err; - if (argc < 1) { + if (!name) { BT_ERR("Insufficient number of arguments"); return -EINVAL; } - err = bt_settings_decode_key(argv[0], &addr); + err = bt_settings_decode_key(name, &addr); if (err) { - BT_ERR("Unable to decode address %s", argv[0]); + BT_ERR("Unable to decode address %s", name); return -EINVAL; } @@ -3662,7 +3665,7 @@ BT_SETTINGS_DEFINE(cf, cf_set, NULL, NULL); static u8_t stored_hash[16]; -static int db_hash_set(int argc, char **argv, size_t len_rd, +static int db_hash_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { int len; diff --git a/subsys/bluetooth/host/keys.c b/subsys/bluetooth/host/keys.c index 2e21ee159d0fb..b37a1d445eac2 100644 --- a/subsys/bluetooth/host/keys.c +++ b/subsys/bluetooth/host/keys.c @@ -277,8 +277,8 @@ int bt_keys_store(struct bt_keys *keys) return 0; } -static int keys_set(int argc, char **argv, size_t len_rd, - settings_read_cb read_cb, void *cb_arg) +static int keys_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) { struct bt_keys *keys; bt_addr_le_t addr; @@ -286,8 +286,9 @@ static int keys_set(int argc, char **argv, size_t len_rd, size_t len; int err; char val[BT_KEYS_STORAGE_LEN]; + const char *next; - if (argc < 1) { + if (!name) { BT_ERR("Insufficient number of arguments"); return -EINVAL; } @@ -298,18 +299,20 @@ static int keys_set(int argc, char **argv, size_t len_rd, return -EINVAL; } - BT_DBG("argv[0] %s val %s", argv[0], (len) ? val : "(null)"); + BT_DBG("name %s val %s", name, (len) ? val : "(null)"); - err = bt_settings_decode_key(argv[0], &addr); + err = bt_settings_decode_key(name, &addr); if (err) { - BT_ERR("Unable to decode address %s", argv[0]); + BT_ERR("Unable to decode address %s", name); return -EINVAL; } - if (argc == 1) { + settings_name_next(name, &next); + + if (!next) { id = BT_ID_DEFAULT; } else { - id = strtol(argv[1], NULL, 10); + id = strtol(next, NULL, 10); } if (!len) { diff --git a/subsys/bluetooth/host/mesh/settings.c b/subsys/bluetooth/host/mesh/settings.c index 3f19d00f2efbf..49521af803439 100644 --- a/subsys/bluetooth/host/mesh/settings.c +++ b/subsys/bluetooth/host/mesh/settings.c @@ -146,8 +146,8 @@ static inline int mesh_x_set(settings_read_cb read_cb, void *cb_arg, void *out, return 0; } -static int net_set(int argc, char **argv, size_t len_rd, - settings_read_cb read_cb, void *cb_arg) +static int net_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) { struct net_val net; int err; @@ -175,8 +175,8 @@ static int net_set(int argc, char **argv, size_t len_rd, return 0; } -static int iv_set(int argc, char **argv, size_t len_rd, - settings_read_cb read_cb, void *cb_arg) +static int iv_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) { struct iv_val iv; int err; @@ -205,8 +205,8 @@ static int iv_set(int argc, char **argv, size_t len_rd, return 0; } -static int seq_set(int argc, char **argv, size_t len_rd, - settings_read_cb read_cb, void *cb_arg) +static int seq_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) { struct seq_val seq; int err; @@ -269,7 +269,7 @@ static struct bt_mesh_rpl *rpl_alloc(u16_t src) return NULL; } -static int rpl_set(int argc, char **argv, size_t len_rd, +static int rpl_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { struct bt_mesh_rpl *entry; @@ -277,12 +277,12 @@ static int rpl_set(int argc, char **argv, size_t len_rd, int err; u16_t src; - if (argc < 1) { - BT_ERR("Invalid argc (%d)", argc); + if (!name) { + BT_ERR("Insufficient number of arguments"); return -ENOENT; } - src = strtol(argv[0], NULL, 16); + src = strtol(name, NULL, 16); entry = rpl_find(src); if (len_rd == 0) { @@ -319,7 +319,7 @@ static int rpl_set(int argc, char **argv, size_t len_rd, return 0; } -static int net_key_set(int argc, char **argv, size_t len_rd, +static int net_key_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { struct bt_mesh_subnet *sub; @@ -327,7 +327,12 @@ static int net_key_set(int argc, char **argv, size_t len_rd, int i, err; u16_t net_idx; - net_idx = strtol(argv[0], NULL, 16); + if (!name) { + BT_ERR("Insufficient number of arguments"); + return -ENOENT; + } + + net_idx = strtol(name, NULL, 16); sub = bt_mesh_subnet_get(net_idx); if (len_rd == 0) { @@ -382,7 +387,7 @@ static int net_key_set(int argc, char **argv, size_t len_rd, return 0; } -static int app_key_set(int argc, char **argv, size_t len_rd, +static int app_key_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { struct bt_mesh_app_key *app; @@ -390,7 +395,12 @@ static int app_key_set(int argc, char **argv, size_t len_rd, u16_t app_idx; int err; - app_idx = strtol(argv[0], NULL, 16); + if (!name) { + BT_ERR("Insufficient number of arguments"); + return -ENOENT; + } + + app_idx = strtol(name, NULL, 16); if (len_rd == 0) { BT_DBG("val (null)"); @@ -434,7 +444,7 @@ static int app_key_set(int argc, char **argv, size_t len_rd, return 0; } -static int hb_pub_set(int argc, char **argv, size_t len_rd, +static int hb_pub_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { struct bt_mesh_hb_pub *pub = bt_mesh_hb_pub_get(); @@ -479,7 +489,7 @@ static int hb_pub_set(int argc, char **argv, size_t len_rd, return 0; } -static int cfg_set(int argc, char **argv, size_t len_rd, +static int cfg_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { struct bt_mesh_cfg_srv *cfg = bt_mesh_cfg_get(); @@ -604,19 +614,21 @@ static int mod_set_pub(struct bt_mesh_model *mod, size_t len_rd, return 0; } -static int mod_set(bool vnd, int argc, char **argv, size_t len_rd, +static int mod_set(bool vnd, const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { struct bt_mesh_model *mod; u8_t elem_idx, mod_idx; u16_t mod_key; + int len; + const char *next; - if (argc < 2) { - BT_ERR("Too small argc (%d)", argc); + if (!name) { + BT_ERR("Insufficient number of arguments"); return -ENOENT; } - mod_key = strtol(argv[0], NULL, 16); + mod_key = strtol(name, NULL, 16); elem_idx = mod_key >> 8; mod_idx = mod_key; @@ -630,37 +642,44 @@ static int mod_set(bool vnd, int argc, char **argv, size_t len_rd, return -ENOENT; } - if (!strcmp(argv[1], "bind")) { + len = settings_name_next(name, &next); + + if (!next) { + BT_ERR("Insufficient number of arguments"); + return -ENOENT; + } + + if (!strncmp(next, "bind", len)) { return mod_set_bind(mod, len_rd, read_cb, cb_arg); } - if (!strcmp(argv[1], "sub")) { + if (!strncmp(next, "sub", len)) { return mod_set_sub(mod, len_rd, read_cb, cb_arg); } - if (!strcmp(argv[1], "pub")) { + if (!strncmp(next, "pub", len)) { return mod_set_pub(mod, len_rd, read_cb, cb_arg); } - BT_WARN("Unknown module key %s", argv[1]); + BT_WARN("Unknown module key %s", next); return -ENOENT; } -static int sig_mod_set(int argc, char **argv, size_t len_rd, +static int sig_mod_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { - return mod_set(false, argc, argv, len_rd, read_cb, cb_arg); + return mod_set(false, name, len_rd, read_cb, cb_arg); } -static int vnd_mod_set(int argc, char **argv, size_t len_rd, +static int vnd_mod_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { - return mod_set(true, argc, argv, len_rd, read_cb, cb_arg); + return mod_set(true, name, len_rd, read_cb, cb_arg); } const struct mesh_setting { const char *name; - int (*func)(int argc, char **argv, size_t len_rd, + int (*func)(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg); } settings[] = { { "Net", net_set }, @@ -675,27 +694,26 @@ const struct mesh_setting { { "v", vnd_mod_set }, }; -static int mesh_set(int argc, char **argv, size_t len_rd, +static int mesh_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { - int i; + int i, len; + const char *next; - if (argc < 1) { + if (!name) { BT_ERR("Insufficient number of arguments"); return -EINVAL; } - for (i = 0; i < ARRAY_SIZE(settings); i++) { - if (!strcmp(settings[i].name, argv[0])) { - argc--; - argv++; + len = settings_name_next(name, &next); - return settings[i].func(argc, argv, len_rd, read_cb, - cb_arg); + for (i = 0; i < ARRAY_SIZE(settings); i++) { + if (!strncmp(settings[i].name, name, len)) { + return settings[i].func(next, len_rd, read_cb, cb_arg); } } - BT_WARN("No matching handler for key %s", log_strdup(argv[0])); + BT_WARN("No matching handler for key %s", log_strdup(name)); return -ENOENT; } diff --git a/subsys/bluetooth/host/settings.c b/subsys/bluetooth/host/settings.c index e7ebe4ef87987..e2a0483a99248 100644 --- a/subsys/bluetooth/host/settings.c +++ b/subsys/bluetooth/host/settings.c @@ -43,12 +43,12 @@ void bt_settings_encode_key(char *path, size_t path_size, const char *subsys, BT_DBG("Encoded path %s", log_strdup(path)); } -int bt_settings_decode_key(char *key, bt_addr_le_t *addr) +int bt_settings_decode_key(const char *key, bt_addr_le_t *addr) { bool high; int i; - if (strlen(key) != 13) { + if (settings_name_next(key, NULL) != 13) { return -EINVAL; } @@ -86,24 +86,27 @@ int bt_settings_decode_key(char *key, bt_addr_le_t *addr) return 0; } -static int set(int argc, char **argv, size_t len_rd, settings_read_cb read_cb, +static int set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) { ssize_t len; - const struct bt_settings_handler *h; + const char *next; - for (h = _bt_settings_start; h < _bt_settings_end; h++) { - if (!strcmp(argv[0], h->name)) { - argc--; - argv++; + if (!name) { + BT_ERR("Insufficient number of arguments"); + return -ENOENT; + } - return h->set(argc, argv, len_rd, read_cb, - cb_arg); + len = settings_name_next(name, &next); + + for (h = _bt_settings_start; h < _bt_settings_end; h++) { + if (!strncmp(name, h->name, len)) { + return h->set(next, len_rd, read_cb, cb_arg); } } - if (!strcmp(argv[0], "id")) { + if (!strncmp(name, "id", len)) { /* Any previously provided identities supersede flash */ if (atomic_test_bit(bt_dev.flags, BT_DEV_PRESET_ID)) { BT_WARN("Ignoring identities stored in flash"); @@ -137,7 +140,7 @@ static int set(int argc, char **argv, size_t len_rd, settings_read_cb read_cb, } #if defined(CONFIG_BT_DEVICE_NAME_DYNAMIC) - if (!strcmp(argv[0], "name")) { + if (!strncmp(name, "name", len)) { len = read_cb(cb_arg, &bt_dev.name, sizeof(bt_dev.name) - 1); if (len < 0) { BT_ERR("Failed to read device name from storage" @@ -152,7 +155,7 @@ static int set(int argc, char **argv, size_t len_rd, settings_read_cb read_cb, #endif #if defined(CONFIG_BT_PRIVACY) - if (!strcmp(argv[0], "irk")) { + if (!strncmp(name, "irk", len)) { len = read_cb(cb_arg, bt_dev.irk, sizeof(bt_dev.irk)); if (len < sizeof(bt_dev.irk[0])) { if (len < 0) { @@ -184,7 +187,7 @@ static int set(int argc, char **argv, size_t len_rd, settings_read_cb read_cb, static void save_id(struct k_work *work) { int err; - + BT_INFO("Saving ID"); err = settings_save_one("bt/id", &bt_dev.id_addr, ID_DATA_LEN(bt_dev.id_addr)); if (err) { diff --git a/subsys/bluetooth/host/settings.h b/subsys/bluetooth/host/settings.h index 88f436dd63cd4..069346d9eaec8 100644 --- a/subsys/bluetooth/host/settings.h +++ b/subsys/bluetooth/host/settings.h @@ -6,7 +6,7 @@ struct bt_settings_handler { const char *name; - int (*set)(int argc, char **argv, size_t len, settings_read_cb read_cb, + int (*set)(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); int (*commit)(void); int (*export)(int (*func)(const char *name, @@ -30,7 +30,7 @@ struct bt_settings_handler { /* Helpers for keys containing a bdaddr */ void bt_settings_encode_key(char *path, size_t path_size, const char *subsys, bt_addr_le_t *addr, const char *key); -int bt_settings_decode_key(char *key, bt_addr_le_t *addr); +int bt_settings_decode_key(const char *key, bt_addr_le_t *addr); void bt_settings_save_id(void); diff --git a/subsys/bluetooth/services/dis.c b/subsys/bluetooth/services/dis.c index c98615a84fbeb..3383504893b40 100644 --- a/subsys/bluetooth/services/dis.c +++ b/subsys/bluetooth/services/dis.c @@ -144,12 +144,14 @@ BT_GATT_SERVICE_DEFINE(dis_svc, ); #if defined(CONFIG_BT_SETTINGS) && defined(CONFIG_BT_GATT_DIS_SETTINGS) -static int dis_set(int argc, char **argv, size_t len_rd, +static int dis_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *store) { - int len; + int len, nlen; + const char *next; - if (!strcmp(argv[0], "manuf")) { + nlen = settings_name_next(name, &next); + if (!strncmp(name, "manuf", nlen)) { len = read_cb(store, &dis_manuf, sizeof(dis_manuf) - 1); if (len < 0) { BT_ERR("Failed to read manufacturer from storage" @@ -161,7 +163,7 @@ static int dis_set(int argc, char **argv, size_t len_rd, } return 0; } - if (!strcmp(argv[0], "model")) { + if (!strncmp(name, "model", nlen)) { len = read_cb(store, &dis_model, sizeof(dis_model) - 1); if (len < 0) { BT_ERR("Failed to read model from storage" @@ -174,7 +176,7 @@ static int dis_set(int argc, char **argv, size_t len_rd, return 0; } #if defined(CONFIG_BT_GATT_DIS_SERIAL_NUMBER) - if (!strcmp(argv[0], "serial")) { + if (!strncmp(name, "serial", nlen)) { len = read_cb(store, &dis_serial_number, sizeof(dis_serial_number) - 1); if (len < 0) { @@ -189,7 +191,7 @@ static int dis_set(int argc, char **argv, size_t len_rd, } #endif #if defined(CONFIG_BT_GATT_DIS_FW_REV) - if (!strcmp(argv[0], "fw")) { + if (!strncmp(name, "fw", nlen)) { len = read_cb(store, &dis_fw_rev, sizeof(dis_fw_rev) - 1); if (len < 0) { BT_ERR("Failed to read firmware revision from storage" @@ -203,7 +205,7 @@ static int dis_set(int argc, char **argv, size_t len_rd, } #endif #if defined(CONFIG_BT_GATT_DIS_HW_REV) - if (!strcmp(argv[0], "hw")) { + if (!strncmp(name, "hw", nlen)) { len = read_cb(store, &dis_hw_rev, sizeof(dis_hw_rev) - 1); if (len < 0) { BT_ERR("Failed to read hardware revision from storage" @@ -217,7 +219,7 @@ static int dis_set(int argc, char **argv, size_t len_rd, } #endif #if defined(CONFIG_BT_GATT_DIS_SW_REV) - if (!strcmp(argv[0], "sw")) { + if (!strncmp(name, "sw", nlen)) { len = read_cb(store, &dis_sw_rev, sizeof(dis_sw_rev) - 1); if (len < 0) { BT_ERR("Failed to read software revision from storage"