Hi Paul,
On Thu, 2023-03-16 at 21:53 +0100, Paul Menzel wrote:
> Dear Inga,
>
>
> Thank you for your patch.
>
> Am 16.03.23 um 19:07 schrieb Inga Stotland:
> > This fixes the situation when subsequent requests to get a node
> > composition result in appending element's model list with duplicate
> > models.
> > This adds a check for a presence of a model on a element when
> > attempting
>
> a*n* element
OK
>
> > to add a new model ID to a model list on this element.
>
> How can this issue be reproduced?
The simplest way to reproduce the issue is to call composition-get on
the local node (i.e., target address 0001) and then issue command list-
nodes. Without the fix, the model list grows with each subsequent call
to composition-get.
>
> > ---
> > tools/mesh/mesh-db.c | 17 ++++++++++++-----
> > tools/mesh/remote.c | 12 ++++++++++++
> > 2 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/mesh/mesh-db.c b/tools/mesh/mesh-db.c
> > index c0c05a29a..1d047691d 100644
> > --- a/tools/mesh/mesh-db.c
> > +++ b/tools/mesh/mesh-db.c
> > @@ -1964,28 +1964,35 @@ bool mesh_db_node_set_composition(uint16_t
> > unicast, uint8_t *data, uint16_t len)
> >
> > while (len >= 2 && m--) {
> > mod_id = l_get_le16(data);
> > + data += 2;
> > + len -= 2;
> > +
> > + jobj = get_model(unicast, unicast + i,
> > mod_id, false);
> > + if (jobj)
> > + continue;
> >
> > jobj = init_model(mod_id);
> > if (!jobj)
> > goto fail;
> >
> > json_object_array_add(jmods, jobj);
> > - data += 2;
> > - len -= 2;
> > }
> >
> > while (len >= 4 && v--) {
> > mod_id = l_get_le16(data + 2);
> > mod_id = l_get_le16(data) << 16 | mod_id;
> > + data += 4;
> > + len -= 4;
> > +
> > + jobj = get_model(unicast, unicast + i,
> > mod_id, true);
> > + if (jobj)
> > + continue;
> >
> > jobj = init_vendor_model(mod_id);
> > if (!jobj)
> > goto fail;
> >
> > json_object_array_add(jmods, jobj);
> > -
> > - data += 4;
> > - len -= 4;
> > }
> >
> > i++;
> > diff --git a/tools/mesh/remote.c b/tools/mesh/remote.c
> > index cee711dec..ec1476ac9 100644
> > --- a/tools/mesh/remote.c
> > +++ b/tools/mesh/remote.c
> > @@ -54,6 +54,14 @@ struct rejected_addr {
> > static struct l_queue *nodes;
> > static struct l_queue *reject_list;
> >
> > +static bool match_mod_id(const void *a, const void *b)
> > +{
> > + uint32_t id1 = L_PTR_TO_UINT(a);
> > + uint32_t id2 = L_PTR_TO_UINT(b);
>
> Why not `unsigned int`?
Not sure what exactly you're asking here. Model ID is 32 bit long.
Anyway, will change to straight comparison without potential type
conversion.
>
> > +
> > + return id1 == id2;
> > +}
> > +
> > static int compare_mod_id(const void *a, const void *b, void
> > *user_data)
> > {
> > uint32_t id1 = L_PTR_TO_UINT(a);
> > @@ -227,6 +235,10 @@ bool remote_set_model(uint16_t unicast,
> > uint8_t ele_idx, uint32_t mod_id,
> > if (!vendor)
> > mod_id = VENDOR_ID_MASK | mod_id;
> >
> > + if (l_queue_find(rmt->els[ele_idx], match_mod_id,
> > + L_UINT_TO_P
> > TR(mod_id)))
> > + return true;
> > +
> > l_queue_insert(rmt->els[ele_idx], L_UINT_TO_PTR(mod_id),
> > compare_mod
> > _id, NULL);
> >
>
>
> Kind regards,
>
> Paul
Best regards,
Inga
