Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200426210202.700-1-pali@kernel.org> <CABBYNZKJUw2PL3vK5xLq9RDQ+V8sW4D9dP-Eh9m=0SXgChcagQ@mail.gmail.com>
 <20200427180707.62igtkaz255viyj6@pali>
In-Reply-To: <20200427180707.62igtkaz255viyj6@pali>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 28 Apr 2020 17:37:11 -0700
Message-ID: <CABBYNZJVTV4KjYWm=L3U26YyQzaMe+ntq=MzHtsW_nGRp+j0iw@mail.gmail.com>
Subject: Re: [PATCH] device: Fix calling DisconnectProfile DBus call with
 disconnected profile UUID
To:     =?UTF-8?Q?Pali_Roh=C3=A1r?= <pali@kernel.org>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Pali,

On Mon, Apr 27, 2020 at 11:07 AM Pali Roh=C3=A1r <pali@kernel.org> wrote:
>
> 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=C3=A1r <pali@kernel.org> wrote=
:
> > >
> > > When DisconnectProfile is called with disconnected UUID, bluetooth da=
emon
> > > often returned error "Operation already in progress". This change fix=
ed 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(DBusConn=
ection *conn, DBusMessage *msg,
> > >         if (dev->disconnect)
> > >                 return btd_error_in_progress(msg);
> > >
> > > +       if (btd_service_get_state(service) =3D=3D BTD_SERVICE_STATE_D=
ISCONNECTED)
> > > +               return dbus_message_new_method_return(msg);
> > > +
> > >         dev->disconnect =3D dbus_message_ref(msg);
> > >
> > >         err =3D btd_service_disconnect(service);
> > > @@ -2065,6 +2068,8 @@ static DBusMessage *disconnect_profile(DBusConn=
ection *conn, DBusMessage *msg,
> > >
> > >         if (err =3D=3D -ENOTSUP)
> > >                 return btd_error_not_supported(msg);
> > > +       else if (err =3D=3D -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 *se=
rvice)
> > >         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

Applied, thanks.

--=20
Luiz Augusto von Dentz