[PATCH BlueZ 1/9] gdbus: Remove connection from g_dbus_remove_watch

[PATCH BlueZ 1/9] gdbus: Remove connection from g_dbus_remove_watch

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The connection is not really needed since the list of listeners is
global not per connection, besides it is more convenient this way as
only the id is needed.
---
 gdbus/gdbus.h | 2 +-
 gdbus/watch.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index a96c97f..065fd6f 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -243,7 +243,7 @@ guint g_dbus_add_signal_watch(DBusConnection *connection,
 				const char *interface, const char *member,
 				GDBusSignalFunction function, void *user_data,
 				GDBusDestroyFunction destroy);
-gboolean g_dbus_remove_watch(DBusConnection *connection, guint tag);
+gboolean g_dbus_remove_watch(guint id);
 void g_dbus_remove_all_watches(DBusConnection *connection);
 
 void g_dbus_pending_property_success(DBusConnection *connection,
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 07feb61..00cedae 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -285,7 +285,7 @@ static void filter_data_free(struct filter_data *data)
 		g_free(l->data);
 
 	g_slist_free(data->callbacks);
-	g_dbus_remove_watch(data->connection, data->name_watch);
+	g_dbus_remove_watch(data->name_watch);
 	g_free(data->name);
 	g_free(data->owner);
 	g_free(data->path);
@@ -752,7 +752,7 @@ guint g_dbus_add_signal_watch(DBusConnection *connection,
 	return cb->id;
 }
 
-gboolean g_dbus_remove_watch(DBusConnection *connection, guint id)
+gboolean g_dbus_remove_watch(guint id)
 {
 	struct filter_data *data;
 	struct filter_callback *cb;
-- 
1.7.11.4

[PATCH BlueZ 2/9] Fix passing D-Bus connection to g_dbus_remove_watch

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

All it needs now is the id
---
 attrib/client.c                    | 4 ++--
 audio/gateway.c                    | 2 +-
 audio/media.c                      | 9 ++++-----
 audio/telephony-maemo6.c           | 2 +-
 audio/telephony-ofono.c            | 4 ++--
 audio/transport.c                  | 2 +-
 plugins/neard.c                    | 2 +-
 plugins/service.c                  | 4 ++--
 profiles/alert/server.c            | 2 +-
 profiles/health/hdp_util.c         | 3 +--
 profiles/heartrate/heartrate.c     | 6 +++---
 profiles/network/connection.c      | 4 ++--
 profiles/network/server.c          | 2 +-
 profiles/thermometer/thermometer.c | 6 +++---
 src/adapter.c                      | 2 +-
 src/agent.c                        | 3 +--
 src/device.c                       | 6 ++----
 src/profile.c                      | 4 ++--
 18 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 30513d1..b3d8b54 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -283,7 +283,7 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
 	DBG("%s watcher %s exited", gatt->path, watcher->name);
 
 	gatt->watchers = g_slist_remove(gatt->watchers, watcher);
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
+	g_dbus_remove_watch(watcher->id);
 	remove_attio(gatt);
 }
 
@@ -489,7 +489,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
 
 	watcher = l->data;
 	gatt->watchers = g_slist_remove(gatt->watchers, watcher);
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
+	g_dbus_remove_watch(watcher->id);
 	remove_attio(gatt);
 
 	return dbus_message_new_method_return(msg);
diff --git a/audio/gateway.c b/audio/gateway.c
index b4d96f0..bf766f9 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -746,7 +746,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn,
 	if (strcmp(gw->agent->path, path) != 0)
 		return btd_error_does_not_exist(msg);
 
-	g_dbus_remove_watch(conn, gw->agent->watch);
+	g_dbus_remove_watch(gw->agent->watch);
 
 	agent_free(gw->agent);
 	gw->agent = NULL;
diff --git a/audio/media.c b/audio/media.c
index c88afc0..5c26035 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -159,7 +159,7 @@ static void media_endpoint_destroy(struct media_endpoint *endpoint)
 	g_slist_free_full(endpoint->transports,
 				(GDestroyNotify) media_transport_destroy);
 
-	g_dbus_remove_watch(btd_get_dbus_connection(), endpoint->watch);
+	g_dbus_remove_watch(endpoint->watch);
 	g_free(endpoint->capabilities);
 	g_free(endpoint->sender);
 	g_free(endpoint->path);
@@ -966,7 +966,6 @@ static void release_player(struct media_player *mp)
 
 static void media_player_free(gpointer data)
 {
-	DBusConnection *conn = btd_get_dbus_connection();
 	struct media_player *mp = data;
 	struct media_adapter *adapter = mp->adapter;
 
@@ -975,9 +974,9 @@ static void media_player_free(gpointer data)
 		release_player(mp);
 	}
 
-	g_dbus_remove_watch(conn, mp->watch);
-	g_dbus_remove_watch(conn, mp->property_watch);
-	g_dbus_remove_watch(conn, mp->track_watch);
+	g_dbus_remove_watch(mp->watch);
+	g_dbus_remove_watch(mp->property_watch);
+	g_dbus_remove_watch(mp->track_watch);
 
 	if (mp->track)
 		g_hash_table_unref(mp->track);
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index d000a2a..ef997ca 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -2167,7 +2167,7 @@ int telephony_init(void)
 
 static void remove_watch(gpointer data)
 {
-	g_dbus_remove_watch(btd_get_dbus_connection(), GPOINTER_TO_UINT(data));
+	g_dbus_remove_watch(GPOINTER_TO_UINT(data));
 }
 
 void telephony_exit(void)
diff --git a/audio/telephony-ofono.c b/audio/telephony-ofono.c
index f962c7e..f87b652 100644
--- a/audio/telephony-ofono.c
+++ b/audio/telephony-ofono.c
@@ -657,7 +657,7 @@ static void call_free(void *data)
 	if (vc->status == CALL_STATUS_INCOMING)
 		telephony_calling_stopped_ind();
 
-	g_dbus_remove_watch(btd_get_dbus_connection(), vc->watch);
+	g_dbus_remove_watch(vc->watch);
 	g_free(vc->obj_path);
 	g_free(vc->number);
 	g_free(vc);
@@ -1603,7 +1603,7 @@ int telephony_init(void)
 
 static void remove_watch(gpointer data)
 {
-	g_dbus_remove_watch(btd_get_dbus_connection(), GPOINTER_TO_UINT(data));
+	g_dbus_remove_watch(GPOINTER_TO_UINT(data));
 }
 
 static void pending_free(void *data)
diff --git a/audio/transport.c b/audio/transport.c
index daafff8..4615280 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -320,7 +320,7 @@ static void media_transport_remove(struct media_transport *transport,
 	transport->owners = g_slist_remove(transport->owners, owner);
 
 	if (owner->watch)
-		g_dbus_remove_watch(btd_get_dbus_connection(), owner->watch);
+		g_dbus_remove_watch(owner->watch);
 
 	media_owner_free(owner);
 
diff --git a/plugins/neard.c b/plugins/neard.c
index 2da5024..45f73fc 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -532,7 +532,7 @@ static void neard_exit(void)
 {
 	DBG("Cleanup neard plugin");
 
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher_id);
+	g_dbus_remove_watch(watcher_id);
 	watcher_id = 0;
 
 	if (agent_registered)
diff --git a/plugins/service.c b/plugins/service.c
index e81452c..cea36e0 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -250,7 +250,7 @@ static int remove_record(const char *sender,
 
 	DBG("listner_id %d", user_record->listener_id);
 
-	g_dbus_remove_watch(conn, user_record->listener_id);
+	g_dbus_remove_watch(user_record->listener_id);
 
 	exit_callback(conn, user_record);
 
@@ -495,7 +495,7 @@ static void path_unregister(void *data)
 
 		next = l->next;
 
-		g_dbus_remove_watch(conn, user_record->listener_id);
+		g_dbus_remove_watch(user_record->listener_id);
 		exit_callback(conn, user_record);
 	}
 
diff --git a/profiles/alert/server.c b/profiles/alert/server.c
index b8ea141..83e4c2a 100644
--- a/profiles/alert/server.c
+++ b/profiles/alert/server.c
@@ -172,7 +172,7 @@ static void alert_data_destroy(gpointer user_data)
 	struct alert_data *alert = user_data;
 
 	if (alert->watcher)
-		g_dbus_remove_watch(btd_get_dbus_connection(), alert->watcher);
+		g_dbus_remove_watch(alert->watcher);
 
 	g_free(alert->srv);
 	g_free(alert->path);
diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 1fd33e5..fcc6f04 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -1184,8 +1184,7 @@ gboolean hdp_get_dcpsm(struct hdp_device *device, hdp_continue_dcpsm_f func,
 static void hdp_free_application(struct hdp_application *app)
 {
 	if (app->dbus_watcher > 0)
-		g_dbus_remove_watch(btd_get_dbus_connection(),
-							app->dbus_watcher);
+		g_dbus_remove_watch(app->dbus_watcher);
 
 	g_free(app->oname);
 	g_free(app->description);
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 94d4b8d..8db20dc 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -209,7 +209,7 @@ static void remove_watcher(gpointer user_data)
 {
 	struct watcher *watcher = user_data;
 
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
+	g_dbus_remove_watch(watcher->id);
 }
 
 static void destroy_heartrate_adapter(gpointer user_data)
@@ -573,7 +573,7 @@ static void watcher_exit_cb(DBusConnection *conn, void *user_data)
 	DBG("heartrate watcher [%s] disconnected", watcher->path);
 
 	hradapter->watchers = g_slist_remove(hradapter->watchers, watcher);
-	g_dbus_remove_watch(conn, watcher->id);
+	g_dbus_remove_watch(watcher->id);
 
 	if (g_slist_length(hradapter->watchers) == 0)
 		g_slist_foreach(hradapter->devices, disable_measurement, 0);
@@ -629,7 +629,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
 		return btd_error_does_not_exist(msg);
 
 	hradapter->watchers = g_slist_remove(hradapter->watchers, watcher);
-	g_dbus_remove_watch(conn, watcher->id);
+	g_dbus_remove_watch(watcher->id);
 
 	if (g_slist_length(hradapter->watchers) == 0)
 		g_slist_foreach(hradapter->devices, disable_measurement, 0);
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 9646e5a..b5b950b 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -129,7 +129,7 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
 	device_remove_disconnect_watch(nc->peer->device, nc->dc_id);
 	nc->dc_id = 0;
 	if (nc->watch) {
-		g_dbus_remove_watch(btd_get_dbus_connection(), nc->watch);
+		g_dbus_remove_watch(nc->watch);
 		nc->watch = 0;
 	}
 
@@ -154,7 +154,7 @@ static void cancel_connection(struct network_conn *nc, const char *err_msg)
 	}
 
 	if (nc->watch) {
-		g_dbus_remove_watch(conn, nc->watch);
+		g_dbus_remove_watch(nc->watch);
 		nc->watch = 0;
 	}
 
diff --git a/profiles/network/server.c b/profiles/network/server.c
index ccf6e69..6095d10 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -674,7 +674,7 @@ static DBusMessage *unregister_server(DBusConnection *conn,
 	if (!reply)
 		return NULL;
 
-	g_dbus_remove_watch(conn, ns->watch_id);
+	g_dbus_remove_watch(ns->watch_id);
 
 	server_disconnect(conn, ns);
 
diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 3506ba7..f0988a4 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -145,7 +145,7 @@ static void remove_watcher(gpointer user_data)
 {
 	struct watcher *watcher = user_data;
 
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
+	g_dbus_remove_watch(watcher->id);
 }
 
 static void destroy_char(gpointer user_data)
@@ -820,7 +820,7 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
 	remove_int_watcher(t, watcher);
 
 	t->fwatchers = g_slist_remove(t->fwatchers, watcher);
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
+	g_dbus_remove_watch(watcher->id);
 
 	if (g_slist_length(t->fwatchers) == 0)
 		disable_final_measurement(t);
@@ -899,7 +899,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
 	remove_int_watcher(t, watcher);
 
 	t->fwatchers = g_slist_remove(t->fwatchers, watcher);
-	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
+	g_dbus_remove_watch(watcher->id);
 
 	if (g_slist_length(t->fwatchers) == 0)
 		disable_final_measurement(t);
diff --git a/src/adapter.c b/src/adapter.c
index 2f8c8a3..090e8ae 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -568,7 +568,7 @@ static void session_free(void *data)
 	struct session_req *req = data;
 
 	if (req->id)
-		g_dbus_remove_watch(btd_get_dbus_connection(), req->id);
+		g_dbus_remove_watch(req->id);
 
 	if (req->msg) {
 		dbus_message_unref(req->msg);
diff --git a/src/agent.c b/src/agent.c
index e206c56..07e88e0 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -179,8 +179,7 @@ void agent_free(struct agent *agent)
 	}
 
 	if (!agent->exited) {
-		g_dbus_remove_watch(btd_get_dbus_connection(),
-							agent->listener_id);
+		g_dbus_remove_watch(agent->listener_id);
 		agent_release(agent);
 	}
 
diff --git a/src/device.c b/src/device.c
index 656f109..969fd0c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -187,8 +187,7 @@ static uint16_t uuid_list[] = {
 static void browse_request_free(struct browse_req *req)
 {
 	if (req->listener_id)
-		g_dbus_remove_watch(btd_get_dbus_connection(),
-							req->listener_id);
+		g_dbus_remove_watch(req->listener_id);
 	if (req->msg)
 		dbus_message_unref(req->msg);
 	if (req->device)
@@ -2004,8 +2003,7 @@ static void bonding_request_free(struct bonding_req *bonding)
 		return;
 
 	if (bonding->listener_id)
-		g_dbus_remove_watch(btd_get_dbus_connection(),
-							bonding->listener_id);
+		g_dbus_remove_watch(bonding->listener_id);
 
 	if (bonding->msg)
 		dbus_message_unref(bonding->msg);
diff --git a/src/profile.c b/src/profile.c
index eb63e1e..99b1b24 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1089,7 +1089,7 @@ DBusMessage *btd_profile_unreg_ext(DBusConnection *conn, DBusMessage *msg,
 	if (!ext)
 		return btd_error_does_not_exist(msg);
 
-	g_dbus_remove_watch(conn, ext->id);
+	g_dbus_remove_watch(ext->id);
 	remove_ext(ext);
 
 	return dbus_message_new_method_return(msg);
@@ -1113,7 +1113,7 @@ void btd_profile_cleanup(void)
 		if (msg)
 			g_dbus_send_message(conn, msg);
 
-		g_dbus_remove_watch(conn, ext->id);
+		g_dbus_remove_watch(ext->id);
 		remove_ext(ext);
 
 	}
-- 
1.7.11.4

[PATCH BlueZ 3/9] audio: Fix not freeing gateway agent data on exit

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

434 (168 direct, 266 indirect) bytes in 7 blocks are definitely lost in loss record 322 of 338
   at 0x4A06F18: calloc (vg_replace_malloc.c:566)
   by 0x4C802C6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3200.4)
   by 0x126C0C: register_agent (gateway.c:673)
   by 0x122960: process_message.isra.0 (object.c:197)
   by 0x4F70684: ??? (in /usr/lib64/libdbus-1.so.3.5.6)
   by 0x4F6290C: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.5.6)
   by 0x120FA7: message_dispatch (mainloop.c:76)
   by 0x4C7B22A: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
   by 0x4C7A694: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
   by 0x4C7A9C7: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
   by 0x4C7ADC1: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
   by 0x120671: main (main.c:551)
---
 audio/gateway.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index bf766f9..b1e9332 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -116,6 +116,8 @@ static void agent_free(struct hf_agent *agent)
 	if (agent->call)
 		dbus_pending_call_unref(agent->call);
 
+	g_dbus_remove_watch(agent->watch);
+
 	g_free(agent->name);
 	g_free(agent->path);
 	g_free(agent);
@@ -746,8 +748,6 @@ static DBusMessage *unregister_agent(DBusConnection *conn,
 	if (strcmp(gw->agent->path, path) != 0)
 		return btd_error_does_not_exist(msg);
 
-	g_dbus_remove_watch(gw->agent->watch);
-
 	agent_free(gw->agent);
 	gw->agent = NULL;
 
@@ -783,6 +783,9 @@ static void path_unregister(void *data)
 
 	gateway_close(dev);
 
+	if (dev->gateway->agent)
+		agent_free(dev->gateway->agent);
+
 	g_free(dev->gateway);
 	dev->gateway = NULL;
 }
-- 
1.7.11.4

[PATCH BlueZ 4/9] AVCTP: Simplify channel handling

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Make both control and browsing channels to use the same structure to
store its channel information.
---
 audio/avctp.c | 155 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 82 insertions(+), 73 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index 53c28ec..092a5e2 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -131,6 +131,12 @@ struct avctp_rsp_handler {
 	void *user_data;
 };
 
+struct avctp_channel {
+	GIOChannel *io;
+	guint watch;
+	uint16_t mtu;
+};
+
 struct avctp {
 	struct avctp_server *server;
 	bdaddr_t dst;
@@ -139,14 +145,10 @@ struct avctp {
 
 	int uinput;
 
-	GIOChannel *control_io;
-	GIOChannel *browsing_io;
-	guint control_io_id;
-	guint browsing_io_id;
 	guint auth_id;
 
-	uint16_t control_mtu;
-	uint16_t browsing_mtu;
+	struct avctp_channel *control;
+	struct avctp_channel *browsing;
 
 	uint8_t key_quirks[256];
 	GSList *handlers;
@@ -333,6 +335,17 @@ static struct avctp_pdu_handler *find_handler(GSList *list, uint8_t opcode)
 	return NULL;
 }
 
+static void avctp_channel_destroy(struct avctp_channel *chan)
+{
+	g_io_channel_shutdown(chan->io, TRUE, NULL);
+	g_io_channel_unref(chan->io);
+
+	if (chan->watch)
+		g_source_remove(chan->watch);
+
+	g_free(chan);
+}
+
 static void avctp_disconnected(struct avctp *session)
 {
 	struct avctp_server *server;
@@ -340,31 +353,15 @@ static void avctp_disconnected(struct avctp *session)
 	if (!session)
 		return;
 
-	if (session->browsing_io) {
-		g_io_channel_shutdown(session->browsing_io, TRUE, NULL);
-		g_io_channel_unref(session->browsing_io);
-		session->browsing_io = NULL;
-	}
-
-	if (session->browsing_io_id) {
-		g_source_remove(session->browsing_io_id);
-		session->browsing_io_id = 0;
-	}
-
-	if (session->control_io) {
-		g_io_channel_shutdown(session->control_io, TRUE, NULL);
-		g_io_channel_unref(session->control_io);
-		session->control_io = NULL;
-	}
+	if (session->browsing)
+		avctp_channel_destroy(session->browsing);
 
-	if (session->control_io_id) {
-		g_source_remove(session->control_io_id);
-		session->control_io_id = 0;
+	if (session->control)
+		avctp_channel_destroy(session->control);
 
-		if (session->auth_id != 0) {
-			btd_cancel_authorization(session->auth_id);
-			session->auth_id = 0;
-		}
+	if (session->auth_id != 0) {
+		btd_cancel_authorization(session->auth_id);
+		session->auth_id = 0;
 	}
 
 	if (session->uinput >= 0) {
@@ -420,7 +417,7 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
 	}
 }
 
-static void handle_response(struct avctp *session, struct avctp_header *avctp,
+static void control_response(struct avctp *session, struct avctp_header *avctp,
 				struct avc_header *avc, uint8_t *operands,
 				size_t operand_count)
 {
@@ -456,7 +453,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
 		goto failed;
 
-	sock = g_io_channel_unix_get_fd(session->browsing_io);
+	sock = g_io_channel_unix_get_fd(chan);
 
 	ret = read(sock, buf, sizeof(buf));
 	if (ret <= 0)
@@ -508,7 +505,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
 		goto failed;
 
-	sock = g_io_channel_unix_get_fd(session->control_io);
+	sock = g_io_channel_unix_get_fd(chan);
 
 	ret = read(sock, buf, sizeof(buf));
 	if (ret <= 0)
@@ -548,7 +545,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 			avc->opcode, operand_count);
 
 	if (avctp->cr == AVCTP_RESPONSE) {
-		handle_response(session, avctp, avc, operands, operand_count);
+		control_response(session, avctp, avc, operands, operand_count);
 		return TRUE;
 	}
 
@@ -676,6 +673,16 @@ static void init_uinput(struct avctp *session)
 		DBG("AVRCP: uinput initialized for %s", address);
 }
 
+static struct avctp_channel *avctp_channel_create(GIOChannel *io)
+{
+	struct avctp_channel *chan;
+
+	chan = g_new0(struct avctp_channel, 1);
+	chan->io = g_io_channel_ref(io);
+
+	return chan;
+}
+
 static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 							gpointer data)
 {
@@ -686,10 +693,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 
 	if (err) {
 		error("Browsing: %s", err->message);
-		g_io_channel_shutdown(chan, TRUE, NULL);
-		g_io_channel_unref(chan);
-		session->browsing_io = NULL;
-		return;
+		goto fail;
 	}
 
 	bt_io_get(chan, &gerr,
@@ -700,20 +704,28 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 		error("%s", gerr->message);
 		g_io_channel_shutdown(chan, TRUE, NULL);
 		g_io_channel_unref(chan);
-		session->browsing_io = NULL;
 		g_error_free(gerr);
-		return;
+		goto fail;
 	}
 
 	DBG("AVCTP Browsing: connected to %s", address);
 
-	if (!session->browsing_io)
-		session->browsing_io = g_io_channel_ref(chan);
+	if (session->browsing == NULL)
+		session->browsing = avctp_channel_create(chan);
 
-	session->browsing_mtu = imtu;
-	session->browsing_io_id = g_io_add_watch(chan,
+	session->browsing->mtu = imtu;
+	session->browsing->watch = g_io_add_watch(session->browsing->io,
 				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-				(GIOFunc) session_browsing_cb, session); }
+				(GIOFunc) session_browsing_cb, session);
+
+	return;
+
+fail:
+	if (session->browsing) {
+		avctp_channel_destroy(session->browsing);
+		session->browsing = NULL;
+	}
+}
 
 static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 {
@@ -741,16 +753,17 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 
 	DBG("AVCTP: connected to %s", address);
 
-	if (!session->control_io)
-		session->control_io = g_io_channel_ref(chan);
+	if (session->control == NULL)
+		session->control = avctp_channel_create(chan);
+
+	session->control->mtu = imtu;
+	session->control->watch = g_io_add_watch(session->control->io,
+				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+				(GIOFunc) session_cb, session);
 
 	init_uinput(session);
 
 	avctp_set_state(session, AVCTP_STATE_CONNECTED);
-	session->control_mtu = imtu;
-	session->control_io_id = g_io_add_watch(chan,
-				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-				(GIOFunc) session_cb, session);
 }
 
 static void auth_cb(DBusError *derr, void *user_data)
@@ -760,9 +773,9 @@ static void auth_cb(DBusError *derr, void *user_data)
 
 	session->auth_id = 0;
 
-	if (session->control_io_id) {
-		g_source_remove(session->control_io_id);
-		session->control_io_id = 0;
+	if (session->control->watch > 0) {
+		g_source_remove(session->control->watch);
+		session->control->watch = 0;
 	}
 
 	if (derr && dbus_error_is_set(derr)) {
@@ -771,7 +784,7 @@ static void auth_cb(DBusError *derr, void *user_data)
 		return;
 	}
 
-	if (!bt_io_accept(session->control_io, avctp_connect_cb, session,
+	if (!bt_io_accept(session->control->io, avctp_connect_cb, session,
 								NULL, &err)) {
 		error("bt_io_accept: %s", err->message);
 		g_error_free(err);
@@ -836,14 +849,14 @@ static struct avctp *avctp_get_internal(const bdaddr_t *src,
 static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
 						struct audio_device *dev)
 {
-	if (session->control_io) {
+	if (session->control != NULL) {
 		error("Control: Refusing unexpected connect");
 		g_io_channel_shutdown(chan, TRUE, NULL);
 		return;
 	}
 
 	avctp_set_state(session, AVCTP_STATE_CONNECTING);
-	session->control_io = g_io_channel_ref(chan);
+	session->control = avctp_channel_create(chan);
 
 	session->auth_id = btd_request_authorization(&dev->src, &dev->dst,
 							AVRCP_TARGET_UUID,
@@ -851,7 +864,7 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
 	if (session->auth_id == 0)
 		goto drop;
 
-	session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
+	session->control->watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
 						G_IO_NVAL, session_cb, session);
 	return;
 
@@ -864,7 +877,7 @@ static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
 {
 	GError *err = NULL;
 
-	if (session->control_io == NULL || session->browsing_io) {
+	if (session->control == NULL || session->browsing != NULL) {
 		error("Browsing: Refusing unexpected connect");
 		g_io_channel_shutdown(chan, TRUE, NULL);
 		return;
@@ -911,8 +924,8 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
 	DBG("AVCTP: incoming connect from %s", address);
 
 	session = avctp_get_internal(&src, &dst);
-	if (!session)
-		goto drop;
+	if (session == NULL)
+		return;
 
 	dev = manager_get_device(&src, &dst, FALSE);
 	if (!dev) {
@@ -942,13 +955,7 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
 	return;
 
 drop:
-	if (session && session->browsing_io)
-		g_io_channel_unref(session->browsing_io);
-
-	if (session && session->control_io)
-		g_io_channel_unref(session->control_io);
-
-	if (session && psm == AVCTP_CONTROL_PSM)
+	if (psm == AVCTP_CONTROL_PSM)
 		avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
 }
 
@@ -1086,7 +1093,7 @@ int avctp_send_passthrough(struct avctp *session, uint8_t op)
 	operands[0] = op & 0x7f;
 	operands[1] = 0;
 
-	sk = g_io_channel_unix_get_fd(session->control_io);
+	sk = g_io_channel_unix_get_fd(session->control->io);
 
 	if (write(sk, buf, sizeof(buf)) < 0)
 		return -errno;
@@ -1115,7 +1122,7 @@ static int avctp_send(struct avctp *session, uint8_t transaction, uint8_t cr,
 	if (session->state != AVCTP_STATE_CONNECTED)
 		return -ENOTCONN;
 
-	sk = g_io_channel_unix_get_fd(session->control_io);
+	sk = g_io_channel_unix_get_fd(session->control->io);
 
 	memset(buf, 0, sizeof(buf));
 
@@ -1299,7 +1306,8 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
 		return NULL;
 	}
 
-	session->control_io = io;
+	session->control = avctp_channel_create(io);
+	g_io_channel_unref(io);
 
 	return session;
 }
@@ -1312,7 +1320,7 @@ int avctp_connect_browsing(struct avctp *session)
 	if (session->state != AVCTP_STATE_CONNECTED)
 		return -ENOTCONN;
 
-	if (session->browsing_io != NULL)
+	if (session->browsing != NULL)
 		return 0;
 
 	io = bt_io_connect(avctp_connect_browsing_cb, session, NULL, &err,
@@ -1327,14 +1335,15 @@ int avctp_connect_browsing(struct avctp *session)
 		return -EIO;
 	}
 
-	session->browsing_io = io;
+	session->browsing = avctp_channel_create(io);
+	g_io_channel_unref(io);
 
 	return 0;
 }
 
 void avctp_disconnect(struct avctp *session)
 {
-	if (!session->control_io)
+	if (session->state == AVCTP_STATE_DISCONNECTED)
 		return;
 
 	avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
-- 
1.7.11.4

[PATCH BlueZ 5/9] AVCTP: Allocate memory to hold incoming/outgoing PDUs

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes possible to the handler to respond asyncronous as the memory
remains valid after it returns.

In addition to that it uses the MTU to calculate the buffer size
necessary.
---
 audio/avctp.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index 092a5e2..aa9a1ca 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -134,7 +134,9 @@ struct avctp_rsp_handler {
 struct avctp_channel {
 	GIOChannel *io;
 	guint watch;
-	uint16_t mtu;
+	uint16_t imtu;
+	uint16_t omtu;
+	uint8_t *buffer;
 };
 
 struct avctp {
@@ -343,6 +345,7 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
 	if (chan->watch)
 		g_source_remove(chan->watch);
 
+	g_free(chan->buffer);
 	g_free(chan);
 }
 
@@ -446,7 +449,9 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
 				gpointer data)
 {
 	struct avctp *session = data;
-	uint8_t buf[1024], *operands;
+	struct avctp_channel *browsing = session->browsing;
+	uint8_t *buf = browsing->buffer;
+	uint8_t *operands;
 	struct avctp_header *avctp;
 	int sock, ret, packet_size, operand_count;
 
@@ -455,7 +460,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
 
 	sock = g_io_channel_unix_get_fd(chan);
 
-	ret = read(sock, buf, sizeof(buf));
+	ret = read(sock, buf, sizeof(browsing->imtu));
 	if (ret <= 0)
 		goto failed;
 
@@ -496,7 +501,9 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 				gpointer data)
 {
 	struct avctp *session = data;
-	uint8_t buf[1024], *operands, code, subunit;
+	struct avctp_channel *control = session->control;
+	uint8_t *buf = control->buffer;
+	uint8_t *operands, code, subunit;
 	struct avctp_header *avctp;
 	struct avc_header *avc;
 	int ret, packet_size, operand_count, sock;
@@ -507,7 +514,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 
 	sock = g_io_channel_unix_get_fd(chan);
 
-	ret = read(sock, buf, sizeof(buf));
+	ret = read(sock, buf, control->imtu);
 	if (ret <= 0)
 		goto failed;
 
@@ -688,7 +695,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 {
 	struct avctp *session = data;
 	char address[18];
-	uint16_t imtu;
+	uint16_t imtu, omtu;
 	GError *gerr = NULL;
 
 	if (err) {
@@ -699,6 +706,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 	bt_io_get(chan, &gerr,
 			BT_IO_OPT_DEST, &address,
 			BT_IO_OPT_IMTU, &imtu,
+			BT_IO_OPT_OMTU, &omtu,
 			BT_IO_OPT_INVALID);
 	if (gerr) {
 		error("%s", gerr->message);
@@ -713,7 +721,9 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 	if (session->browsing == NULL)
 		session->browsing = avctp_channel_create(chan);
 
-	session->browsing->mtu = imtu;
+	session->browsing->imtu = imtu;
+	session->browsing->omtu = omtu;
+	session->browsing->buffer = g_malloc0(MAX(imtu, omtu));
 	session->browsing->watch = g_io_add_watch(session->browsing->io,
 				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
 				(GIOFunc) session_browsing_cb, session);
@@ -731,7 +741,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 {
 	struct avctp *session = data;
 	char address[18];
-	uint16_t imtu;
+	uint16_t imtu, omtu;
 	GError *gerr = NULL;
 
 	if (err) {
@@ -743,6 +753,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	bt_io_get(chan, &gerr,
 			BT_IO_OPT_DEST, &address,
 			BT_IO_OPT_IMTU, &imtu,
+			BT_IO_OPT_IMTU, &omtu,
 			BT_IO_OPT_INVALID);
 	if (gerr) {
 		avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
@@ -756,7 +767,9 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	if (session->control == NULL)
 		session->control = avctp_channel_create(chan);
 
-	session->control->mtu = imtu;
+	session->control->imtu = imtu;
+	session->control->omtu = omtu;
+	session->control->buffer = g_malloc0(MAX(imtu, omtu));
 	session->control->watch = g_io_add_watch(session->control->io,
 				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
 				(GIOFunc) session_cb, session);
-- 
1.7.11.4

[PATCH BlueZ 6/9] AVRCP: Register to AVCTP state changes without depending on player

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

It is not longer necessary to have a player to be able to register the
extra pdu handlers.
---
 audio/avrcp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index aa62987..295a1e1 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1467,6 +1467,9 @@ int avrcp_register(const bdaddr_t *src, GKeyFile *config)
 
 	servers = g_slist_append(servers, server);
 
+	if (!avctp_id)
+		avctp_id = avctp_add_state_cb(state_changed, NULL);
+
 	return 0;
 }
 
@@ -1528,9 +1531,6 @@ struct avrcp_player *avrcp_register_player(const bdaddr_t *src,
 	player->user_data = user_data;
 	player->destroy = destroy;
 
-	if (!avctp_id)
-		avctp_id = avctp_add_state_cb(state_changed, NULL);
-
 	server->players = g_slist_append(server->players, player);
 
 	/* Assign player to session without current player */
-- 
1.7.11.4

[PATCH BlueZ 7/9] AVRCP: Don't respond with errors when no player is registered

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Some devices w.g. Sony MW600 will stop using certain commands if an
error happen, so the code now just fake a player and once a real
player is registered it takes place of the fake one.
---
 audio/avrcp.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 63 insertions(+), 14 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 295a1e1..1ed6a24 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -477,6 +477,17 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
 	return;
 }
 
+static void *player_get_metadata(struct avrcp_player *player, uint32_t attr)
+{
+	if (player != NULL)
+		return player->cb->get_metadata(attr, player->user_data);
+
+	if (attr == AVRCP_MEDIA_ATTRIBUTE_TITLE)
+		return "";
+
+	return NULL;
+}
+
 static uint16_t player_write_media_attribute(struct avrcp_player *player,
 						uint32_t id, uint8_t *buf,
 						uint16_t *pos,
@@ -489,7 +500,7 @@ static uint16_t player_write_media_attribute(struct avrcp_player *player,
 
 	DBG("%u", id);
 
-	value = player->cb->get_metadata(id, player->user_data);
+	value = player_get_metadata(player, id);
 	if (value == NULL) {
 		*offset = 0;
 		return 0;
@@ -588,6 +599,9 @@ static int player_set_attribute(struct avrcp_player *player,
 {
 	DBG("Change attribute: %u %u", attr, val);
 
+	if (player == NULL)
+		return -ENOENT;
+
 	return player->cb->set_setting(attr, val, player->user_data);
 }
 
@@ -597,6 +611,9 @@ static int player_get_attribute(struct avrcp_player *player, uint8_t attr)
 
 	DBG("attr %u", attr);
 
+	if (player == NULL)
+		return -ENOENT;
+
 	value = player->cb->get_setting(attr, player->user_data);
 	if (value < 0)
 		DBG("attr %u not supported by player", attr);
@@ -653,7 +670,7 @@ static uint8_t avrcp_handle_list_player_attributes(struct avrcp *session,
 	uint16_t len = ntohs(pdu->params_len);
 	unsigned int i;
 
-	if (len != 0 || player == NULL) {
+	if (len != 0) {
 		pdu->params_len = htons(1);
 		pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
 		return AVC_CTYPE_REJECTED;
@@ -685,7 +702,7 @@ static uint8_t avrcp_handle_list_player_values(struct avrcp *session,
 	uint16_t len = ntohs(pdu->params_len);
 	unsigned int i;
 
-	if (len != 1 || player == NULL)
+	if (len != 1)
 		goto err;
 
 	if (player_get_attribute(player, pdu->params[0]) < 0)
@@ -707,6 +724,15 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static GList *player_list_metadata(struct avrcp_player *player)
+{
+	if (player != NULL)
+		return player->cb->list_metadata(player->user_data);
+
+	return g_list_prepend(NULL,
+				GUINT_TO_POINTER(AVRCP_MEDIA_ATTRIBUTE_TITLE));
+}
+
 static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -719,7 +745,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
 	GList *attr_ids;
 	uint16_t offset;
 
-	if (len < 9 || identifier != 0 || player == NULL)
+	if (len < 9 || identifier != 0)
 		goto err;
 
 	nattr = pdu->params[8];
@@ -732,7 +758,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
 		 * Return all available information, at least
 		 * title must be returned if there's a track selected.
 		 */
-		attr_ids = player->cb->list_metadata(player->user_data);
+		attr_ids = player_list_metadata(player);
 		len = g_list_length(attr_ids);
 	} else {
 		unsigned int i;
@@ -788,8 +814,7 @@ static uint8_t avrcp_handle_get_current_player_value(struct avrcp *session,
 	uint8_t *settings;
 	unsigned int i;
 
-	if (player == NULL || len <= 1 || pdu->params[0] != len - 1 ||
-							player == NULL)
+	if (len <= 1 || pdu->params[0] != len - 1)
 		goto err;
 
 	/*
@@ -922,6 +947,14 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static uint32_t player_get_position(struct avrcp_player *player)
+{
+	if (player == NULL)
+		return 0;
+
+	return player->cb->get_position(player->user_data);
+}
+
 static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -932,15 +965,15 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 	uint32_t duration;
 	void *pduration;
 
-	if (len != 0 || player == NULL) {
+	if (len != 0) {
 		pdu->params_len = htons(1);
 		pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
 		return AVC_CTYPE_REJECTED;
 	}
 
-	position = player->cb->get_position(player->user_data);
-	pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
-							player->user_data);
+	position = player_get_position(player);
+	pduration = player_get_metadata(player,
+					AVRCP_MEDIA_ATTRIBUTE_DURATION);
 	if (pduration != NULL)
 		duration = htonl(GPOINTER_TO_UINT(pduration));
 	else
@@ -957,6 +990,22 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 	return AVC_CTYPE_STABLE;
 }
 
+static uint64_t player_get_uid(struct avrcp_player *player)
+{
+	if (player == NULL)
+		return UINT64_MAX;
+
+	return player->cb->get_status(player->user_data);
+}
+
+static uint8_t player_get_status(struct avrcp_player *player)
+{
+	if (player == NULL)
+		return AVRCP_PLAY_STATUS_STOPPED;
+
+	return player->cb->get_status(player->user_data);
+}
+
 static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -970,18 +1019,18 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 	 * one is applicable only for EVENT_PLAYBACK_POS_CHANGED. See AVRCP
 	 * 1.3 spec, section 5.4.2.
 	 */
-	if (len != 5 || player == NULL)
+	if (len != 5)
 		goto err;
 
 	switch (pdu->params[0]) {
 	case AVRCP_EVENT_STATUS_CHANGED:
 		len = 2;
-		pdu->params[1] = player->cb->get_status(player->user_data);
+		pdu->params[1] = player_get_status(player);
 
 		break;
 	case AVRCP_EVENT_TRACK_CHANGED:
 		len = 9;
-		uid = player->cb->get_uid(player->user_data);
+		uid = player_get_uid(player);
 		memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
 
 		break;
-- 
1.7.11.4

[PATCH BlueZ 8/9] AVRCP: Fix crash on disconnect

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

In case of multiple session being active the code was registering one
PDU hanlder per AVRCP session and given the session pointer as user data
causing the following:

    24 bytes in 1 blocks are definitely lost in loss record 370 of 893
       at 0x4A0884D: malloc (vg_replace_malloc.c:263)
       by 0x4C803FE: g_malloc (in /usr/lib64/libglib-2.0.so.0.3200.4)
       by 0x12EE9D: avctp_register_browsing_pdu_handler (avctp.c:1259)
       by 0x12FD7B: state_changed (avrcp.c:1402)
       by 0x12D391: avctp_set_state (avctp.c:403)
       by 0x12E6B4: avctp_confirm_cb (avctp.c:871)
       by 0x1606A3: server_cb (btio.c:254)
       by 0x4C7A824: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
       by 0x4C7AB57: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
       by 0x4C7AF51: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
       by 0x120EA1: main (main.c:551)

To fix this the PDU handlers are now per AVCTP session/channel so each
AVRCP session can register its own handler and pass itself as user
data.
---
 audio/avctp.c | 180 ++++++++++++++++++++++++++++++++++++----------------------
 audio/avctp.h |  10 ++--
 audio/avrcp.c |  56 +++++++++---------
 audio/avrcp.h |   1 +
 4 files changed, 148 insertions(+), 99 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index aa9a1ca..228d5b7 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -137,6 +137,7 @@ struct avctp_channel {
 	uint16_t imtu;
 	uint16_t omtu;
 	uint8_t *buffer;
+	GSList *handlers;
 };
 
 struct avctp {
@@ -148,6 +149,9 @@ struct avctp {
 	int uinput;
 
 	guint auth_id;
+	unsigned int passthrough_id;
+	unsigned int unit_id;
+	unsigned int subunit_id;
 
 	struct avctp_channel *control;
 	struct avctp_channel *browsing;
@@ -186,9 +190,7 @@ static struct {
 
 static GSList *callbacks = NULL;
 static GSList *servers = NULL;
-static GSList *control_handlers = NULL;
 static uint8_t id = 0;
-static struct avctp_browsing_pdu_handler *browsing_handler = NULL;
 
 static void auth_cb(DBusError *derr, void *user_data);
 
@@ -346,6 +348,7 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
 		g_source_remove(chan->watch);
 
 	g_free(chan->buffer);
+	g_slist_free_full(chan->handlers, g_free);
 	g_free(chan);
 }
 
@@ -454,6 +457,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
 	uint8_t *operands;
 	struct avctp_header *avctp;
 	int sock, ret, packet_size, operand_count;
+	struct avctp_browsing_pdu_handler *handler;
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
 		goto failed;
@@ -480,10 +484,18 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
 	packet_size = AVCTP_HEADER_LENGTH;
 	avctp->cr = AVCTP_RESPONSE;
 
-	packet_size += browsing_handler->cb(session, avctp->transaction,
+	handler = g_slist_nth_data(browsing->handlers, 0);
+	if (handler == NULL) {
+		DBG("handler not found");
+		packet_size += avrcp_browsing_general_reject(operands);
+		goto send;
+	}
+
+	packet_size += handler->cb(session, avctp->transaction,
 						operands, operand_count,
-						browsing_handler->user_data);
+						handler->user_data);
 
+send:
 	if (packet_size != 0) {
 		ret = write(sock, buf, packet_size);
 		if (ret != packet_size)
@@ -570,7 +582,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 		goto done;
 	}
 
-	handler = find_handler(control_handlers, avc->opcode);
+	handler = find_handler(control->handlers, avc->opcode);
 	if (!handler) {
 		DBG("handler not found for 0x%02x", avc->opcode);
 		packet_size += avrcp_handle_vendor_reject(&code, operands);
@@ -774,6 +786,19 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
 				(GIOFunc) session_cb, session);
 
+	session->passthrough_id = avctp_register_pdu_handler(session,
+						AVC_OP_PASSTHROUGH,
+						handle_panel_passthrough,
+						NULL);
+	session->unit_id = avctp_register_pdu_handler(session,
+						AVC_OP_UNITINFO,
+						handle_unit_info,
+						NULL);
+	session->subunit_id = avctp_register_pdu_handler(session,
+						AVC_OP_SUBUNITINFO,
+						handle_subunit_info,
+						NULL);
+
 	init_uinput(session);
 
 	avctp_set_state(session, AVCTP_STATE_CONNECTED);
@@ -896,12 +921,6 @@ static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
 		return;
 	}
 
-	if (browsing_handler == NULL) {
-		error("Browsing: Handler not registered");
-		g_io_channel_shutdown(chan, TRUE, NULL);
-		return;
-	}
-
 	if (bt_io_accept(chan, avctp_connect_browsing_cb, session, NULL,
 								&err))
 		return;
@@ -994,10 +1013,6 @@ static GIOChannel *avctp_server_socket(const bdaddr_t *src, gboolean master,
 	return io;
 }
 
-static unsigned int passthrough_id = 0;
-static unsigned int unit_id = 0;
-static unsigned int subunit_id = 0;
-
 int avctp_register(const bdaddr_t *src, gboolean master)
 {
 	struct avctp_server *server;
@@ -1026,18 +1041,6 @@ int avctp_register(const bdaddr_t *src, gboolean master)
 
 	servers = g_slist_append(servers, server);
 
-	if (!passthrough_id)
-		passthrough_id = avctp_register_pdu_handler(AVC_OP_PASSTHROUGH,
-					handle_panel_passthrough, NULL);
-
-	if (!unit_id)
-		unit_id = avctp_register_pdu_handler(AVC_OP_UNITINFO, handle_unit_info,
-									NULL);
-
-	if (!subunit_id)
-		subunit_id = avctp_register_pdu_handler(AVC_OP_SUBUNITINFO,
-						handle_subunit_info, NULL);
-
 	return 0;
 }
 
@@ -1061,24 +1064,6 @@ void avctp_unregister(const bdaddr_t *src)
 	g_io_channel_shutdown(server->control_io, TRUE, NULL);
 	g_io_channel_unref(server->control_io);
 	g_free(server);
-
-	if (servers)
-		return;
-
-	if (passthrough_id) {
-		avctp_unregister_pdu_handler(passthrough_id);
-		passthrough_id = 0;
-	}
-
-	if (unit_id) {
-		avctp_unregister_pdu_handler(unit_id);
-		passthrough_id = 0;
-	}
-
-	if (subunit_id) {
-		avctp_unregister_pdu_handler(subunit_id);
-		subunit_id = 0;
-	}
 }
 
 int avctp_send_passthrough(struct avctp *session, uint8_t op)
@@ -1230,13 +1215,18 @@ gboolean avctp_remove_state_cb(unsigned int id)
 	return FALSE;
 }
 
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
-							void *user_data)
+unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
+						avctp_control_pdu_cb cb,
+						void *user_data)
 {
+	struct avctp_channel *control = session->control;
 	struct avctp_pdu_handler *handler;
 	static unsigned int id = 0;
 
-	handler = find_handler(control_handlers, opcode);
+	if (control == NULL)
+		return 0;
+
+	handler = find_handler(control->handlers, opcode);
 	if (handler)
 		return 0;
 
@@ -1246,36 +1236,63 @@ unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
 	handler->user_data = user_data;
 	handler->id = ++id;
 
-	control_handlers = g_slist_append(control_handlers, handler);
+	control->handlers = g_slist_append(control->handlers, handler);
 
 	return handler->id;
 }
 
-unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
-					void *user_data)
+unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
+						avctp_browsing_pdu_cb cb,
+						void *user_data)
 {
-	unsigned int id = 0;
+	struct avctp_channel *browsing = session->browsing;
+	struct avctp_browsing_pdu_handler *handler;
+	static unsigned int id = 0;
+
+	if (browsing == NULL)
+		return 0;
+
+	if (browsing->handlers != NULL)
+		return 0;
+
+	handler = g_new(struct avctp_browsing_pdu_handler, 1);
+	handler->cb = cb;
+	handler->user_data = user_data;
+	handler->id = ++id;
 
-	browsing_handler = g_new(struct avctp_browsing_pdu_handler, 1);
-	browsing_handler->cb = cb;
-	browsing_handler->user_data = user_data;
-	browsing_handler->id = ++id;
+	browsing->handlers = g_slist_append(browsing->handlers, handler);
 
-	return browsing_handler->id;
+	return handler->id;
 }
 
 gboolean avctp_unregister_pdu_handler(unsigned int id)
 {
 	GSList *l;
 
-	for (l = control_handlers; l != NULL; l = l->next) {
-		struct avctp_pdu_handler *handler = l->data;
+	for (l = servers; l; l = l->next) {
+		struct avctp_server *server = l->data;
+		GSList *s;
 
-		if (handler->id == id) {
-			control_handlers = g_slist_remove(control_handlers,
-								handler);
-			g_free(handler);
-			return TRUE;
+		for (s = server->sessions; s; s = s->next) {
+			struct avctp *session = s->data;
+			struct avctp_channel *control = session->control;
+			GSList *h;
+
+			if (control == NULL)
+				continue;
+
+			for (h = control->handlers; h; h = h->next) {
+				struct avctp_pdu_handler *handler = h->data;
+
+				if (handler->id != id)
+					continue;
+
+				control->handlers = g_slist_remove(
+							control->handlers,
+							handler);
+				g_free(handler);
+				return TRUE;
+			}
 		}
 	}
 
@@ -1284,12 +1301,37 @@ gboolean avctp_unregister_pdu_handler(unsigned int id)
 
 gboolean avctp_unregister_browsing_pdu_handler(unsigned int id)
 {
-	if (browsing_handler->id != id)
-		return FALSE;
+	GSList *l;
 
-	g_free(browsing_handler);
-	browsing_handler = NULL;
-	return TRUE;
+	for (l = servers; l; l = l->next) {
+		struct avctp_server *server = l->data;
+		GSList *s;
+
+		for (s = server->sessions; s; s = s->next) {
+			struct avctp *session = l->data;
+			struct avctp_channel *browsing = session->browsing;
+			GSList *h;
+
+			if (browsing == NULL)
+				continue;
+
+			for (h = browsing->handlers; h; h = h->next) {
+				struct avctp_browsing_pdu_handler *handler =
+								h->data;
+
+				if (handler->id != id)
+					continue;
+
+				browsing->handlers = g_slist_remove(
+							browsing->handlers,
+							handler);
+				g_free(handler);
+				return TRUE;
+			}
+		}
+	}
+
+	return FALSE;
 }
 
 struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
diff --git a/audio/avctp.h b/audio/avctp.h
index 6d39a20..41cf6c5 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -98,12 +98,14 @@ struct avctp *avctp_get(const bdaddr_t *src, const bdaddr_t *dst);
 int avctp_connect_browsing(struct avctp *session);
 void avctp_disconnect(struct avctp *session);
 
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
-							void *user_data);
+unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
+						avctp_control_pdu_cb cb,
+						void *user_data);
 gboolean avctp_unregister_pdu_handler(unsigned int id);
 
-unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
-							void *user_data);
+unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
+						avctp_browsing_pdu_cb cb,
+						void *user_data);
 gboolean avctp_unregister_browsing_pdu_handler(unsigned int id);
 
 int avctp_send_passthrough(struct avctp *session, uint8_t op);
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 1ed6a24..f31372b 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -184,8 +184,8 @@ struct avrcp {
 	uint16_t version;
 	int features;
 
-	unsigned int control_handler;
-	unsigned int browsing_handler;
+	unsigned int control_id;
+	unsigned int browsing_id;
 	uint16_t registered_events;
 	uint8_t transaction;
 	uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
@@ -1228,6 +1228,19 @@ static struct browsing_pdu_handler {
 		{ },
 };
 
+size_t avrcp_browsing_general_reject(uint8_t *operands)
+{
+	struct avrcp_browsing_header *pdu = (void *) operands;
+	uint8_t status;
+
+	pdu->pdu_id = AVRCP_GENERAL_REJECT;
+	status = AVRCP_STATUS_INVALID_COMMAND;
+
+	pdu->param_len = htons(sizeof(status));
+	memcpy(pdu->params, &status, (sizeof(status)));
+	return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
+}
+
 static size_t handle_browsing_pdu(struct avctp *conn,
 					uint8_t transaction, uint8_t *operands,
 					size_t operand_count, void *user_data)
@@ -1235,7 +1248,6 @@ static size_t handle_browsing_pdu(struct avctp *conn,
 	struct avrcp *session = user_data;
 	struct browsing_pdu_handler *handler;
 	struct avrcp_browsing_header *pdu = (void *) operands;
-	uint8_t status;
 
 	DBG("AVRCP Browsing PDU 0x%02X, len 0x%04X", pdu->pdu_id,
 							pdu->param_len);
@@ -1245,20 +1257,12 @@ static size_t handle_browsing_pdu(struct avctp *conn,
 			break;
 	}
 
-	if (handler == NULL || handler->func == NULL) {
-		pdu->pdu_id = AVRCP_GENERAL_REJECT;
-		status = AVRCP_STATUS_INVALID_COMMAND;
-		goto err;
-	}
+	if (handler == NULL || handler->func == NULL)
+		return avrcp_browsing_general_reject(operands);
 
 	session->transaction = transaction;
 	handler->func(session, pdu, transaction);
 	return AVRCP_BROWSING_HEADER_LENGTH + ntohs(pdu->param_len);
-
-err:
-	pdu->param_len = htons(sizeof(status));
-	memcpy(pdu->params, &status, (sizeof(status)));
-	return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
 }
 
 size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands)
@@ -1370,12 +1374,12 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 
 		server->sessions = g_slist_remove(server->sessions, session);
 
-		if (session->control_handler)
-			avctp_unregister_pdu_handler(session->control_handler);
+		if (session->control_id > 0)
+			avctp_unregister_pdu_handler(session->control_id);
 
-		if (session->browsing_handler)
+		if (session->browsing_id > 0)
 			avctp_unregister_browsing_pdu_handler(
-						session->browsing_handler);
+							session->browsing_id);
 
 		if (session->player != NULL)
 			session->player->sessions = g_slist_remove(
@@ -1394,15 +1398,6 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 		session->dev = dev;
 		session->player = g_slist_nth_data(server->players, 0);
 
-		session->control_handler = avctp_register_pdu_handler(
-							AVC_OP_VENDORDEP,
-							handle_vendordep_pdu,
-							session);
-		session->browsing_handler =
-					avctp_register_browsing_pdu_handler(
-							handle_browsing_pdu,
-							session);
-
 		server->sessions = g_slist_append(server->sessions, session);
 
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
@@ -1429,6 +1424,15 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 			if (session->features & AVRCP_FEATURE_BROWSING)
 				avctp_connect_browsing(session->conn);
 		}
+
+		session->control_id = avctp_register_pdu_handler(session->conn,
+							AVC_OP_VENDORDEP,
+							handle_vendordep_pdu,
+							session);
+		session->browsing_id = avctp_register_browsing_pdu_handler(
+							session->conn,
+							handle_browsing_pdu,
+							session);
 	default:
 		return;
 	}
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 42d902b..6c651dd 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -105,3 +105,4 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data);
 
 
 size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
+size_t avrcp_browsing_general_reject(uint8_t *operands);
-- 
1.7.11.4

[PATCH BlueZ 9/9] AVRCP: Simplify state_changed callback

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Move session creation and destroy to their own functions
---
 audio/avrcp.c | 91 ++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 53 insertions(+), 38 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index f31372b..7188039 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1352,15 +1352,63 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
 	return NULL;
 }
 
-static void state_changed(struct audio_device *dev, avctp_state_t old_state,
-				avctp_state_t new_state, void *user_data)
+static struct avrcp *session_create(struct avrcp_server *server,
+						struct audio_device *dev)
 {
-	struct avrcp_server *server;
 	struct avrcp *session;
 	const sdp_record_t *rec;
 	sdp_list_t *list;
 	sdp_profile_desc_t *desc;
 
+	session = g_new0(struct avrcp, 1);
+	session->server = server;
+	session->conn = avctp_connect(&dev->src, &dev->dst);
+	session->dev = dev;
+	session->player = g_slist_nth_data(server->players, 0);
+
+	server->sessions = g_slist_append(server->sessions, session);
+
+	rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
+	if (rec == NULL)
+		return session;
+
+	if (sdp_get_profile_descs(rec, &list) < 0)
+		return session;
+
+	desc = list->data;
+	session->version = desc->version;
+	sdp_get_int_attr(rec, SDP_ATTR_SUPPORTED_FEATURES, &session->features);
+
+	sdp_list_free(list, free);
+
+	return session;
+}
+
+static void session_destroy(struct avrcp *session)
+{
+	struct avrcp_server *server = session->server;
+	struct avrcp_player *player = session->player;
+
+	server->sessions = g_slist_remove(server->sessions, session);
+
+	if (session->control_id > 0)
+		avctp_unregister_pdu_handler(session->control_id);
+
+	if (session->browsing_id > 0)
+		avctp_unregister_browsing_pdu_handler(session->browsing_id);
+
+	if (player != NULL)
+		player->sessions = g_slist_remove(player->sessions, session);
+
+	g_free(session);
+}
+
+static void state_changed(struct audio_device *dev, avctp_state_t old_state,
+				avctp_state_t new_state, void *user_data)
+{
+	struct avrcp_server *server;
+	struct avrcp *session;
+
 	server = find_server(servers, &dev->src);
 	if (!server)
 		return;
@@ -1372,47 +1420,14 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 		if (session == NULL)
 			break;
 
-		server->sessions = g_slist_remove(server->sessions, session);
+		session_destroy(session);
 
-		if (session->control_id > 0)
-			avctp_unregister_pdu_handler(session->control_id);
-
-		if (session->browsing_id > 0)
-			avctp_unregister_browsing_pdu_handler(
-							session->browsing_id);
-
-		if (session->player != NULL)
-			session->player->sessions = g_slist_remove(
-						session->player->sessions,
-						session);
-
-		g_free(session);
 		break;
 	case AVCTP_STATE_CONNECTING:
 		if (session != NULL)
 			break;
 
-		session = g_new0(struct avrcp, 1);
-		session->server = server;
-		session->conn = avctp_connect(&dev->src, &dev->dst);
-		session->dev = dev;
-		session->player = g_slist_nth_data(server->players, 0);
-
-		server->sessions = g_slist_append(server->sessions, session);
-
-		rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
-		if (rec == NULL)
-			return;
-
-		if (sdp_get_profile_descs(rec, &list) < 0)
-			return;
-
-		desc = list->data;
-		session->version = desc->version;
-		sdp_get_int_attr(rec, SDP_ATTR_SUPPORTED_FEATURES,
-							&session->features);
-
-		sdp_list_free(list, free);
+		session_create(server, dev);
 
 		break;
 	case AVCTP_STATE_CONNECTED:
-- 
1.7.11.4

Re: [PATCH BlueZ 1/9] gdbus: Remove connection from g_dbus_remove_watch

[Marcel Holtmann]

Hi Luiz,

> The connection is not really needed since the list of listeners is
> global not per connection, besides it is more convenient this way as
> only the id is needed.

they are not global, they are per connection. That we do not fully
implement this correctly is another story. That should be fixed here.

Regards

Marcel

Re: [PATCH BlueZ 1/9] gdbus: Remove connection from g_dbus_remove_watch

[Luiz Augusto von Dentz]

Hi Marcel,

On Sat, Oct 6, 2012 at 4:40 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> The connection is not really needed since the list of listeners is
>> global not per connection, besides it is more convenient this way as
>> only the id is needed.
>
> they are not global, they are per connection. That we do not fully
> implement this correctly is another story. That should be fixed here.

Not sure if Im following you, the point here is that the caller only
need to store the id because the data that the id refer to already
store the connection as context. Btw let me paste what
g_dbus_remove_watch does:

	for (ldata = listeners; ldata; ldata = ldata->next) {
		data = ldata->data;

		cb = filter_data_find_callback(data, id);
		if (cb) {
			filter_data_remove_callback(data, cb);
			return TRUE;
		}
	}

So as per current implementation the listeners are global, you can
argue it is not very efficient and can cause extra iteration if there
are more connections but overall this seems to pay off to avoid having
the caller to manage connection. Btw this is the same approach glib
uses regarding io watches, they are global so you don't have to store
the GIOChannel together with the id to be able to remove them as the
g_source_remove only takes the id.

-- 
Luiz Augusto von Dentz