* [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result
@ 2017-09-14 13:15 Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 2/3] adapter: Fix not waiting for stop " Luiz Augusto von Dentz
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-09-14 13:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
We should not reply until the start discovery completes otherwise
clients may attempt to stop the discovery before it even has started.
On top of this it will now block clients so they so not be able to
queue more requests.
---
src/adapter.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 8 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index a571b1870..4b9f5a1cd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -160,6 +160,7 @@ struct discovery_filter {
struct watch_client {
struct btd_adapter *adapter;
+ DBusMessage *msg;
char *owner;
guint watch;
struct discovery_filter *discovery_filter;
@@ -1421,13 +1422,17 @@ static void start_discovery_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
struct btd_adapter *adapter = user_data;
+ struct watch_client *client = adapter->discovery_list->data;
const struct mgmt_cp_start_discovery *rp = param;
+ DBusMessage *reply;
DBG("status 0x%02x", status);
if (length < sizeof(*rp)) {
btd_error(adapter->dev_id,
"Wrong size of start discovery return parameters");
+ if (client->msg)
+ goto fail;
return;
}
@@ -1440,6 +1445,14 @@ static void start_discovery_complete(uint8_t status, uint16_t length,
else
adapter->filtered_discovery = false;
+ if (client->msg) {
+ reply = g_dbus_create_reply(client->msg,
+ DBUS_TYPE_INVALID);
+ g_dbus_send_message(dbus_conn, reply);
+ dbus_message_unref(client->msg);
+ client->msg = NULL;
+ }
+
if (adapter->discovering)
return;
@@ -1449,6 +1462,15 @@ static void start_discovery_complete(uint8_t status, uint16_t length,
return;
}
+fail:
+ /* Reply with an error if the first discovery has failed */
+ if (client->msg) {
+ reply = btd_error_busy(client->msg);
+ g_dbus_send_message(dbus_conn, reply);
+ g_dbus_remove_watch(dbus_conn, client->watch);
+ return;
+ }
+
/*
* In case the restart of the discovery failed, then just trigger
* it for the next idle timeout again.
@@ -1933,7 +1955,7 @@ static bool filters_equal(struct mgmt_cp_start_service_discovery *a,
return true;
}
-static void update_discovery_filter(struct btd_adapter *adapter)
+static int update_discovery_filter(struct btd_adapter *adapter)
{
struct mgmt_cp_start_service_discovery *sd_cp;
@@ -1942,7 +1964,7 @@ static void update_discovery_filter(struct btd_adapter *adapter)
if (discovery_filter_to_mgmt_cp(adapter, &sd_cp)) {
btd_error(adapter->dev_id,
"discovery_filter_to_mgmt_cp returned error");
- return;
+ return -ENOMEM;
}
/*
@@ -1953,13 +1975,15 @@ static void update_discovery_filter(struct btd_adapter *adapter)
adapter->discovering != 0) {
DBG("filters were equal, deciding to not restart the scan.");
g_free(sd_cp);
- return;
+ return 0;
}
g_free(adapter->current_discovery_filter);
adapter->current_discovery_filter = sd_cp;
trigger_start_discovery(adapter, 0);
+
+ return -EINPROGRESS;
}
static void discovery_destroy(void *user_data)
@@ -1980,6 +2004,9 @@ static void discovery_destroy(void *user_data)
client->discovery_filter = NULL;
}
+ if (client->msg)
+ dbus_message_unref(client->msg);
+
g_free(client->owner);
g_free(client);
@@ -2087,6 +2114,7 @@ static DBusMessage *start_discovery(DBusConnection *conn,
const char *sender = dbus_message_get_sender(msg);
struct watch_client *client;
bool is_discovering;
+ int err;
DBG("sender %s", sender);
@@ -2107,12 +2135,14 @@ static DBusMessage *start_discovery(DBusConnection *conn,
* and trigger scan.
*/
if (client) {
+ if (client->msg)
+ return btd_error_busy(msg);
+
adapter->set_filter_list = g_slist_remove(
adapter->set_filter_list, client);
adapter->discovery_list = g_slist_prepend(
adapter->discovery_list, client);
- update_discovery_filter(adapter);
- return dbus_message_new_method_return(msg);
+ goto done;
}
client = g_new0(struct watch_client, 1);
@@ -2126,14 +2156,23 @@ static DBusMessage *start_discovery(DBusConnection *conn,
adapter->discovery_list = g_slist_prepend(adapter->discovery_list,
client);
+done:
/*
* Just trigger the discovery here. In case an already running
* discovery in idle phase exists, it will be restarted right
* away.
*/
- update_discovery_filter(adapter);
+ err = update_discovery_filter(adapter);
+ if (!err)
+ return dbus_message_new_method_return(msg);
- return dbus_message_new_method_return(msg);
+ /* If the discovery has to be started wait it complete to reply */
+ if (err == -EINPROGRESS) {
+ client->msg = dbus_message_ref(msg);
+ return NULL;
+ }
+
+ return btd_error_failed(msg, strerror(-err));
}
static bool parse_uuids(DBusMessageIter *value, struct discovery_filter *filter)
@@ -2984,7 +3023,7 @@ static DBusMessage *get_discovery_filters(DBusConnection *conn,
}
static const GDBusMethodTable adapter_methods[] = {
- { GDBUS_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
+ { GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
{ GDBUS_METHOD("SetDiscoveryFilter",
GDBUS_ARGS({ "properties", "a{sv}" }), NULL,
set_discovery_filter) },
--
2.13.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 2/3] adapter: Fix not waiting for stop discovery result
2017-09-14 13:15 [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Luiz Augusto von Dentz
@ 2017-09-14 13:15 ` Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 3/3] adapter: Refactor code around discovery Luiz Augusto von Dentz
2017-09-15 0:57 ` [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Vinicius Costa Gomes
2 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-09-14 13:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
We should not reply until the stop discovery completes otherwise
clients may attempt to start the discovery before it even has stopped.
On top of this it will now block clients so they so not be able to
queue more requests.
---
src/adapter.c | 214 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 113 insertions(+), 101 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 4b9f5a1cd..872167c73 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1712,34 +1712,6 @@ static void discovering_callback(uint16_t index, uint16_t length,
}
}
-static void stop_discovery_complete(uint8_t status, uint16_t length,
- const void *param, void *user_data)
-{
- struct btd_adapter *adapter = user_data;
-
- DBG("status 0x%02x", status);
-
- if (status == MGMT_STATUS_SUCCESS) {
- adapter->discovery_type = 0x00;
- adapter->discovery_enable = 0x00;
- adapter->filtered_discovery = false;
- adapter->no_scan_restart_delay = false;
- adapter->discovering = false;
- g_dbus_emit_property_changed(dbus_conn, adapter->path,
- ADAPTER_INTERFACE, "Discovering");
-
- trigger_passive_scanning(adapter);
- }
-}
-
-static int compare_sender(gconstpointer a, gconstpointer b)
-{
- const struct watch_client *client = a;
- const char *sender = b;
-
- return g_strcmp0(client->owner, sender);
-}
-
static void invalidate_rssi_and_tx_power(gpointer a)
{
struct btd_device *dev = a;
@@ -1776,6 +1748,102 @@ static gboolean remove_temp_devices(gpointer user_data)
return FALSE;
}
+static void discovery_destroy(void *user_data)
+{
+ struct watch_client *client = user_data;
+ struct btd_adapter *adapter = client->adapter;
+
+ DBG("owner %s", client->owner);
+
+ adapter->set_filter_list = g_slist_remove(adapter->set_filter_list,
+ client);
+
+ adapter->discovery_list = g_slist_remove(adapter->discovery_list,
+ client);
+
+ if (client->watch)
+ g_dbus_remove_watch(dbus_conn, client->watch);
+
+ if (client->discovery_filter) {
+ free_discovery_filter(client->discovery_filter);
+ client->discovery_filter = NULL;
+ }
+
+ if (client->msg)
+ dbus_message_unref(client->msg);
+
+ g_free(client->owner);
+ g_free(client);
+
+ /*
+ * If there are other client discoveries in progress, then leave
+ * it active. If not, then make sure to stop the restart timeout.
+ */
+ if (adapter->discovery_list)
+ return;
+
+ adapter->discovery_type = 0x00;
+
+ if (adapter->discovery_idle_timeout > 0) {
+ g_source_remove(adapter->discovery_idle_timeout);
+ adapter->discovery_idle_timeout = 0;
+ }
+
+ if (adapter->temp_devices_timeout > 0) {
+ g_source_remove(adapter->temp_devices_timeout);
+ adapter->temp_devices_timeout = 0;
+ }
+
+ discovery_cleanup(adapter);
+
+ adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
+ remove_temp_devices, adapter);
+}
+
+static void stop_discovery_complete(uint8_t status, uint16_t length,
+ const void *param, void *user_data)
+{
+ struct watch_client *client = user_data;
+ struct btd_adapter *adapter = client->adapter;
+ DBusMessage *reply;
+
+ DBG("status 0x%02x", status);
+
+ if (status != MGMT_STATUS_SUCCESS) {
+ if (client->msg) {
+ reply = btd_error_busy(client->msg);
+ g_dbus_send_message(dbus_conn, reply);
+ }
+ goto done;
+ }
+
+ if (client->msg) {
+ reply = g_dbus_create_reply(client->msg, DBUS_TYPE_INVALID);
+ g_dbus_send_message(dbus_conn, reply);
+ }
+
+ adapter->discovery_type = 0x00;
+ adapter->discovery_enable = 0x00;
+ adapter->filtered_discovery = false;
+ adapter->no_scan_restart_delay = false;
+ adapter->discovering = false;
+ g_dbus_emit_property_changed(dbus_conn, adapter->path,
+ ADAPTER_INTERFACE, "Discovering");
+
+ trigger_passive_scanning(adapter);
+
+done:
+ discovery_destroy(client);
+}
+
+static int compare_sender(gconstpointer a, gconstpointer b)
+{
+ const struct watch_client *client = a;
+ const char *sender = b;
+
+ return g_strcmp0(client->owner, sender);
+}
+
static gint g_strcmp(gconstpointer a, gconstpointer b)
{
return strcmp(a, b);
@@ -1986,55 +2054,6 @@ static int update_discovery_filter(struct btd_adapter *adapter)
return -EINPROGRESS;
}
-static void discovery_destroy(void *user_data)
-{
- struct watch_client *client = user_data;
- struct btd_adapter *adapter = client->adapter;
-
- DBG("owner %s", client->owner);
-
- adapter->set_filter_list = g_slist_remove(adapter->set_filter_list,
- client);
-
- adapter->discovery_list = g_slist_remove(adapter->discovery_list,
- client);
-
- if (client->discovery_filter) {
- free_discovery_filter(client->discovery_filter);
- client->discovery_filter = NULL;
- }
-
- if (client->msg)
- dbus_message_unref(client->msg);
-
- g_free(client->owner);
- g_free(client);
-
- /*
- * If there are other client discoveries in progress, then leave
- * it active. If not, then make sure to stop the restart timeout.
- */
- if (adapter->discovery_list)
- return;
-
- adapter->discovery_type = 0x00;
-
- if (adapter->discovery_idle_timeout > 0) {
- g_source_remove(adapter->discovery_idle_timeout);
- adapter->discovery_idle_timeout = 0;
- }
-
- if (adapter->temp_devices_timeout > 0) {
- g_source_remove(adapter->temp_devices_timeout);
- adapter->temp_devices_timeout = 0;
- }
-
- discovery_cleanup(adapter);
-
- adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
- remove_temp_devices, adapter);
-}
-
static void discovery_disconnect(DBusConnection *conn, void *user_data)
{
struct watch_client *client = user_data;
@@ -2078,7 +2097,7 @@ static void discovery_disconnect(DBusConnection *conn, void *user_data)
mgmt_send(adapter->mgmt, MGMT_OP_STOP_DISCOVERY,
adapter->dev_id, sizeof(cp), &cp,
- stop_discovery_complete, adapter, NULL);
+ stop_discovery_complete, client, NULL);
}
/*
@@ -2152,7 +2171,7 @@ static DBusMessage *start_discovery(DBusConnection *conn,
client->discovery_filter = NULL;
client->watch = g_dbus_add_disconnect_watch(dbus_conn, sender,
discovery_disconnect, client,
- discovery_destroy);
+ NULL);
adapter->discovery_list = g_slist_prepend(adapter->discovery_list,
client);
@@ -2419,7 +2438,7 @@ static DBusMessage *set_discovery_filter(DBusConnection *conn,
client->discovery_filter = discovery_filter;
client->watch = g_dbus_add_disconnect_watch(dbus_conn, sender,
discovery_disconnect, client,
- discovery_destroy);
+ NULL);
adapter->set_filter_list = g_slist_prepend(
adapter->set_filter_list, client);
@@ -2450,24 +2469,23 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
client = list->data;
- cp.type = adapter->discovery_type;
-
- /*
- * The destroy function will cleanup the client information and
- * also remove it from the list of discovery clients.
- */
- g_dbus_remove_watch(dbus_conn, client->watch);
+ if (client->msg)
+ return btd_error_busy(msg);
- if (adapter->discovery_list) {
+ if (g_slist_length(adapter->discovery_list) > 1) {
+ discovery_destroy(client);
update_discovery_filter(adapter);
return dbus_message_new_method_return(msg);
}
+ cp.type = adapter->discovery_type;
+
/*
* In the idle phase of a discovery, there is no need to stop it
* and so it is enough to send out the signal and just return.
*/
if (adapter->discovery_enable == 0x00) {
+ discovery_destroy(client);
adapter->discovering = false;
g_dbus_emit_property_changed(dbus_conn, adapter->path,
ADAPTER_INTERFACE, "Discovering");
@@ -2477,11 +2495,13 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
return dbus_message_new_method_return(msg);
}
+ client->msg = dbus_message_ref(msg);
+
mgmt_send(adapter->mgmt, MGMT_OP_STOP_DISCOVERY,
adapter->dev_id, sizeof(cp), &cp,
- stop_discovery_complete, adapter, NULL);
+ stop_discovery_complete, client, NULL);
- return dbus_message_new_method_return(msg);
+ return NULL;
}
static gboolean property_get_address(const GDBusPropertyTable *property,
@@ -3027,7 +3047,7 @@ static const GDBusMethodTable adapter_methods[] = {
{ GDBUS_METHOD("SetDiscoveryFilter",
GDBUS_ARGS({ "properties", "a{sv}" }), NULL,
set_discovery_filter) },
- { GDBUS_METHOD("StopDiscovery", NULL, NULL, stop_discovery) },
+ { GDBUS_ASYNC_METHOD("StopDiscovery", NULL, NULL, stop_discovery) },
{ GDBUS_ASYNC_METHOD("RemoveDevice",
GDBUS_ARGS({ "device", "o" }), NULL, remove_device) },
{ GDBUS_METHOD("GetDiscoveryFilters", NULL,
@@ -5941,11 +5961,7 @@ static void adapter_stop(struct btd_adapter *adapter)
client = adapter->set_filter_list->data;
- /* g_dbus_remove_watch will remove the client from the
- * adapter's list and free it using the discovery_destroy
- * function.
- */
- g_dbus_remove_watch(dbus_conn, client->watch);
+ discovery_destroy(client);
}
while (adapter->discovery_list) {
@@ -5953,11 +5969,7 @@ static void adapter_stop(struct btd_adapter *adapter)
client = adapter->discovery_list->data;
- /* g_dbus_remove_watch will remove the client from the
- * adapter's list and free it using the discovery_destroy
- * function.
- */
- g_dbus_remove_watch(dbus_conn, client->watch);
+ discovery_destroy(client);
}
adapter->filtered_discovery = false;
--
2.13.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 3/3] adapter: Refactor code around discovery
2017-09-14 13:15 [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 2/3] adapter: Fix not waiting for stop " Luiz Augusto von Dentz
@ 2017-09-14 13:15 ` Luiz Augusto von Dentz
2017-09-15 0:57 ` [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Vinicius Costa Gomes
2 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-09-14 13:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Make it reuse more code by having helpers to stop, remove and free
clients.
---
src/adapter.c | 162 +++++++++++++++++++++++++---------------------------------
1 file changed, 69 insertions(+), 93 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 872167c73..ecb7b1d87 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1720,13 +1720,6 @@ static void invalidate_rssi_and_tx_power(gpointer a)
device_set_tx_power(dev, 127);
}
-static void discovery_cleanup(struct btd_adapter *adapter)
-{
- g_slist_free_full(adapter->discovery_found,
- invalidate_rssi_and_tx_power);
- adapter->discovery_found = NULL;
-}
-
static gboolean remove_temp_devices(gpointer user_data)
{
struct btd_adapter *adapter = user_data;
@@ -1748,18 +1741,31 @@ static gboolean remove_temp_devices(gpointer user_data)
return FALSE;
}
-static void discovery_destroy(void *user_data)
+static void discovery_cleanup(struct btd_adapter *adapter)
{
- struct watch_client *client = user_data;
- struct btd_adapter *adapter = client->adapter;
+ adapter->discovery_type = 0x00;
- DBG("owner %s", client->owner);
+ if (adapter->discovery_idle_timeout > 0) {
+ g_source_remove(adapter->discovery_idle_timeout);
+ adapter->discovery_idle_timeout = 0;
+ }
- adapter->set_filter_list = g_slist_remove(adapter->set_filter_list,
- client);
+ if (adapter->temp_devices_timeout > 0) {
+ g_source_remove(adapter->temp_devices_timeout);
+ adapter->temp_devices_timeout = 0;
+ }
- adapter->discovery_list = g_slist_remove(adapter->discovery_list,
- client);
+ g_slist_free_full(adapter->discovery_found,
+ invalidate_rssi_and_tx_power);
+ adapter->discovery_found = NULL;
+
+ adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
+ remove_temp_devices, adapter);
+}
+
+static void discovery_free(void *user_data)
+{
+ struct watch_client *client = user_data;
if (client->watch)
g_dbus_remove_watch(dbus_conn, client->watch);
@@ -1774,6 +1780,21 @@ static void discovery_destroy(void *user_data)
g_free(client->owner);
g_free(client);
+}
+
+static void discovery_remove(struct watch_client *client)
+{
+ struct btd_adapter *adapter = client->adapter;
+
+ DBG("owner %s", client->owner);
+
+ adapter->set_filter_list = g_slist_remove(adapter->set_filter_list,
+ client);
+
+ adapter->discovery_list = g_slist_remove(adapter->discovery_list,
+ client);
+
+ discovery_free(client);
/*
* If there are other client discoveries in progress, then leave
@@ -1782,22 +1803,7 @@ static void discovery_destroy(void *user_data)
if (adapter->discovery_list)
return;
- adapter->discovery_type = 0x00;
-
- if (adapter->discovery_idle_timeout > 0) {
- g_source_remove(adapter->discovery_idle_timeout);
- adapter->discovery_idle_timeout = 0;
- }
-
- if (adapter->temp_devices_timeout > 0) {
- g_source_remove(adapter->temp_devices_timeout);
- adapter->temp_devices_timeout = 0;
- }
-
discovery_cleanup(adapter);
-
- adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
- remove_temp_devices, adapter);
}
static void stop_discovery_complete(uint8_t status, uint16_t length,
@@ -1833,7 +1839,7 @@ static void stop_discovery_complete(uint8_t status, uint16_t length,
trigger_passive_scanning(adapter);
done:
- discovery_destroy(client);
+ discovery_remove(client);
}
static int compare_sender(gconstpointer a, gconstpointer b)
@@ -2054,30 +2060,16 @@ static int update_discovery_filter(struct btd_adapter *adapter)
return -EINPROGRESS;
}
-static void discovery_disconnect(DBusConnection *conn, void *user_data)
+static int discovery_stop(struct watch_client *client)
{
- struct watch_client *client = user_data;
struct btd_adapter *adapter = client->adapter;
struct mgmt_cp_stop_discovery cp;
- DBG("owner %s", client->owner);
-
- adapter->set_filter_list = g_slist_remove(adapter->set_filter_list,
- client);
-
- adapter->discovery_list = g_slist_remove(adapter->discovery_list,
- client);
-
- /*
- * There is no need for extra cleanup of the client since that
- * will be done by the destroy callback.
- *
- * However in case this is the last client, the discovery in
- * the kernel needs to be disabled.
- */
- if (adapter->discovery_list) {
+ /* Check if there are more client discovering */
+ if (g_slist_next(adapter->discovery_list)) {
+ discovery_remove(client);
update_discovery_filter(adapter);
- return;
+ return 0;
}
/*
@@ -2085,12 +2077,14 @@ static void discovery_disconnect(DBusConnection *conn, void *user_data)
* and so it is enough to send out the signal and just return.
*/
if (adapter->discovery_enable == 0x00) {
+ discovery_remove(client);
adapter->discovering = false;
g_dbus_emit_property_changed(dbus_conn, adapter->path,
ADAPTER_INTERFACE, "Discovering");
trigger_passive_scanning(adapter);
- return;
+
+ return 0;
}
cp.type = adapter->discovery_type;
@@ -2098,6 +2092,17 @@ static void discovery_disconnect(DBusConnection *conn, void *user_data)
mgmt_send(adapter->mgmt, MGMT_OP_STOP_DISCOVERY,
adapter->dev_id, sizeof(cp), &cp,
stop_discovery_complete, client, NULL);
+
+ return -EINPROGRESS;
+}
+
+static void discovery_disconnect(DBusConnection *conn, void *user_data)
+{
+ struct watch_client *client = user_data;
+
+ DBG("owner %s", client->owner);
+
+ discovery_stop(client);
}
/*
@@ -2453,9 +2458,9 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
{
struct btd_adapter *adapter = user_data;
const char *sender = dbus_message_get_sender(msg);
- struct mgmt_cp_stop_discovery cp;
struct watch_client *client;
GSList *list;
+ int err;
DBG("sender %s", sender);
@@ -2472,36 +2477,15 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
if (client->msg)
return btd_error_busy(msg);
- if (g_slist_length(adapter->discovery_list) > 1) {
- discovery_destroy(client);
- update_discovery_filter(adapter);
- return dbus_message_new_method_return(msg);
- }
-
- cp.type = adapter->discovery_type;
-
- /*
- * In the idle phase of a discovery, there is no need to stop it
- * and so it is enough to send out the signal and just return.
- */
- if (adapter->discovery_enable == 0x00) {
- discovery_destroy(client);
- adapter->discovering = false;
- g_dbus_emit_property_changed(dbus_conn, adapter->path,
- ADAPTER_INTERFACE, "Discovering");
-
- trigger_passive_scanning(adapter);
-
+ err = discovery_stop(client);
+ switch (err) {
+ case 0:
return dbus_message_new_method_return(msg);
+ case -EINPROGRESS:
+ return NULL;
+ default:
+ return btd_error_failed(msg, strerror(-err));
}
-
- client->msg = dbus_message_ref(msg);
-
- mgmt_send(adapter->mgmt, MGMT_OP_STOP_DISCOVERY,
- adapter->dev_id, sizeof(cp), &cp,
- stop_discovery_complete, client, NULL);
-
- return NULL;
}
static gboolean property_get_address(const GDBusPropertyTable *property,
@@ -5956,21 +5940,13 @@ static void adapter_stop(struct btd_adapter *adapter)
cancel_passive_scanning(adapter);
- while (adapter->set_filter_list) {
- struct watch_client *client;
-
- client = adapter->set_filter_list->data;
-
- discovery_destroy(client);
- }
+ g_slist_free_full(adapter->set_filter_list, discovery_free);
+ adapter->set_filter_list = NULL;
- while (adapter->discovery_list) {
- struct watch_client *client;
+ g_slist_free_full(adapter->discovery_list, discovery_free);
+ adapter->discovery_list = NULL;
- client = adapter->discovery_list->data;
-
- discovery_destroy(client);
- }
+ discovery_cleanup(adapter);
adapter->filtered_discovery = false;
adapter->no_scan_restart_delay = false;
--
2.13.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result
2017-09-14 13:15 [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 2/3] adapter: Fix not waiting for stop " Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 3/3] adapter: Refactor code around discovery Luiz Augusto von Dentz
@ 2017-09-15 0:57 ` Vinicius Costa Gomes
2017-09-15 7:44 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 5+ messages in thread
From: Vinicius Costa Gomes @ 2017-09-15 0:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
Hi Luiz,
Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> We should not reply until the start discovery completes otherwise
> clients may attempt to stop the discovery before it even has started.
>
> On top of this it will now block clients so they so not be able to
> queue more requests.
> ---
> src/adapter.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index a571b1870..4b9f5a1cd 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -160,6 +160,7 @@ struct discovery_filter {
>
> struct watch_client {
> struct btd_adapter *adapter;
> + DBusMessage *msg;
> char *owner;
> guint watch;
> struct discovery_filter *discovery_filter;
> @@ -1421,13 +1422,17 @@ static void start_discovery_complete(uint8_t status, uint16_t length,
> const void *param, void *user_data)
> {
> struct btd_adapter *adapter = user_data;
> + struct watch_client *client = adapter->discovery_list->data;
What if the client exits the bus after issuing StartDiscovery(), but
before receiving the reply. It seems that 'adapter->discovery_list'
could be empty, no?
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result
2017-09-15 0:57 ` [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Vinicius Costa Gomes
@ 2017-09-15 7:44 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-09-15 7:44 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth@vger.kernel.org
Hi Vinicius,
On Fri, Sep 15, 2017 at 3:57 AM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi Luiz,
>
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> We should not reply until the start discovery completes otherwise
>> clients may attempt to stop the discovery before it even has started.
>>
>> On top of this it will now block clients so they so not be able to
>> queue more requests.
>> ---
>> src/adapter.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 47 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/adapter.c b/src/adapter.c
>> index a571b1870..4b9f5a1cd 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -160,6 +160,7 @@ struct discovery_filter {
>>
>> struct watch_client {
>> struct btd_adapter *adapter;
>> + DBusMessage *msg;
>> char *owner;
>> guint watch;
>> struct discovery_filter *discovery_filter;
>> @@ -1421,13 +1422,17 @@ static void start_discovery_complete(uint8_t status, uint16_t length,
>> const void *param, void *user_data)
>> {
>> struct btd_adapter *adapter = user_data;
>> + struct watch_client *client = adapter->discovery_list->data;
>
> What if the client exits the bus after issuing StartDiscovery(), but
> before receiving the reply. It seems that 'adapter->discovery_list'
> could be empty, no?
Yep, we would have to check if the client has been removed, I will add
a check for that.
--
Luiz Augusto von Dentzy
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-15 7:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-14 13:15 [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 2/3] adapter: Fix not waiting for stop " Luiz Augusto von Dentz
2017-09-14 13:15 ` [PATCH BlueZ 3/3] adapter: Refactor code around discovery Luiz Augusto von Dentz
2017-09-15 0:57 ` [PATCH BlueZ 1/3] adapter: Fix not waiting for start discovery result Vinicius Costa Gomes
2017-09-15 7:44 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.