On 03.05.2017 19:18, Luiz Augusto von Dentz wrote:
> Hi Georg,
>
> On Sat, Apr 29, 2017 at 4:01 PM, Georg Chini <georg at chini.tk> wrote:
>> On 29.04.2017 13:28, Georg Chini wrote:
>>> On 26.04.2017 14:19, Luiz Augusto von Dentz wrote:
>>>> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
>>>>
>>>> This detects if profile has already been called and we are waiting
>>>> the response.
>>>> ---
>>>> src/modules/bluetooth/backend-ofono.c | 19 ++++++++++++++++---
>>>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/modules/bluetooth/backend-ofono.c
>>>> b/src/modules/bluetooth/backend-ofono.c
>>>> index 3fbf153..a847ad0 100644
>>>> --- a/src/modules/bluetooth/backend-ofono.c
>>>> +++ b/src/modules/bluetooth/backend-ofono.c
>>>> @@ -65,6 +65,7 @@ struct hf_audio_card {
>>>> char *remote_address;
>>>> char *local_address;
>>>> + bool connecting;
>>>> int fd;
>>>> uint8_t codec;
>>>> @@ -156,12 +157,22 @@ static int
>>>> hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
>>>> pa_assert(card);
>>>> if (!optional && card->fd < 0) {
>>>> - DBusMessage *m;
>>>> + DBusMessage *m, *r;
>>>> + DBusError derr;
>>>> + if (card->connecting)
>>>> + return -1;
>>>> +
>>>> + card->connecting = true;
>>>> +
>>>> + dbus_error_init(&derr);
>>>> pa_assert_se(m = dbus_message_new_method_call(t->owner,
>>>> t->path, "org.ofono.HandsfreeAudioCard", "Connect"));
>>>> -
>>>> pa_assert_se(dbus_connection_send(pa_dbus_connection_get(card->backend->connection),
>>>> m, NULL));
>>>> + r =
>>>> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
>>>> m, -1, &derr);
>>>> + if (!r)
>>>> + return -1;
>>>> - return -1;
>>>> + if (card->connecting)
>>>> + return -1;
>>>> }
>>>> /* The correct block size should take into account the SCO MTU
>>>> from
>>>> @@ -535,6 +546,8 @@ static DBusMessage
>>>> *hf_audio_agent_new_connection(DBusConnection *c, DBusMessage
>>>> card = pa_hashmap_get(backend->cards, path);
>>>> + card->connecting = false;
>>>
>>> Should card->connecting not be set to false immediately after entering
>>> hf_audio_agent_new_connection()? If you set it here, the card will never
>>> be able to connect again if the sender was wrong or
>>> dbus_message_get_args() fails. Or do you expect there will be another
>>> call to hf_audio_agent_new_connection() in these cases.
>> Sorry, I did not see that the card is not known before that point, so that
>> you can't set the flag before.
>> But nevertheless the question remains what to do if one of the
>> two errors occurs. Maybe you could do the dbus_message_get_args()
>> first to identify the card. I don't know enough about the DBUS API,
>> if only one argument is wrong in dbus_message_get_args(), will the other
>> values be set anyway? If yes you could even try after a failure
>> if the path is set.
>
> This is the earliest we can know that the NewConnection is for the
> card connecting since there could be multiple cards connecting we do
> need dbus_message_get_args to complete, and no I don't think it can
> parse partially as the signature doesn't match, anyway that would be
> something completely wrong with ofono or something is taking its name
> and pretending to be it, so I guess we are doing the right thing here,
> though we probably need to add a card->connection = false to
> hf_audio_agent_transport_release in case it never resumes.
>
>
>
Yes, you are right. It does not help much to swap the calls. So I'm OK
with the patch.
