Hi Felipe, On Thu, Sep 8, 2016 at 6:37 PM, Felipe Ferreri Tonello <eu@xxxxxxxxxxxxxxxxx> wrote: > Hi Luiz, > > On 08/09/16 16:26, Luiz Augusto von Dentz wrote: >> Hi Felipe, >> >> On Thu, Sep 8, 2016 at 5:45 PM, Felipe Ferreri Tonello >> <eu@xxxxxxxxxxxxxxxxx> wrote: >>> Hi Luiz, >>> >>> On 08/09/16 13:38, Luiz Augusto von Dentz wrote: >>>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> >>>> >>>> The accept calback may transit the state to connected on the call itself >>>> since most of the time it is just a matter of selecting the attributes >>>> in case of GATT profiles. >>>> --- >>>> src/service.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/src/service.c b/src/service.c >>>> index f387fc4..20a41d0 100644 >>>> --- a/src/service.c >>>> +++ b/src/service.c >>>> @@ -209,7 +209,8 @@ int service_accept(struct btd_service *service) >>>> return err; >>>> >>>> done: >>>> - change_state(service, BTD_SERVICE_STATE_CONNECTING, 0); >>>> + if (service->state == BTD_SERVICE_STATE_DISCONNECTED) >>>> + change_state(service, BTD_SERVICE_STATE_CONNECTING, 0); >>> >>> This looks really hacky - I know it was like this before already. >> >> This is to avoid resetting the connection if the accept callback has >> set it already. >> >>> Why can you change the state in two different places? What if the >>> profile->accept doesn't change the state to connected? Will this change >>> to connecting and then the connected state will never be set? >> >> Going from connected to connecting is not valid, thus if the driver >> had changed the states there is nothing else to be done. > > Why do you have this done label if the driver is supposed to call > btd_service_connecting_complete() on success? >> >>> I think btd_service_connecting_complete() should always be called with >>> the same err as returned by profile->accept() >> >> Not sure what difference does this make? > > My suggestion is to handle the state based on the return value of the > driver's callback and not let the driver to change the state itself. > It's boilerplate code anyway. The driver may have to respond this asynchronously so we can't really depend on the return alone. -- 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
