linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 00/10] Mesh code clean up
@ 2020-05-22  0:34 Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 01/10] mesh: Remove unused structure member Inga Stotland
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This patchset is contains a number of memory leak fixes and
some stylistic changes that hopefully result in more compact
and readable code.

Inga Stotland (10):
  mesh: Remove unused structure member
  mesh: Free allocated agent in mesh_remove_agent()
  mesh: Remove agent when freeing node's dynamic resources
  mesh: Add finalization of a newly created node
  mesh: Remove unused function prototypes from node.h
  mesh: Create a queue of pending requests in mesh_init()
  mesh: Clean up Import() method call
  mesh: Clean up Attach() method call
  mesh: Fix memory leak in Create, Import & Attach methods
  mesh: Clean up Join() method

 mesh/agent.c |  10 ++--
 mesh/mesh.c  | 136 ++++++++++++++++++---------------------------------
 mesh/node.c  |  57 ++++++++++++---------
 mesh/node.h  |   7 ++-
 4 files changed, 88 insertions(+), 122 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 01/10] mesh: Remove unused structure member
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 02/10] mesh: Free allocated agent in mesh_remove_agent() Inga Stotland
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This removes unused "agent" member from join_data structure.
---
 mesh/mesh.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index 5fb9eedcf..14ac543e2 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -72,7 +72,6 @@ struct bt_mesh {
 
 struct join_data{
 	struct l_dbus_message *msg;
-	struct mesh_agent *agent;
 	char *sender;
 	struct mesh_node *node;
 	uint32_t disc_watch;
@@ -319,8 +318,6 @@ static void free_pending_join_call(bool failed)
 		l_dbus_remove_watch(dbus_get_bus(),
 						join_pending->disc_watch);
 
-	mesh_agent_remove(join_pending->agent);
-
 	if (failed)
 		node_remove(join_pending->node);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 02/10] mesh: Free allocated agent in mesh_remove_agent()
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 01/10] mesh: Remove unused structure member Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 03/10] mesh: Remove agent when freeing node's dynamic resources Inga Stotland
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This adds previously missing call to free memory allocated
for agent structure.
---
 mesh/agent.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mesh/agent.c b/mesh/agent.c
index a06cc2b99..4d200416f 100644
--- a/mesh/agent.c
+++ b/mesh/agent.c
@@ -188,9 +188,6 @@ static void agent_free(void *agent_data)
 	mesh_agent_key_cb_t key_cb;
 	mesh_agent_number_cb_t number_cb;
 
-	if (!l_queue_find(agents, simple_match, agent))
-		return;
-
 	err = MESH_ERROR_DOES_NOT_EXIST;
 
 	if (agent->req && agent->req->cb) {
@@ -228,15 +225,16 @@ static void agent_free(void *agent_data)
 
 	l_free(agent->path);
 	l_free(agent->owner);
+	l_free(agent);
 }
 
 void mesh_agent_remove(struct mesh_agent *agent)
 {
-	if (!agent || !l_queue_find(agents, simple_match, agent))
+	if (!agent)
 		return;
 
-	agent_free(agent);
-	l_queue_remove(agents, agent);
+	if (l_queue_remove(agents, agent))
+		agent_free(agent);
 }
 
 void mesh_agent_cleanup(void)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 03/10] mesh: Remove agent when freeing node's dynamic resources
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 01/10] mesh: Remove unused structure member Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 02/10] mesh: Free allocated agent in mesh_remove_agent() Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 04/10] mesh: Add finalization of a newly created node Inga Stotland
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This adds clean up of node's agent instance when node's dynamic
resources are freed.
---
 mesh/node.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mesh/node.c b/mesh/node.c
index d5e07ce7f..8ad77639e 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -333,6 +333,7 @@ static void free_node_resources(void *data)
 	/* Free dynamic resources */
 	free_node_dbus_resources(node);
 	l_queue_destroy(node->elements, element_free);
+	mesh_agent_remove(node->agent);
 	mesh_config_release(node->cfg);
 	mesh_net_free(node->net);
 	l_free(node->storage_dir);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 04/10] mesh: Add finalization of a newly created node
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (2 preceding siblings ...)
  2020-05-22  0:34 ` [PATCH BlueZ 03/10] mesh: Remove agent when freeing node's dynamic resources Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 05/10] mesh: Remove unused function prototypes from node.h Inga Stotland
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

When a new node is created as a result of successful completion
of either Join() or Create() or Import() methods and has been
confirmed via successful token delivery to the application,
clean up node's D-Bus resources (application path, element paths, etc)
that have been gathered during the initial GetMAnagedObjects() call.
Also, remove the agent instance associaed with the new node.

These resources will be re-populated after the Attach() call
verifies the node's integrity.
---
 mesh/mesh.c |  4 ++--
 mesh/node.c | 39 +++++++++++++++++++++++++--------------
 mesh/node.h |  1 +
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index 14ac543e2..8bd7b3978 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -449,7 +449,7 @@ static void prov_join_complete_reply_cb(struct l_dbus_message *message,
 		failed = true;
 
 	if (!failed)
-		node_attach_io(join_pending->node, mesh.io);
+		node_finalize_new_node(join_pending->node, mesh.io);
 
 	free_pending_join_call(failed);
 }
@@ -693,7 +693,7 @@ static void create_join_complete_reply_cb(struct l_dbus_message *message,
 		return;
 	}
 
-	node_attach_io(node, mesh.io);
+	node_finalize_new_node(node, mesh.io);
 }
 
 static void create_node_ready_cb(void *user_data, int status,
diff --git a/mesh/node.c b/mesh/node.c
index 8ad77639e..8cfe1ddc8 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -992,12 +992,6 @@ static void attach_io(void *a, void *b)
 		mesh_net_attach(node->net, io);
 }
 
-/* Register callback for the node's io */
-void node_attach_io(struct mesh_node *node, struct mesh_io *io)
-{
-	attach_io(node, io);
-}
-
 /* Register callbacks for all nodes io */
 void node_attach_io_all(struct mesh_io *io)
 {
@@ -1467,7 +1461,6 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	const char *path;
 	struct mesh_node *node = req->node;
 	struct node_import *import;
-	void *agent = NULL;
 	bool have_app = false;
 	unsigned int num_ele;
 	struct keyring_net_key net_key;
@@ -1515,13 +1508,11 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 				const char *sender;
 
 				sender = l_dbus_message_get_sender(msg);
-				agent = mesh_agent_create(path, sender,
+				node->agent = mesh_agent_create(path, sender,
 								&properties);
-				if (!agent)
+				if (!node->agent)
 					goto fail;
 
-				node->agent = agent;
-
 			} else if (!strcmp(MESH_PROVISIONER_INTERFACE,
 								interface)) {
 				node->provisioner = true;
@@ -1629,9 +1620,6 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	}
 
 fail:
-	if (agent)
-		mesh_agent_remove(agent);
-
 	/* Handle failed requests */
 	if (node)
 		node_remove(node);
@@ -2348,3 +2336,26 @@ bool node_load_from_storage(const char *storage_dir)
 {
 	return mesh_config_load_nodes(storage_dir, init_from_storage, NULL);
 }
+
+/*
+ * This is called for a new node that:
+ *         - has been created as a result of successful completion of Join()
+ *           or Create() or Import() methods
+ *     and
+ *         - has been confirmed via successful token delivery to the application
+ *
+ * After a node has been created, the information gathered during initial
+ * GetManagedObjects() call is cleared. The subsequent call to Attach() would
+ * verify node's integrity and re-initialize node's D-Bus resources.
+ */
+void node_finalize_new_node(struct mesh_node *node, struct mesh_io *io)
+{
+	if (!node)
+		return;
+
+	free_node_dbus_resources(node);
+	mesh_agent_remove(node->agent);
+
+	/* Register callback for the node's io */
+	attach_io(node, io);
+}
diff --git a/mesh/node.h b/mesh/node.h
index 38aea138f..3019d316b 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -100,3 +100,4 @@ struct mesh_config *node_config_get(struct mesh_node *node);
 struct mesh_agent *node_get_agent(struct mesh_node *node);
 const char *node_get_storage_dir(struct mesh_node *node);
 bool node_load_from_storage(const char *storage_dir);
+void node_finalize_new_node(struct mesh_node *node, struct mesh_io *io);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 05/10] mesh: Remove unused function prototypes from node.h
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (3 preceding siblings ...)
  2020-05-22  0:34 ` [PATCH BlueZ 04/10] mesh: Add finalization of a newly created node Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  1:12   ` [BlueZ,05/10] " bluez.test.bot
  2020-05-22  0:34 ` [PATCH BlueZ 06/10] mesh: Create a queue of pending requests in mesh_init() Inga Stotland
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

---
 mesh/node.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mesh/node.h b/mesh/node.h
index 3019d316b..076714e66 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -92,8 +92,6 @@ bool node_import(const char *app_root, const char *sender, const uint8_t *uuid,
 			uint16_t net_idx, bool kr, bool ivu,
 			uint32_t iv_index, uint16_t unicast,
 			node_ready_func_t cb, void *user_data);
-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);
 struct mesh_config *node_config_get(struct mesh_node *node);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 06/10] mesh: Create a queue of pending requests in mesh_init()
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (4 preceding siblings ...)
  2020-05-22  0:34 ` [PATCH BlueZ 05/10] mesh: Remove unused function prototypes from node.h Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 07/10] mesh: Clean up Import() method call Inga Stotland
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This removes unnnecessary checking for queue existence every time
either Attach(), Create() or Import() methods are called.
---
 mesh/mesh.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index 8bd7b3978..4a3ba171d 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -297,6 +297,8 @@ bool mesh_init(const char *config_dir, const char *mesh_conf_fname,
 	mesh_io_get_caps(mesh.io, &caps);
 	mesh.max_filters = caps.max_num_filters;
 
+	pending_queue = l_queue_new();
+
 	return true;
 }
 
@@ -652,9 +654,6 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
 	sender = l_dbus_message_get_sender(msg);
 
 	pending_msg = l_dbus_message_ref(msg);
-	if (!pending_queue)
-		pending_queue = l_queue_new();
-
 	l_queue_push_tail(pending_queue, pending_msg);
 
 	status = node_attach(app_path, sender, token, attach_ready_cb,
@@ -760,10 +759,8 @@ static struct l_dbus_message *create_network_call(struct l_dbus *dbus,
 							"Bad device UUID");
 
 	sender = l_dbus_message_get_sender(msg);
-	pending_msg = l_dbus_message_ref(msg);
-	if (!pending_queue)
-		pending_queue = l_queue_new();
 
+	pending_msg = l_dbus_message_ref(msg);
 	l_queue_push_tail(pending_queue, pending_msg);
 
 	node_create(app_path, sender, uuid, create_node_ready_cb,
@@ -851,11 +848,8 @@ static struct l_dbus_message *import_call(struct l_dbus *dbus,
 							"Bad address");
 
 	sender = l_dbus_message_get_sender(msg);
-	pending_msg = l_dbus_message_ref(msg);
-
-	if (!pending_queue)
-		pending_queue = l_queue_new();
 
+	pending_msg = l_dbus_message_ref(msg);
 	l_queue_push_tail(pending_queue, pending_msg);
 
 	if (!node_import(app_path, sender, uuid, dev_key, net_key, net_idx,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 07/10] mesh: Clean up Import() method call
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (5 preceding siblings ...)
  2020-05-22  0:34 ` [PATCH BlueZ 06/10] mesh: Create a queue of pending requests in mesh_init() Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:34 ` [PATCH BlueZ 08/10] mesh: Clean up Attach() " Inga Stotland
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This removes unnecessary failing conditions in Import() call and
simplifies iterations through "flags" dictionary.
---
 mesh/mesh.c | 33 +++++++--------------------------
 mesh/node.c |  3 +--
 mesh/node.h |  2 +-
 3 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index 4a3ba171d..e5b36cd94 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -706,22 +706,19 @@ static void create_node_ready_cb(void *user_data, int status,
 	const char *path;
 	const uint8_t *token;
 
-	pending_msg = l_queue_find(pending_queue, simple_match, user_data);
+	pending_msg = l_queue_remove_if(pending_queue, simple_match, user_data);
 	if (!pending_msg)
 		return;
 
 	if (status != MESH_ERROR_NONE) {
 		reply = dbus_error(pending_msg, status, NULL);
-
 		l_dbus_send(dbus_get_bus(), reply);
-		l_queue_remove(pending_queue, pending_msg);
 		return;
 	}
 
 	reply = l_dbus_message_new_method_return(pending_msg);
 
 	l_dbus_send(dbus, reply);
-	l_queue_remove(pending_queue, pending_msg);
 
 	owner = l_dbus_message_get_sender(pending_msg);
 	path = node_get_app_path(node);
@@ -825,19 +822,13 @@ static struct l_dbus_message *import_call(struct l_dbus *dbus,
 							"Bad net index");
 
 	while (l_dbus_message_iter_next_entry(&iter_flags, &key, &var)) {
-		if (!strcmp(key, "IVUpdate")) {
-			if (!l_dbus_message_iter_get_variant(&var, "b",
-								&ivu))
-				goto fail;
+		if (!strcmp(key, "IVUpdate") &&
+			l_dbus_message_iter_get_variant(&var, "b", &ivu))
 			continue;
-		}
 
-		if (!strcmp(key, "KeyRefresh")) {
-			if (!l_dbus_message_iter_get_variant(&var, "b",
-								&kr))
-				goto fail;
+		if (!strcmp(key, "KeyRefresh") &&
+			l_dbus_message_iter_get_variant(&var, "b", &kr))
 			continue;
-		}
 
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
 							"Bad flags");
@@ -852,20 +843,10 @@ static struct l_dbus_message *import_call(struct l_dbus *dbus,
 	pending_msg = l_dbus_message_ref(msg);
 	l_queue_push_tail(pending_queue, pending_msg);
 
-	if (!node_import(app_path, sender, uuid, dev_key, net_key, net_idx,
-					kr, ivu, iv_index, unicast,
-					create_node_ready_cb, pending_msg))
-		goto fail;
+	node_import(app_path, sender, uuid, dev_key, net_key, net_idx, kr, ivu,
+			iv_index, unicast, create_node_ready_cb, pending_msg);
 
 	return NULL;
-
-fail:
-	if (pending_msg) {
-		l_dbus_message_unref(msg);
-		l_queue_remove(pending_queue, pending_msg);
-	}
-
-	return dbus_error(msg, MESH_ERROR_INVALID_ARGS, "Node import failed");
 }
 
 static void setup_network_interface(struct l_dbus_interface *iface)
diff --git a/mesh/node.c b/mesh/node.c
index 8cfe1ddc8..9ba5ad877 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1693,7 +1693,7 @@ void node_join(const char *app_root, const char *sender, const uint8_t *uuid,
 					req, l_free);
 }
 
-bool node_import(const char *app_root, const char *sender, const uint8_t *uuid,
+void node_import(const char *app_root, const char *sender, const uint8_t *uuid,
 			const uint8_t dev_key[16], const uint8_t net_key[16],
 			uint16_t net_idx, bool kr, bool ivu,
 			uint32_t iv_index, uint16_t unicast,
@@ -1725,7 +1725,6 @@ bool node_import(const char *app_root, const char *sender, const uint8_t *uuid,
 						"GetManagedObjects", NULL,
 						get_managed_objects_cb,
 						req, l_free);
-	return true;
 }
 
 void node_create(const char *app_root, const char *sender, const uint8_t *uuid,
diff --git a/mesh/node.h b/mesh/node.h
index 076714e66..ca5d60b6b 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -87,7 +87,7 @@ void node_build_attach_reply(struct mesh_node *node,
 						struct l_dbus_message *reply);
 void node_create(const char *app_root, const char *sender, const uint8_t *uuid,
 					node_ready_func_t cb, void *user_data);
-bool node_import(const char *app_root, const char *sender, const uint8_t *uuid,
+void node_import(const char *app_root, const char *sender, const uint8_t *uuid,
 			const uint8_t dev_key[16], const uint8_t net_key[16],
 			uint16_t net_idx, bool kr, bool ivu,
 			uint32_t iv_index, uint16_t unicast,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 08/10] mesh: Clean up Attach() method call
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (6 preceding siblings ...)
  2020-05-22  0:34 ` [PATCH BlueZ 07/10] mesh: Clean up Import() method call Inga Stotland
@ 2020-05-22  0:34 ` Inga Stotland
  2020-05-22  0:35 ` [PATCH BlueZ 09/10] mesh: Fix memory leak in Create, Import & Attach methods Inga Stotland
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This consolidates error return form one place: off a callback
with unsuccessful status.
---
 mesh/mesh.c | 29 ++++++++---------------------
 mesh/node.c | 14 +++++++-------
 mesh/node.h |  2 +-
 3 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index e5b36cd94..c68436caa 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -617,24 +617,17 @@ static void attach_ready_cb(void *user_data, int status, struct mesh_node *node)
 	struct l_dbus_message *reply;
 	struct l_dbus_message *pending_msg;
 
-	pending_msg = l_queue_find(pending_queue, simple_match, user_data);
+	pending_msg = l_queue_remove_if(pending_queue, simple_match, user_data);
 	if (!pending_msg)
 		return;
 
-	if (status != MESH_ERROR_NONE) {
-		const char *desc = (status == MESH_ERROR_NOT_FOUND) ?
-				"Node match not found" : "Attach failed";
-		reply = dbus_error(pending_msg, status, desc);
-		goto done;
-	}
-
-	reply = l_dbus_message_new_method_return(pending_msg);
+	if (status == MESH_ERROR_NONE) {
+		reply = l_dbus_message_new_method_return(pending_msg);
+		node_build_attach_reply(node, reply);
+	} else
+		reply = dbus_error(pending_msg, status, "Attach failed");
 
-	node_build_attach_reply(node, reply);
-
-done:
 	l_dbus_send(dbus_get_bus(), reply);
-	l_queue_remove(pending_queue, pending_msg);
 }
 
 static struct l_dbus_message *attach_call(struct l_dbus *dbus,
@@ -644,7 +637,6 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
 	uint64_t token;
 	const char *app_path, *sender;
 	struct l_dbus_message *pending_msg;
-	int status;
 
 	l_debug("Attach");
 
@@ -656,14 +648,9 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
 	pending_msg = l_dbus_message_ref(msg);
 	l_queue_push_tail(pending_queue, pending_msg);
 
-	status = node_attach(app_path, sender, token, attach_ready_cb,
-								pending_msg);
-	if (status == MESH_ERROR_NONE)
-		return NULL;
+	node_attach(app_path, sender, token, attach_ready_cb, pending_msg);
 
-	l_queue_remove(pending_queue, pending_msg);
-
-	return dbus_error(msg, status, NULL);
+	return NULL;
 }
 
 static struct l_dbus_message *leave_call(struct l_dbus *dbus,
diff --git a/mesh/node.c b/mesh/node.c
index 9ba5ad877..dd28dfd77 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1634,20 +1634,23 @@ fail:
 }
 
 /* Establish relationship between application and mesh node */
-int node_attach(const char *app_root, const char *sender, uint64_t token,
+void node_attach(const char *app_root, const char *sender, uint64_t token,
 					node_ready_func_t cb, void *user_data)
 {
 	struct managed_obj_request *req;
 	struct mesh_node *node;
 
 	node = l_queue_find(nodes, match_token, (void *) &token);
-	if (!node)
-		return MESH_ERROR_NOT_FOUND;
+	if (!node) {
+		cb(user_data, MESH_ERROR_NOT_FOUND, NULL);
+		return;
+	}
 
 	/* Check if the node is already in use */
 	if (node->owner) {
 		l_warn("The node is already in use");
-		return MESH_ERROR_ALREADY_EXISTS;
+		cb(user_data, MESH_ERROR_ALREADY_EXISTS, NULL);
+		return;
 	}
 
 	req = l_new(struct managed_obj_request, 1);
@@ -1668,11 +1671,8 @@ int node_attach(const char *app_root, const char *sender, uint64_t token,
 					"GetManagedObjects", NULL,
 					get_managed_objects_cb,
 					req, l_free);
-	return MESH_ERROR_NONE;
-
 }
 
-
 /* Create a temporary pre-provisioned node */
 void node_join(const char *app_root, const char *sender, const uint8_t *uuid,
 						node_join_ready_func_t cb)
diff --git a/mesh/node.h b/mesh/node.h
index ca5d60b6b..290681e28 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -81,7 +81,7 @@ const char *node_get_app_path(struct mesh_node *node);
 bool node_add_pending_local(struct mesh_node *node, void *info);
 void node_attach_io_all(struct mesh_io *io);
 void node_attach_io(struct mesh_node *node, struct mesh_io *io);
-int node_attach(const char *app_root, const char *sender, uint64_t token,
+void node_attach(const char *app_root, const char *sender, uint64_t token,
 					node_ready_func_t cb, void *user_data);
 void node_build_attach_reply(struct mesh_node *node,
 						struct l_dbus_message *reply);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 09/10] mesh: Fix memory leak in Create, Import & Attach methods
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (7 preceding siblings ...)
  2020-05-22  0:34 ` [PATCH BlueZ 08/10] mesh: Clean up Attach() " Inga Stotland
@ 2020-05-22  0:35 ` Inga Stotland
  2020-05-22  0:35 ` [PATCH BlueZ 10/10] mesh: Clean up Join() method Inga Stotland
  2020-05-22 20:54 ` [PATCH BlueZ 00/10] Mesh code clean up Gix, Brian
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This ensures that every time l_dbus_message_ref() is used to preserve
a message for a pending method reply, there is a matching call to
l_dbus_message_unref().
---
 mesh/mesh.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index c68436caa..6f8974745 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -309,6 +309,7 @@ static void pending_request_exit(void *data)
 
 	reply = dbus_error(msg, MESH_ERROR_FAILED, "Failed. Exiting");
 	l_dbus_send(dbus_get_bus(), reply);
+	l_dbus_message_unref(msg);
 }
 
 static void free_pending_join_call(bool failed)
@@ -628,6 +629,7 @@ static void attach_ready_cb(void *user_data, int status, struct mesh_node *node)
 		reply = dbus_error(pending_msg, status, "Attach failed");
 
 	l_dbus_send(dbus_get_bus(), reply);
+	l_dbus_message_unref(pending_msg);
 }
 
 static struct l_dbus_message *attach_call(struct l_dbus *dbus,
@@ -700,6 +702,7 @@ static void create_node_ready_cb(void *user_data, int status,
 	if (status != MESH_ERROR_NONE) {
 		reply = dbus_error(pending_msg, status, NULL);
 		l_dbus_send(dbus_get_bus(), reply);
+		l_dbus_message_unref(pending_msg);
 		return;
 	}
 
@@ -719,6 +722,7 @@ static void create_node_ready_cb(void *user_data, int status,
 	l_dbus_message_set_arguments(msg, "t", l_get_be64(token));
 	dbus_send_with_timeout(dbus, msg, create_join_complete_reply_cb,
 						node, DEFAULT_DBUS_TIMEOUT);
+	l_dbus_message_unref(pending_msg);
 }
 
 static struct l_dbus_message *create_network_call(struct l_dbus *dbus,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH BlueZ 10/10] mesh: Clean up Join() method
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (8 preceding siblings ...)
  2020-05-22  0:35 ` [PATCH BlueZ 09/10] mesh: Fix memory leak in Create, Import & Attach methods Inga Stotland
@ 2020-05-22  0:35 ` Inga Stotland
  2020-05-22 20:54 ` [PATCH BlueZ 00/10] Mesh code clean up Gix, Brian
  10 siblings, 0 replies; 13+ messages in thread
From: Inga Stotland @ 2020-05-22  0:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This consolidates various places where a pending response
to Join() is created and makes sure that l_dus_message_unref()
is called correctly.
---
 mesh/mesh.c | 49 ++++++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index 6f8974745..24ea3afd6 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -341,6 +341,7 @@ void mesh_cleanup(void)
 			reply = dbus_error(join_pending->msg, MESH_ERROR_FAILED,
 							"Failed. Exiting");
 			l_dbus_send(dbus_get_bus(), reply);
+			l_dbus_message_unref(join_pending->msg);
 		}
 
 		acceptor_cancel(&mesh);
@@ -391,11 +392,6 @@ static void prov_disc_cb(struct l_dbus *bus, void *user_data)
 	if (!join_pending)
 		return;
 
-	if (join_pending->msg) {
-		l_dbus_message_unref(join_pending->msg);
-		join_pending->msg = NULL;
-	}
-
 	acceptor_cancel(&mesh);
 	join_pending->disc_watch = 0;
 
@@ -501,39 +497,40 @@ static void node_init_cb(struct mesh_node *node, struct mesh_agent *agent)
 {
 	struct l_dbus_message *reply;
 	uint8_t num_ele;
+	bool is_error = false;
+	struct l_dbus *dbus = dbus_get_bus();
 
 	if (!node) {
 		reply = dbus_error(join_pending->msg, MESH_ERROR_FAILED,
 				"Failed to create node from application");
-		goto fail;
+		is_error = true;
+		goto done;
 	}
 
 	join_pending->node = node;
 	num_ele = node_get_num_elements(node);
 
 	if (!acceptor_start(num_ele, join_pending->uuid, mesh.algorithms,
-				mesh.prov_timeout, agent, prov_complete_cb,
-				&mesh))
-	{
+						mesh.prov_timeout, agent,
+						prov_complete_cb, &mesh)) {
 		reply = dbus_error(join_pending->msg, MESH_ERROR_FAILED,
 				"Failed to start provisioning acceptor");
-		goto fail;
-	}
+		is_error = true;
+	} else
+		reply = l_dbus_message_new_method_return(join_pending->msg);
 
-	reply = l_dbus_message_new_method_return(join_pending->msg);
-	l_dbus_send(dbus_get_bus(), reply);
+done:
+	l_dbus_send(dbus, reply);
+	l_dbus_message_unref(join_pending->msg);
 	join_pending->msg = NULL;
 
-	/* Setup disconnect watch */
-	join_pending->disc_watch = l_dbus_add_disconnect_watch(dbus_get_bus(),
+	if (is_error)
+		free_pending_join_call(true);
+	else
+		/* Setup disconnect watch */
+		join_pending->disc_watch = l_dbus_add_disconnect_watch(dbus,
 						join_pending->sender,
 						prov_disc_cb, NULL, NULL);
-
-	return;
-
-fail:
-	l_dbus_send(dbus_get_bus(), reply);
-	free_pending_join_call(true);
 }
 
 static struct l_dbus_message *join_network_call(struct l_dbus *dbus,
@@ -591,25 +588,23 @@ static struct l_dbus_message *cancel_join_call(struct l_dbus *dbus,
 
 	l_debug("Cancel Join");
 
-	if (!join_pending) {
-		reply = dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST,
+	if (!join_pending)
+		return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST,
 							"No join in progress");
-		goto done;
-	}
-
 	acceptor_cancel(&mesh);
 
 	/* Return error to the original Join call */
 	if (join_pending->msg) {
 		reply = dbus_error(join_pending->msg, MESH_ERROR_FAILED, NULL);
 		l_dbus_send(dbus_get_bus(), reply);
+		l_dbus_message_unref(join_pending->msg);
 	}
 
 	reply = l_dbus_message_new_method_return(msg);
 	l_dbus_message_set_arguments(reply, "");
 
 	free_pending_join_call(true);
-done:
+
 	return reply;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [BlueZ,05/10] mesh: Remove unused function prototypes from node.h
  2020-05-22  0:34 ` [PATCH BlueZ 05/10] mesh: Remove unused function prototypes from node.h Inga Stotland
@ 2020-05-22  1:12   ` bluez.test.bot
  0 siblings, 0 replies; 13+ messages in thread
From: bluez.test.bot @ 2020-05-22  1:12 UTC (permalink / raw)
  To: linux-bluetooth, inga.stotland

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]


This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
checkgitlint Failed

Outputs:
3: B6 Body message is missing



---
Regards,
Linux Bluetooth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH BlueZ 00/10] Mesh code clean up
  2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
                   ` (9 preceding siblings ...)
  2020-05-22  0:35 ` [PATCH BlueZ 10/10] mesh: Clean up Join() method Inga Stotland
@ 2020-05-22 20:54 ` Gix, Brian
  10 siblings, 0 replies; 13+ messages in thread
From: Gix, Brian @ 2020-05-22 20:54 UTC (permalink / raw)
  To: linux-bluetooth, Stotland, Inga

Applied Patchset

On Thu, 2020-05-21 at 17:34 -0700, Inga Stotland wrote:
> This patchset is contains a number of memory leak fixes and
> some stylistic changes that hopefully result in more compact
> and readable code.
> 
> Inga Stotland (10):
>   mesh: Remove unused structure member
>   mesh: Free allocated agent in mesh_remove_agent()
>   mesh: Remove agent when freeing node's dynamic resources
>   mesh: Add finalization of a newly created node
>   mesh: Remove unused function prototypes from node.h
>   mesh: Create a queue of pending requests in mesh_init()
>   mesh: Clean up Import() method call
>   mesh: Clean up Attach() method call
>   mesh: Fix memory leak in Create, Import & Attach methods
>   mesh: Clean up Join() method
> 
>  mesh/agent.c |  10 ++--
>  mesh/mesh.c  | 136 ++++++++++++++++++---------------------------------
>  mesh/node.c  |  57 ++++++++++++---------
>  mesh/node.h  |   7 ++-
>  4 files changed, 88 insertions(+), 122 deletions(-)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-05-22 20:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22  0:34 [PATCH BlueZ 00/10] Mesh code clean up Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 01/10] mesh: Remove unused structure member Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 02/10] mesh: Free allocated agent in mesh_remove_agent() Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 03/10] mesh: Remove agent when freeing node's dynamic resources Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 04/10] mesh: Add finalization of a newly created node Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 05/10] mesh: Remove unused function prototypes from node.h Inga Stotland
2020-05-22  1:12   ` [BlueZ,05/10] " bluez.test.bot
2020-05-22  0:34 ` [PATCH BlueZ 06/10] mesh: Create a queue of pending requests in mesh_init() Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 07/10] mesh: Clean up Import() method call Inga Stotland
2020-05-22  0:34 ` [PATCH BlueZ 08/10] mesh: Clean up Attach() " Inga Stotland
2020-05-22  0:35 ` [PATCH BlueZ 09/10] mesh: Fix memory leak in Create, Import & Attach methods Inga Stotland
2020-05-22  0:35 ` [PATCH BlueZ 10/10] mesh: Clean up Join() method Inga Stotland
2020-05-22 20:54 ` [PATCH BlueZ 00/10] Mesh code clean up Gix, Brian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).