2017-12-18 20:35:51

by Stotland, Inga

Subject: [PATCH BlueZ] mesh:meshctl: make model parsing more manageable

Extract functionality for finding an existing model from
parse_configuration_models() into new function find_configured_model().
This removes confusing logic from overloaded implementation of
parse_configuration_models().
---
mesh/prov-db.c | 54 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/mesh/prov-db.c b/mesh/prov-db.c
index 04803a5c8..8a7b47f6e 100644
--- a/mesh/prov-db.c
+++ b/mesh/prov-db.c
@@ -371,14 +371,49 @@ static bool parse_bindings(struct mesh_node *node, int ele_idx,
return true;
}

-static bool parse_configuration_models(struct mesh_node *node, int ele_idx,
- json_object *jmodels, uint32_t target_id, json_object **jtarget)
+static json_object* find_configured_model(struct mesh_node *node, int ele_idx,
+ json_object *jmodels, uint32_t target_id)
{
int model_cnt;
int i;

- if (jtarget)
- *jtarget = NULL;
+ model_cnt = json_object_array_length(jmodels);
+
+ for (i = 0; i < model_cnt; ++i) {
+ json_object *jmodel;
+ json_object *jvalue;
+ char *str;
+ int len;
+ uint32_t model_id;
+
+ jmodel = json_object_array_get_idx(jmodels, i);
+
+ json_object_object_get_ex(jmodel, "modelId", &jvalue);
+ str = (char *)json_object_get_string(jvalue);
+
+ len = strlen(str);
+
+ if (len != 4 && len != 8)
+ return NULL;
+
+ if (sscanf(str, "%08x", &model_id) != 1)
+ return NULL;
+
+ if (len == 4)
+ model_id += 0xffff0000;
+
+ if (model_id == target_id)
+ return jmodel;
+ }
+
+ return NULL;
+}
+
+static bool parse_configuration_models(struct mesh_node *node, int ele_idx,
+ json_object *jmodels)
+{
+ int model_cnt;
+ int i;

model_cnt = json_object_array_length(jmodels);

@@ -405,11 +440,6 @@ static bool parse_configuration_models(struct mesh_node *node, int ele_idx,
if (len == 4)
model_id += 0xffff0000;

- if (jtarget && model_id == target_id) {
- *jtarget = jmodel;
- return true;
- }
-
json_object_object_get_ex(jmodel, "bind", &jarray);
if (jarray && !parse_bindings(node, ele_idx, model_id, jarray))
return false;
@@ -468,7 +498,7 @@ static bool parse_configuration_elements(struct mesh_node *node,
if (!jmodels)
continue;

- if(!parse_configuration_models(node, index, jmodels, 0, NULL))
+ if(!parse_configuration_models(node, index, jmodels))
return false;
}
return true;
@@ -1011,8 +1041,8 @@ static json_object *get_jmodel_obj(struct mesh_node *node, uint8_t ele_idx,
jmodels = json_object_new_array();
json_object_object_add(jelement, "models", jmodels);
} else {
- parse_configuration_models(node, ele_idx, jmodels,
- model_id, &jmodel);
+ jmodel = find_configured_model(node, ele_idx, jmodels,
+ model_id);
}

if (!jmodel) {
--
2.13.6

2017-12-19 16:52:16

by Hedberg, Johan

Subject: Re: [PATCH BlueZ] mesh:meshctl: make model parsing more manageable

Hi Inga,

On Mon, Dec 18, 2017, Inga Stotland wrote:
> Extract functionality for finding an existing model from
> parse_configuration_models() into new function find_configured_model().
> This removes confusing logic from overloaded implementation of
> parse_configuration_models().
> ---
> mesh/prov-db.c | 54 ++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 12 deletions(-)

Applied. Thanks.

Johan

2017-12-19 11:46:34

by Luiz Augusto von Dentz

Subject: Re: [PATCH BlueZ] mesh:meshctl: make model parsing more manageable

Hi Inga,

On Mon, Dec 18, 2017 at 6:35 PM, Inga Stotland <[email protected]> wrote:
> Extract functionality for finding an existing model from
> parse_configuration_models() into new function find_configured_model().
> This removes confusing logic from overloaded implementation of
> parse_configuration_models().
> ---
> mesh/prov-db.c | 54 ++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/mesh/prov-db.c b/mesh/prov-db.c
> index 04803a5c8..8a7b47f6e 100644
> --- a/mesh/prov-db.c
> +++ b/mesh/prov-db.c
> @@ -371,14 +371,49 @@ static bool parse_bindings(struct mesh_node *node, int ele_idx,
> return true;
> }
>
> -static bool parse_configuration_models(struct mesh_node *node, int ele_idx,
> - json_object *jmodels, uint32_t target_id, json_object **jtarget)
> +static json_object* find_configured_model(struct mesh_node *node, int ele_idx,
> + json_object *jmodels, uint32_t target_id)
> {
> int model_cnt;
> int i;
>
> - if (jtarget)
> - *jtarget = NULL;
> + model_cnt = json_object_array_length(jmodels);
> +
> + for (i = 0; i < model_cnt; ++i) {
> + json_object *jmodel;
> + json_object *jvalue;
> + char *str;
> + int len;
> + uint32_t model_id;
> +
> + jmodel = json_object_array_get_idx(jmodels, i);
> +
> + json_object_object_get_ex(jmodel, "modelId", &jvalue);
> + str = (char *)json_object_get_string(jvalue);
> +
> + len = strlen(str);
> +
> + if (len != 4 && len != 8)
> + return NULL;
> +
> + if (sscanf(str, "%08x", &model_id) != 1)
> + return NULL;
> +
> + if (len == 4)
> + model_id += 0xffff0000;
> +
> + if (model_id == target_id)
> + return jmodel;
> + }
> +
> + return NULL;
> +}
> +
> +static bool parse_configuration_models(struct mesh_node *node, int ele_idx,
> + json_object *jmodels)
> +{
> + int model_cnt;
> + int i;
>
> model_cnt = json_object_array_length(jmodels);
>
> @@ -405,11 +440,6 @@ static bool parse_configuration_models(struct mesh_node *node, int ele_idx,
> if (len == 4)
> model_id += 0xffff0000;
>
> - if (jtarget && model_id == target_id) {
> - *jtarget = jmodel;
> - return true;
> - }
> -
> json_object_object_get_ex(jmodel, "bind", &jarray);
> if (jarray && !parse_bindings(node, ele_idx, model_id, jarray))
> return false;
> @@ -468,7 +498,7 @@ static bool parse_configuration_elements(struct mesh_node *node,
> if (!jmodels)
> continue;
>
> - if(!parse_configuration_models(node, index, jmodels, 0, NULL))
> + if(!parse_configuration_models(node, index, jmodels))
> return false;
> }
> return true;
> @@ -1011,8 +1041,8 @@ static json_object *get_jmodel_obj(struct mesh_node *node, uint8_t ele_idx,
> jmodels = json_object_new_array();
> json_object_object_add(jelement, "models", jmodels);
> } else {
> - parse_configuration_models(node, ele_idx, jmodels,
> - model_id, &jmodel);
> + jmodel = find_configured_model(node, ele_idx, jmodels,
> + model_id);
> }
>
> if (!jmodel) {
> --
> 2.13.6

While this looks fine I would suggest we start putting these pieces in
a shared library if we really intend these to be reused.

--
Luiz Augusto von Dentz

2017-12-19 08:41:50

by Steve Brown

Subject: Re: [PATCH BlueZ] mesh:meshctl: make model parsing more manageable

Hi Inga,

On Mon, 2017-12-18 at 12:35 -0800, Inga Stotland wrote:
> Extract functionality for finding an existing model from
> parse_configuration_models() into new function
> find_configured_model().
> This removes confusing logic from overloaded implementation of
> parse_configuration_models().
> ---
> mesh/prov-db.c | 54 ++++++++++++++++++++++++++++++++++++++++++----
> --------
> 1 file changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/mesh/prov-db.c b/mesh/prov-db.c
> index 04803a5c8..8a7b47f6e 100644
> --- a/mesh/prov-db.c
> +++ b/mesh/prov-db.c
> @@ -371,14 +371,49 @@ static bool parse_bindings(struct mesh_node
> *node, int ele_idx,
> return true;
> }
>
> -static bool parse_configuration_models(struct mesh_node *node, int
> ele_idx,
> - json_object *jmodels, uint32_t target_id,
> json_object **jtarget)
> +static json_object* find_configured_model(struct mesh_node *node,
> int ele_idx,
> + json_object *jmodels,
> uint32_t target_id)
> {
> int model_cnt;
> int i;
>
> - if (jtarget)
> - *jtarget = NULL;
> + model_cnt = json_object_array_length(jmodels);
> +
> + for (i = 0; i < model_cnt; ++i) {
> + json_object *jmodel;
> + json_object *jvalue;
> + char *str;
> + int len;
> + uint32_t model_id;
> +
> + jmodel = json_object_array_get_idx(jmodels, i);
> +
> + json_object_object_get_ex(jmodel, "modelId",
> &jvalue);
> + str = (char *)json_object_get_string(jvalue);
> +
> + len = strlen(str);
> +
> + if (len != 4 && len != 8)
> + return NULL;
> +
> + if (sscanf(str, "%08x", &model_id) != 1)
> + return NULL;
> +
> + if (len == 4)
> + model_id += 0xffff0000;
> +
> + if (model_id == target_id)
> + return jmodel;
> + }
> +
> + return NULL;
> +}
> +
> +static bool parse_configuration_models(struct mesh_node *node, int
> ele_idx,
> + json_object
> *jmodels)
> +{
> + int model_cnt;
> + int i;
>
> model_cnt = json_object_array_length(jmodels);
>
> @@ -405,11 +440,6 @@ static bool parse_configuration_models(struct
> mesh_node *node, int ele_idx,
> if (len == 4)
> model_id += 0xffff0000;
>
> - if (jtarget && model_id == target_id) {
> - *jtarget = jmodel;
> - return true;
> - }
> -
> json_object_object_get_ex(jmodel, "bind", &jarray);
> if (jarray && !parse_bindings(node, ele_idx,
> model_id, jarray))
> return false;
> @@ -468,7 +498,7 @@ static bool parse_configuration_elements(struct
> mesh_node *node,
> if (!jmodels)
> continue;
>
> - if(!parse_configuration_models(node, index, jmodels,
> 0, NULL))
> + if(!parse_configuration_models(node, index,
> jmodels))
> return false;
> }
> return true;
> @@ -1011,8 +1041,8 @@ static json_object *get_jmodel_obj(struct
> mesh_node *node, uint8_t ele_idx,
> jmodels = json_object_new_array();
> json_object_object_add(jelement, "models", jmodels);
> } else {
> - parse_configuration_models(node, ele_idx, jmodels,
> - model_id,
> &jmodel);
> + jmodel = find_configured_model(node, ele_idx,
> jmodels,
> + mode
> l_id);
> }
>
> if (!jmodel) {

This patch also solves the problem of duplicate modelId objects being
inserted in the json database for other than the primary element of a
multi-element node.

Steve