Hi Inga > -----Original Message----- > From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth- > owner@xxxxxxxxxxxxxxx] On Behalf Of Inga Stotland > Sent: Thursday, February 21, 2019 10:32 PM > To: linux-bluetooth@xxxxxxxxxxxxxxx > Cc: Gix, Brian <brian.gix@xxxxxxxxx>; johan.hedberg@xxxxxxxxx; > luiz.dentz@xxxxxxxxx; Stotland, Inga <inga.stotland@xxxxxxxxx> > Subject: [PATCH BlueZ 2/3 v2] mesh: Cleanup storage save and remove > procedures > > To remove a node config directory completely, the directory needs to be > empty. Both node.json and node,json.bak files must are deleted. > > Also, change storage_save_config() type to void to eliminate meaningless > checks. > --- > mesh/node.c | 14 ++++++++++---- > mesh/node.h | 1 + > mesh/storage.c | 34 ++++++++++++++++++++++++---------- > mesh/storage.h | 2 +- > 4 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/mesh/node.c b/mesh/node.c > index 6c5cd9c39..6a7b4a260 100644 > --- a/mesh/node.c > +++ b/mesh/node.c > @@ -392,8 +392,7 @@ static void cleanup_node(void *data) > /* Preserve the last sequence number */ > storage_write_sequence_number(net, > mesh_net_get_seq_num(net)); > > - if (storage_save_config(node, true, NULL, NULL)) > - l_info("Saved final config to %s", node->cfg_file); > + storage_save_config(node, true, NULL, NULL); > } > > free_node_resources(node); > @@ -958,6 +957,14 @@ void node_id_set(struct mesh_node *node, > uint16_t id) > node->id = id; > } > > +uint16_t node_id_get(struct mesh_node *node) { > + if (!node) > + return 0; > + > + return node->id; > +} > + > static void attach_io(void *a, void *b) { > struct mesh_node *node = a; > @@ -1757,8 +1764,7 @@ bool node_add_pending_local(struct mesh_node > *node, void *prov_node_info, > return false; > } > > - if (!storage_save_config(node, true, NULL, NULL)) > - return false; > + storage_save_config(node, true, NULL, NULL); > > /* Initialize configuration server model */ > mesh_config_srv_init(node, PRIMARY_ELE_IDX); diff --git > a/mesh/node.h b/mesh/node.h index ee1d4a600..954dfca75 100644 > --- a/mesh/node.h > +++ b/mesh/node.h > @@ -87,6 +87,7 @@ int node_attach(const char *app_path, const char > *sender, uint64_t token, > node_attach_ready_func_t > cb); > void node_build_attach_reply(struct l_dbus_message *reply, uint64_t > token); void node_id_set(struct mesh_node *node, uint16_t node_id); > +uint16_t node_id_get(struct mesh_node *node); > bool node_dbus_init(struct l_dbus *bus); void node_cleanup_all(void); > void node_jconfig_set(struct mesh_node *node, void *jconfig); diff --git > a/mesh/storage.c b/mesh/storage.c index e79037375..fe3102fba 100644 > --- a/mesh/storage.c > +++ b/mesh/storage.c > @@ -341,14 +341,14 @@ bool storage_write_sequence_number(struct > mesh_net *net, uint32_t seq) > struct mesh_node *node = mesh_net_node_get(net); > json_object *jnode = node_jconfig_get(node); > bool result; > - l_debug(""); > + > result = mesh_db_write_int(jnode, "sequenceNumber", seq); > if (!result) > return false; > > - result = storage_save_config(node, false, NULL, NULL); > + storage_save_config(node, false, NULL, NULL); > > - return result; > + return true; > } > > static bool save_config(json_object *jnode, const char *config_name) @@ - > 408,15 +408,12 @@ static void idle_save_config(void *user_data) > l_free(info); > } > > -bool storage_save_config(struct mesh_node *node, bool no_wait, > +void storage_save_config(struct mesh_node *node, bool no_wait, > mesh_status_func_t cb, void > *user_data) { > struct write_info *info; > > info = l_new(struct write_info, 1); > - if (!info) > - return false; > - l_debug(""); > info->jnode = node_jconfig_get(node); > info->config_name = node_cfg_file_get(node); > info->cb = cb; > @@ -426,8 +423,6 @@ bool storage_save_config(struct mesh_node *node, > bool no_wait, > idle_save_config(info); > else > l_idle_oneshot(idle_save_config, info, NULL); > - > - return true; > } > > static int create_dir(const char *dirname) @@ -543,7 +538,7 @@ bool > storage_create_node_config(struct mesh_node *node, void *data) > > do { > l_getrandom(&node_id, 2); > - if (!l_queue_find(node_ids, simple_match, > + if (node_id && !l_queue_find(node_ids, simple_match, > L_UINT_TO_PTR(node_id))) > break; > } while (++num_tries < 10); > @@ -571,6 +566,8 @@ bool storage_create_node_config(struct mesh_node > *node, void *data) > node_jconfig_set(node, jnode); > node_cfg_file_set(node, filename); > > + l_queue_push_tail(node_ids, L_UINT_TO_PTR(node_id)); > + > return true; > fail: > json_object_put(jnode); > @@ -583,15 +580,20 @@ void storage_remove_node_config(struct > mesh_node *node) > char *cfgname; > struct json_object *jnode; > const char *dir_name; > + uint16_t node_id; > + size_t len; > + char *bak; > > if (!node) > return; > > + /* Free the node config json object */ > jnode = node_jconfig_get(node); > if (jnode) > json_object_put(jnode); > node_jconfig_set(node, NULL); > > + /* Delete node configuration file */ > cfgname = (char *) node_cfg_file_get(node); > if (!cfgname) > return; > @@ -599,6 +601,15 @@ void storage_remove_node_config(struct > mesh_node *node) > l_debug("Delete node config file %s", cfgname); > remove(cfgname); > > + /* Delete the backup file */ > + len = strlen(cfgname) + 5; > + bak = l_malloc(len); > + strncpy(bak, cfgname, len); > + bak = strncat(bak, ".bak", 5); > + remove(bak); > + l_free(bak); The only issue I really have is that the node deletion will fail if more than the expected files are in the directory. The number of files I personally expect to grow, and rather than growing a hard coded list of files that need to be removed, I really think we should try to (safely) remove the entire tree. The code will be smaller and simpler. But we should definitely limit the deletion root to a validated path (perhaps containing .../bluetooth/mesh/<xxxx>) > + > + /* Delete the node directory */ > dir_name = dirname(cfgname); > > l_debug("Delete directory %s", dir_name); @@ -606,4 +617,7 @@ > void storage_remove_node_config(struct mesh_node *node) > > l_free(cfgname); > node_cfg_file_set(node, NULL); > + > + node_id = node_id_get(node); > + l_queue_remove(node_ids, L_UINT_TO_PTR(node_id)); > } > diff --git a/mesh/storage.h b/mesh/storage.h index 85f7899bc..8b14c8e8e > 100644 > --- a/mesh/storage.h > +++ b/mesh/storage.h > @@ -23,7 +23,7 @@ struct mesh_node; > bool storage_load_nodes(const char *dir); bool > storage_create_node_config(struct mesh_node *node, void *db_node); > void storage_remove_node_config(struct mesh_node *node); -bool > storage_save_config(struct mesh_node *node, bool no_wait, > +void storage_save_config(struct mesh_node *node, bool no_wait, > mesh_status_func_t cb, void > *user_data); bool storage_model_bind(struct mesh_node *node, uint16_t > addr, uint32_t id, > uint16_t app_idx, bool > unbind); > -- > 2.17.2
