2019-12-10 23:06:51

by Stotland, Inga

Subject: [PATCH BlueZ] mesh: Remove redundant code in mesh IO mgmt

This removes unnecessary housekeeping for hci controllers
---
mesh/mesh-mgmt.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/mesh/mesh-mgmt.c b/mesh/mesh-mgmt.c
index 27272d4d2..2cf2ebac2 100644
--- a/mesh/mesh-mgmt.c
+++ b/mesh/mesh-mgmt.c
@@ -42,14 +42,8 @@ struct read_info_req {
};

static struct mgmt *mgmt_mesh;
-static struct l_queue *controllers;
static struct l_queue *read_info_regs;

-static bool simple_match(const void *a, const void *b)
-{
- return a == b;
-}
-
static void process_read_info_req(void *data, void *user_data)
{
struct read_info_reg *reg = data;
@@ -101,23 +95,14 @@ static void read_info_cb(uint8_t status, uint16_t length,
static void index_added(uint16_t index, uint16_t length, const void *param,
void *user_data)
{
- if (l_queue_find(controllers, simple_match, L_UINT_TO_PTR(index)))
- return;
-
- l_queue_push_tail(controllers, L_UINT_TO_PTR(index));
-
- if (mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
- read_info_cb, L_UINT_TO_PTR(index), NULL) != 0)
- return;
-
- l_queue_remove(controllers, L_UINT_TO_PTR(index));
+ mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
+ read_info_cb, L_UINT_TO_PTR(index), NULL);
}

static void index_removed(uint16_t index, uint16_t length, const void *param,
void *user_data)
{
l_warn("Hci dev %4.4x removed", index);
- l_queue_remove(controllers, L_UINT_TO_PTR(index));
}

static void read_index_list_cb(uint8_t status, uint16_t length,
@@ -157,9 +142,6 @@ static void read_index_list_cb(uint8_t status, uint16_t length,

static bool mesh_mgmt_init(void)
{
- if (!controllers)
- controllers = l_queue_new();
-
if (!read_info_regs)
read_info_regs = l_queue_new();

--
2.21.0

2019-12-11 19:20:17

by Gix, Brian

Subject: Re: [PATCH BlueZ] mesh: Remove redundant code in mesh IO mgmt

Hi Inga,

On Tue, 2019-12-10 at 15:06 -0800, Inga Stotland wrote:
> This removes unnecessary housekeeping for hci controllers
> ---
> mesh/mesh-mgmt.c | 22 ++--------------------
> 1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/mesh/mesh-mgmt.c b/mesh/mesh-mgmt.c
> index 27272d4d2..2cf2ebac2 100644
> --- a/mesh/mesh-mgmt.c
> +++ b/mesh/mesh-mgmt.c
> @@ -42,14 +42,8 @@ struct read_info_req {
> };
>
> static struct mgmt *mgmt_mesh;
> -static struct l_queue *controllers;
> static struct l_queue *read_info_regs;
>
> -static bool simple_match(const void *a, const void *b)
> -{
> - return a == b;
> -}
> -
> static void process_read_info_req(void *data, void *user_data)
> {
> struct read_info_reg *reg = data;
> @@ -101,23 +95,14 @@ static void read_info_cb(uint8_t status, uint16_t length,
> static void index_added(uint16_t index, uint16_t length, const void *param,
> void *user_data)
> {
> - if (l_queue_find(controllers, simple_match, L_UINT_TO_PTR(index)))
> - return;
> -
> - l_queue_push_tail(controllers, L_UINT_TO_PTR(index));

I think before applying this, we should re-work the controller index to be consistently uint16_t, rather than
the awkward mix of int and uint16_t.

> -
> - if (mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
> - read_info_cb, L_UINT_TO_PTR(index), NULL) != 0)
> - return;
> -
> - l_queue_remove(controllers, L_UINT_TO_PTR(index));
> + mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
> + read_info_cb, L_UINT_TO_PTR(index), NULL);
> }
>
> static void index_removed(uint16_t index, uint16_t length, const void *param,
> void *user_data)
> {
> l_warn("Hci dev %4.4x removed", index);
> - l_queue_remove(controllers, L_UINT_TO_PTR(index));
> }
>
> static void read_index_list_cb(uint8_t status, uint16_t length,
> @@ -157,9 +142,6 @@ static void read_index_list_cb(uint8_t status, uint16_t length,
>
> static bool mesh_mgmt_init(void)
> {
> - if (!controllers)
> - controllers = l_queue_new();
> -
> if (!read_info_regs)
> read_info_regs = l_queue_new();
>

2019-12-11 22:56:34

by Stotland, Inga

Subject: Re: [PATCH BlueZ] mesh: Remove redundant code in mesh IO mgmt

Hi Brian,

On Wed, 2019-12-11 at 19:19 +0000, Gix, Brian wrote:
> Hi Inga,
>
> On Tue, 2019-12-10 at 15:06 -0800, Inga Stotland wrote:
> > This removes unnecessary housekeeping for hci controllers
> > ---
> > mesh/mesh-mgmt.c | 22 ++--------------------
> > 1 file changed, 2 insertions(+), 20 deletions(-)
> >
> > diff --git a/mesh/mesh-mgmt.c b/mesh/mesh-mgmt.c
> > index 27272d4d2..2cf2ebac2 100644
> > --- a/mesh/mesh-mgmt.c
> > +++ b/mesh/mesh-mgmt.c
> > @@ -42,14 +42,8 @@ struct read_info_req {
> > };
> >
> > static struct mgmt *mgmt_mesh;
> > -static struct l_queue *controllers;
> > static struct l_queue *read_info_regs;
> >
> > -static bool simple_match(const void *a, const void *b)
> > -{
> > - return a == b;
> > -}
> > -
> > static void process_read_info_req(void *data, void *user_data)
> > {
> > struct read_info_reg *reg = data;
> > @@ -101,23 +95,14 @@ static void read_info_cb(uint8_t status, uint16_t length,
> > static void index_added(uint16_t index, uint16_t length, const void *param,
> > void *user_data)
> > {
> > - if (l_queue_find(controllers, simple_match, L_UINT_TO_PTR(index)))
> > - return;
> > -
> > - l_queue_push_tail(controllers, L_UINT_TO_PTR(index));
>
> I think before applying this, we should re-work the controller index to be consistently uint16_t, rather than
> the awkward mix of int and uint16_t.

I agree that consistency would be good.
However, I disagree that this needs to be a part of this patch: thios
patch removes code that is not needed. Nothing to do with indices.

>
> > -
> > - if (mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
> > - read_info_cb, L_UINT_TO_PTR(index), NULL) != 0)
> > - return;
> > -
> > - l_queue_remove(controllers, L_UINT_TO_PTR(index));
> > + mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
> > + read_info_cb, L_UINT_TO_PTR(index), NULL);
> > }
> >
> > static void index_removed(uint16_t index, uint16_t length, const void *param,
> > void *user_data)
> > {
> > l_warn("Hci dev %4.4x removed", index);
> > - l_queue_remove(controllers, L_UINT_TO_PTR(index));
> > }
> >
> > static void read_index_list_cb(uint8_t status, uint16_t length,
> > @@ -157,9 +142,6 @@ static void read_index_list_cb(uint8_t status, uint16_t length,
> >
> > static bool mesh_mgmt_init(void)
> > {
> > - if (!controllers)
> > - controllers = l_queue_new();
> > -
> > if (!read_info_regs)
> > read_info_regs = l_queue_new();
> >

Best regards,

Inga

2019-12-15 18:00:08

by Gix, Brian

Subject: Re: [PATCH BlueZ] mesh: Remove redundant code in mesh IO mgmt

Applied

On Tue, 2019-12-10 at 15:06 -0800, Inga Stotland wrote:
> This removes unnecessary housekeeping for hci controllers
> ---
> mesh/mesh-mgmt.c | 22 ++--------------------
> 1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/mesh/mesh-mgmt.c b/mesh/mesh-mgmt.c
> index 27272d4d2..2cf2ebac2 100644
> --- a/mesh/mesh-mgmt.c
> +++ b/mesh/mesh-mgmt.c
> @@ -42,14 +42,8 @@ struct read_info_req {
> };
>
> static struct mgmt *mgmt_mesh;
> -static struct l_queue *controllers;
> static struct l_queue *read_info_regs;
>
> -static bool simple_match(const void *a, const void *b)
> -{
> - return a == b;
> -}
> -
> static void process_read_info_req(void *data, void *user_data)
> {
> struct read_info_reg *reg = data;
> @@ -101,23 +95,14 @@ static void read_info_cb(uint8_t status, uint16_t length,
> static void index_added(uint16_t index, uint16_t length, const void *param,
> void *user_data)
> {
> - if (l_queue_find(controllers, simple_match, L_UINT_TO_PTR(index)))
> - return;
> -
> - l_queue_push_tail(controllers, L_UINT_TO_PTR(index));
> -
> - if (mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
> - read_info_cb, L_UINT_TO_PTR(index), NULL) != 0)
> - return;
> -
> - l_queue_remove(controllers, L_UINT_TO_PTR(index));
> + mgmt_send(mgmt_mesh, MGMT_OP_READ_INFO, index, 0, NULL,
> + read_info_cb, L_UINT_TO_PTR(index), NULL);
> }
>
> static void index_removed(uint16_t index, uint16_t length, const void *param,
> void *user_data)
> {
> l_warn("Hci dev %4.4x removed", index);
> - l_queue_remove(controllers, L_UINT_TO_PTR(index));
> }
>
> static void read_index_list_cb(uint8_t status, uint16_t length,
> @@ -157,9 +142,6 @@ static void read_index_list_cb(uint8_t status, uint16_t length,
>
> static bool mesh_mgmt_init(void)
> {
> - if (!controllers)
> - controllers = l_queue_new();
> -
> if (!read_info_regs)
> read_info_regs = l_queue_new();
>