Hi Luiz,
Thank you for the patch. However I see that the patch doesn't address
the case where start discovery happens during stop discovery (after
stop_discovery but before stop_discovery_complete).
On Sun, Sep 10, 2017 at 12:24 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi,
>
> On Thu, Sep 7, 2017 at 2:59 PM, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>
>> 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
>
> Could you guys please check if this does address the problem you are having?
>
>
> --
> Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
