Re: [PATCH BlueZ] mesh: Fix clean up of node's DBUS assosiated resources

["Gix, Brian" <brian.gix@intel.com>]

Ap[plied

On Wed, 2019-07-03 at 16:02 -0700, Inga Stotland wrote:
> This consolidates multiple places where node's object paths,
> interfaces, etc. are de-allocated, into one routine:
> free_node_dbus_resources().
> This also addresses memory leaks assosiated with inconsistent freeing
> of object path strings.
> ---
>  mesh/node.c | 66 +++++++++++++++++++++++------------------------------
>  1 file changed, 29 insertions(+), 37 deletions(-)
> 
> diff --git a/mesh/node.c b/mesh/node.c
> index adc2aa93e..67e0dd014 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -226,34 +226,46 @@ static void element_free(void *data)
>  	l_free(element);
>  }
>  
> -static void free_node_resources(void *data)
> +static void free_node_dbus_resources(struct mesh_node *node)
>  {
> -	struct mesh_node *node = data;
> +	if (!node)
> +		return;
>  
> -	/* Unregister io callbacks */
> -	if (node->net)
> -		mesh_net_detach(node->net);
> -	mesh_net_free(node->net);
> +	if (node->disc_watch) {
> +		l_dbus_remove_watch(dbus_get_bus(), node->disc_watch);
> +		node->disc_watch = 0;
> +	}
>  
> -	l_queue_destroy(node->elements, element_free);
> -	l_free(node->comp);
> -	l_free(node->app_path);
> +	l_queue_foreach(node->elements, free_element_path, NULL);
>  	l_free(node->owner);
> -	l_free(node->node_path);
> -
> -	if (node->disc_watch)
> -		l_dbus_remove_watch(dbus_get_bus(), node->disc_watch);
> +	node->owner = NULL;
> +	l_free(node->app_path);
> +	node->app_path = NULL;
>  
>  	if (node->path) {
>  		l_dbus_object_remove_interface(dbus_get_bus(), node->path,
>  							MESH_NODE_INTERFACE);
>  
>  		l_dbus_object_remove_interface(dbus_get_bus(), node->path,
> -					       MESH_MANAGEMENT_INTERFACE);
> +						MESH_MANAGEMENT_INTERFACE);
> +		l_free(node->path);
> +		node->path = NULL;
>  	}
> +}
>  
> -	l_free(node->path);
> +static void free_node_resources(void *data)
> +{
> +	struct mesh_node *node = data;
>  
> +	/* Unregister io callbacks */
> +	if (node->net)
> +		mesh_net_detach(node->net);
> +	mesh_net_free(node->net);
> +
> +	l_queue_destroy(node->elements, element_free);
> +	l_free(node->comp);
> +
> +	free_node_dbus_resources(node);
>  	l_free(node);
>  }
>  
> @@ -1033,24 +1045,9 @@ static void app_disc_cb(struct l_dbus *bus, void *user_data)
>  	l_info("App %s disconnected (%u)", node->owner, node->disc_watch);
>  
>  	node->disc_watch = 0;
> -
> -	l_queue_foreach(node->elements, free_element_path, NULL);
> -
> -	l_free(node->owner);
> -	node->owner = NULL;
> -
> -	if (node->path) {
> -		l_dbus_object_remove_interface(dbus_get_bus(), node->path,
> -							MESH_NODE_INTERFACE);
> -
> -		l_dbus_object_remove_interface(dbus_get_bus(), node->path,
> -						MESH_MANAGEMENT_INTERFACE);
> -		l_free(node->app_path);
> -		node->app_path = NULL;
> -	}
> +	free_node_dbus_resources(node);
>  }
>  
> -
>  static bool validate_model_property(struct node_element *ele,
>  					struct l_dbus_message_iter *property,
>  					uint8_t *num_models, bool vendor)
> @@ -1611,14 +1608,9 @@ fail:
>  		/* Handle failed Attach request */
>  		node_ready_func_t cb = req->cb;
>  
> -		l_queue_foreach(node->elements, free_element_path, NULL);
> -		l_free(node->app_path);
> -		node->app_path = NULL;
> +		free_node_dbus_resources(node);
>  
> -		l_free(node->owner);
> -		node->owner = NULL;
>  		cb(req->user_data, MESH_ERROR_FAILED, node);
> -
>  	} else {
>  		/* Handle failed Join and Create requests */
>  		if (node)