Re: [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID

["Pali Rohár" <pali@kernel.org>]

On Monday 27 April 2020 09:37:39 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Sun, Apr 26, 2020 at 2:05 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > When DisconnectProfile is called with disconnected UUID, bluetooth daemon
> > often returned error "Operation already in progress". This change fixed it
> > and no error message is returned for this case.
> > ---
> >  src/device.c  | 5 +++++
> >  src/service.c | 3 ++-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/device.c b/src/device.c
> > index 149c45160..229579378 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -2054,6 +2054,9 @@ static DBusMessage *disconnect_profile(DBusConnection *conn, DBusMessage *msg,
> >         if (dev->disconnect)
> >                 return btd_error_in_progress(msg);
> >
> > +       if (btd_service_get_state(service) == BTD_SERVICE_STATE_DISCONNECTED)
> > +               return dbus_message_new_method_return(msg);
> > +
> >         dev->disconnect = dbus_message_ref(msg);
> >
> >         err = btd_service_disconnect(service);
> > @@ -2065,6 +2068,8 @@ static DBusMessage *disconnect_profile(DBusConnection *conn, DBusMessage *msg,
> >
> >         if (err == -ENOTSUP)
> >                 return btd_error_not_supported(msg);
> > +       else if (err == -EALREADY)
> > +               return dbus_message_new_method_return(msg);
> >
> >         return btd_error_failed(msg, strerror(-err));
> >  }
> > diff --git a/src/service.c b/src/service.c
> > index c14ee0aca..e4d747a6e 100644
> > --- a/src/service.c
> > +++ b/src/service.c
> > @@ -281,8 +281,9 @@ int btd_service_disconnect(struct btd_service *service)
> >         case BTD_SERVICE_STATE_UNAVAILABLE:
> >                 return -EINVAL;
> >         case BTD_SERVICE_STATE_DISCONNECTED:
> > -       case BTD_SERVICE_STATE_DISCONNECTING:
> >                 return -EALREADY;
> > +       case BTD_SERVICE_STATE_DISCONNECTING:
> > +               return 0;
> 
> Have you checked if there are no side effects of this change? Since
> you have a call to btd_service_get_service I wonder if this is really
> necessary or you want to change the behavior to not return an error
> even in case of disconnecting state? But even in that case you are
> checking if for -EALREADY and returning no error.

Same logic is used in btd_service_connect() function. When service is in
disconnecting state then it will receive state change to disconnected.
So returning zero is should same as issuing disconnect request again.

> >         case BTD_SERVICE_STATE_CONNECTING:
> >         case BTD_SERVICE_STATE_CONNECTED:
> >                 break;
> > --
> > 2.20.1
> >
> 
> 
> -- 
> Luiz Augusto von Dentz