Applied
On Fri, 2019-12-20 at 11:13 -0800, Inga Stotland wrote:
> This removes a roundabout logic that was required to delete
> a json entry from a json array using libjsonc utilities.
> A new API function, json_object_array_del_idx(), implemented in
> json-c version 0.13 simplifies array entry removal to one call.
>
> Also, add requirement to configure.ac for json-c version >= 0.13
> ---
> configure.ac | 4 +-
> mesh/mesh-config-json.c | 126 ++++++++--------------------------------
> tools/mesh/mesh-db.c | 42 +++-----------
> 3 files changed, 34 insertions(+), 138 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 57a816fd5..6f7717f24 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -230,8 +230,8 @@ AC_ARG_ENABLE(mesh, AC_HELP_STRING([--enable-mesh],
> AM_CONDITIONAL(MESH, test "${enable_mesh}" = "yes")
>
> if (test "${enable_mesh}" = "yes"); then
> - PKG_CHECK_MODULES(JSONC, json-c, dummy=yes,
> - AC_MSG_ERROR(json-c is required))
> + PKG_CHECK_MODULES(JSONC, json-c >= 0.13, dummy=yes,
> + AC_MSG_ERROR(json-c >= 0.13 is required))
> AC_SUBST(JSON_CFLAGS)
> AC_SUBST(JSON_LIBS)
> fi
> diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
> index 5ca2961b0..755caab0e 100644
> --- a/mesh/mesh-config-json.c
> +++ b/mesh/mesh-config-json.c
> @@ -232,15 +232,9 @@ static bool jarray_has_string(json_object *jarray, char *str, size_t len)
> return false;
> }
>
> -static json_object *jarray_string_del(json_object *jarray, char *str,
> - size_t len)
> +static void jarray_string_del(json_object *jarray, char *str, size_t len)
> {
> int i, sz = json_object_array_length(jarray);
> - json_object *jarray_new;
> -
> - jarray_new = json_object_new_array();
> - if (!jarray_new)
> - return NULL;
>
> for (i = 0; i < sz; ++i) {
> json_object *jentry;
> @@ -248,14 +242,13 @@ static json_object *jarray_string_del(json_object *jarray, char *str,
>
> jentry = json_object_array_get_idx(jarray, i);
> str_entry = (char *)json_object_get_string(jentry);
> - if (str_entry && !strncmp(str, str_entry, len))
> - continue;
>
> - json_object_get(jentry);
> - json_object_array_add(jarray_new, jentry);
> - }
> + if (str_entry && !strncmp(str, str_entry, len)) {
> + json_object_array_del_idx(jarray, i, 1);
> + return;
> + }
>
> - return jarray_new;
> + }
> }
>
> static json_object *get_key_object(json_object *jarray, uint16_t idx)
> @@ -304,29 +297,21 @@ static bool get_key_index(json_object *jobj, const char *keyword,
> return true;
> }
>
> -static json_object *jarray_key_del(json_object *jarray, int16_t idx)
> +static void jarray_key_del(json_object *jarray, int16_t idx)
> {
> - json_object *jarray_new;
> int i, sz = json_object_array_length(jarray);
>
> - jarray_new = json_object_new_array();
> - if (!jarray_new)
> - return NULL;
> -
> for (i = 0; i < sz; ++i) {
> json_object *jentry;
> uint16_t nidx;
>
> jentry = json_object_array_get_idx(jarray, i);
>
> - if (get_key_index(jentry, "index", &nidx) && nidx == idx)
> - continue;
> -
> - json_object_get(jentry);
> - json_object_array_add(jarray_new, jentry);
> + if (get_key_index(jentry, "index", &nidx) && nidx == idx) {
> + json_object_array_del_idx(jarray, i, 1);
> + return;
> + }
> }
> -
> - return jarray_new;
> }
>
> static bool read_unicast_address(json_object *jobj, uint16_t *unicast)
> @@ -660,39 +645,20 @@ bool mesh_config_net_key_update(struct mesh_config *cfg, uint16_t idx,
>
> bool mesh_config_net_key_del(struct mesh_config *cfg, uint16_t idx)
> {
> - json_object *jnode, *jarray, *jarray_new;
> + json_object *jnode, *jarray;
>
> if (!cfg)
> return false;
>
> jnode = cfg->jnode;
>
> - /* TODO: Decide if we treat this as an error: no network keys??? */
> if (!json_object_object_get_ex(jnode, "netKeys", &jarray))
> return true;
>
> - /* Check if matching entry exists */
> - if (!get_key_object(jarray, idx))
> - return true;
> + jarray_key_del(jarray, idx);
>
> - if (json_object_array_length(jarray) == 1) {
> + if (!json_object_array_length(jarray))
> json_object_object_del(jnode, "netKeys");
> - /* TODO: Do we raise an error here? */
> - l_warn("Removing the last network key! Zero keys left.");
> - return save_config(jnode, cfg->node_dir_path);
> - }
> -
> - /*
> - * There is no easy way to delete a value from json array.
> - * Create a new copy without specified element and
> - * then remove old array.
> - */
> - jarray_new = jarray_key_del(jarray, idx);
> - if (!jarray_new)
> - return false;
> -
> - json_object_object_del(jnode, "netKeys");
> - json_object_object_add(jnode, "netKeys", jarray_new);
>
> return save_config(jnode, cfg->node_dir_path);
> }
> @@ -810,7 +776,7 @@ bool mesh_config_app_key_update(struct mesh_config *cfg, uint16_t app_idx,
> bool mesh_config_app_key_del(struct mesh_config *cfg, uint16_t net_idx,
> uint16_t idx)
> {
> - json_object *jnode, *jarray, *jarray_new;
> + json_object *jnode, *jarray;
>
> if (!cfg)
> return false;
> @@ -820,26 +786,10 @@ bool mesh_config_app_key_del(struct mesh_config *cfg, uint16_t net_idx,
> if (!json_object_object_get_ex(jnode, "appKeys", &jarray))
> return true;
>
> - /* Check if matching entry exists */
> - if (!get_key_object(jarray, idx))
> - return true;
> + jarray_key_del(jarray, idx);
>
> - if (json_object_array_length(jarray) == 1) {
> + if (!json_object_array_length(jarray))
> json_object_object_del(jnode, "appKeys");
> - return true;
> - }
> -
> - /*
> - * There is no easy way to delete a value from json array.
> - * Create a new copy without specified element and
> - * then remove old array.
> - */
> - jarray_new = jarray_key_del(jarray, idx);
> - if (!jarray_new)
> - return false;
> -
> - json_object_object_del(jnode, "appKeys");
> - json_object_object_add(jnode, "appKeys", jarray_new);
>
> return save_config(jnode, cfg->node_dir_path);
> }
> @@ -893,7 +843,7 @@ bool mesh_config_model_binding_del(struct mesh_config *cfg, uint16_t ele_addr,
> bool vendor, uint32_t mod_id,
> uint16_t app_idx)
> {
> - json_object *jnode, *jmodel, *jarray, *jarray_new;
> + json_object *jnode, *jmodel, *jarray;
> int ele_idx;
> char buf[5];
>
> @@ -915,25 +865,10 @@ bool mesh_config_model_binding_del(struct mesh_config *cfg, uint16_t ele_addr,
>
> snprintf(buf, 5, "%4.4x", app_idx);
>
> - if (!jarray_has_string(jarray, buf, 4))
> - return true;
> + jarray_string_del(jarray, buf, 4);
>
> - if (json_object_array_length(jarray) == 1) {
> + if (!json_object_array_length(jarray))
> json_object_object_del(jmodel, "bind");
> - return true;
> - }
> -
> - /*
> - * There is no easy way to delete a value from json array.
> - * Create a new copy without specified element and
> - * then remove old array.
> - */
> - jarray_new = jarray_string_del(jarray, buf, 4);
> - if (!jarray_new)
> - return false;
> -
> - json_object_object_del(jmodel, "bind");
> - json_object_object_add(jmodel, "bind", jarray_new);
>
> return save_config(jnode, cfg->node_dir_path);
> }
> @@ -1994,7 +1929,7 @@ bool mesh_config_model_sub_del(struct mesh_config *cfg, uint16_t ele_addr,
> uint32_t mod_id, bool vendor,
> struct mesh_config_sub *sub)
> {
> - json_object *jnode, *jmodel, *jarray, *jarray_new;
> + json_object *jnode, *jmodel, *jarray;
> char buf[33];
> int len, ele_idx;
>
> @@ -2022,25 +1957,10 @@ bool mesh_config_model_sub_del(struct mesh_config *cfg, uint16_t ele_addr,
> len = 32;
> }
>
> - if (!jarray_has_string(jarray, buf, len))
> - return true;
> + jarray_string_del(jarray, buf, len);
>
> - if (json_object_array_length(jarray) == 1) {
> + if (!json_object_array_length(jarray))
> json_object_object_del(jmodel, "subscribe");
> - return true;
> - }
> -
> - /*
> - * There is no easy way to delete a value from a json array.
> - * Create a new copy without specified element and
> - * then remove old array.
> - */
> - jarray_new = jarray_string_del(jarray, buf, len);
> - if (!jarray_new)
> - return false;
> -
> - json_object_object_del(jmodel, "subscribe");
> - json_object_object_add(jmodel, "subscribe", jarray_new);
>
> return save_config(jnode, cfg->node_dir_path);
> }
> diff --git a/tools/mesh/mesh-db.c b/tools/mesh/mesh-db.c
> index 8f5cd186f..5dbb91440 100644
> --- a/tools/mesh/mesh-db.c
> +++ b/tools/mesh/mesh-db.c
> @@ -419,15 +419,10 @@ bool mesh_db_node_net_key_add(uint16_t unicast, uint16_t idx)
> return add_node_key(jnode, "netKeys", idx);
> }
>
> -static json_object *jarray_key_del(json_object *jarray, int16_t idx)
> +static void jarray_key_del(json_object *jarray, int16_t idx)
> {
> - json_object *jarray_new;
> int i, sz = json_object_array_length(jarray);
>
> - jarray_new = json_object_new_array();
> - if (!jarray_new)
> - return NULL;
> -
> for (i = 0; i < sz; ++i) {
> json_object *jentry, *jval;
> uint16_t val;
> @@ -436,48 +431,29 @@ static json_object *jarray_key_del(json_object *jarray, int16_t idx)
> jentry = json_object_array_get_idx(jarray, i);
>
> if (!json_object_object_get_ex(jentry, "index", &jval))
> - goto fail;
> + continue;
>
> str = json_object_get_string(jval);
>
> if (sscanf(str, "%04hx", &val) != 1)
> - goto fail;
> -
> - if (val == idx)
> continue;
>
> - json_object_get(jentry);
> - json_object_array_add(jarray_new, jentry);
> - }
> + if (val == idx) {
> + json_object_array_del_idx(jarray, i, 1);
> + return;
> + }
>
> - return jarray_new;
> -fail:
> - json_object_put(jarray_new);
> - return NULL;
> + }
> }
>
> static bool delete_key(json_object *jobj, const char *desc, uint16_t idx)
> {
> - json_object *jarray, *jarray_new;
> + json_object *jarray;
>
> if (!json_object_object_get_ex(jobj, desc, &jarray))
> return true;
>
> - /* Check if matching entry exists */
> - if (!get_key_object(jarray, idx))
> - return true;
> -
> - /*
> - * There is no easy way to delete a value from a json array.
> - * Create a new copy without specified element and
> - * then remove old array.
> - */
> - jarray_new = jarray_key_del(jarray, idx);
> - if (!jarray_new)
> - return false;
> -
> - json_object_object_del(jobj, desc);
> - json_object_object_add(jobj, desc, jarray_new);
> + jarray_key_del(jarray, idx);
>
> return mesh_config_save((struct mesh_config *) cfg, true,
> NULL, NULL);
