All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID
@ 2020-04-26 21:02 Pali Rohár
  2020-04-27 16:37 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Pali Rohár @ 2020-04-26 21:02 UTC (permalink / raw)
  To: linux-bluetooth

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;
 	case BTD_SERVICE_STATE_CONNECTING:
 	case BTD_SERVICE_STATE_CONNECTED:
 		break;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID
  2020-04-26 21:02 [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID Pali Rohár
@ 2020-04-27 16:37 ` Luiz Augusto von Dentz
  2020-04-27 18:07   ` Pali Rohár
  0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2020-04-27 16:37 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

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.

>         case BTD_SERVICE_STATE_CONNECTING:
>         case BTD_SERVICE_STATE_CONNECTED:
>                 break;
> --
> 2.20.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID
  2020-04-27 16:37 ` Luiz Augusto von Dentz
@ 2020-04-27 18:07   ` Pali Rohár
  2020-04-29  0:37     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Pali Rohár @ 2020-04-27 18:07 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID
  2020-04-27 18:07   ` Pali Rohár
@ 2020-04-29  0:37     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2020-04-29  0:37 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Mon, Apr 27, 2020 at 11:07 AM Pali Rohár <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á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

Applied, thanks.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-04-29  0:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-26 21:02 [PATCH] device: Fix calling DisconnectProfile DBus call with disconnected profile UUID Pali Rohár
2020-04-27 16:37 ` Luiz Augusto von Dentz
2020-04-27 18:07   ` Pali Rohár
2020-04-29  0:37     ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.