2020-06-04 23:25:45

by Pali Rohár

[permalink] [raw]
Subject: [PATCH] sap: Improve error messages

When bluetoohd daemon is starting, it prints following error messages:

bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver initialization failed.
bluetoothd[19117]: sap-server: Operation not permitted (1)

Initialization is failing because sap server is enabled only when
bluetoothd daemon is started with --experimental option.

And "Operation not permitted" is result of returning error code -1.

This patch improves error messages. When --experimental option is not used
then bluetoothd prints more explaining error message. And in case function
sap_init() fails then -EOPNOTSUPP "Operation not supported" is returned
instead of -EPERM (-1).
---
profiles/sap/server.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/profiles/sap/server.c b/profiles/sap/server.c
index 5de682a33..99ff80297 100644
--- a/profiles/sap/server.c
+++ b/profiles/sap/server.c
@@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
GIOChannel *io;
struct sap_server *server;

+ if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
+ error("Sap driver is disabled without --experimental option");
+ return -EOPNOTSUPP;
+ }
+
if (sap_init() < 0) {
error("Sap driver initialization failed.");
- return -1;
+ return -EOPNOTSUPP;
}

record = create_sap_record(SAP_SERVER_CHANNEL);
--
2.20.1


2020-06-05 00:13:17

by bluez.test.bot

[permalink] [raw]
Subject: RE: sap: Improve error messages


This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
checkpatch Failed

Outputs:
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#8:
bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver initialization failed.

- total: 0 errors, 1 warnings, 15 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.



---
Regards,
Linux Bluetooth

2020-06-05 00:13:22

by bluez.test.bot

[permalink] [raw]
Subject: RE: sap: Improve error messages


This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
checkgitlint Failed

Outputs:
5: B1 Line exceeds max length (96>80): "bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver initialization failed."



---
Regards,
Linux Bluetooth

2020-06-05 00:15:00

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] sap: Improve error messages

Hi Pali,

On Thu, Jun 4, 2020 at 4:27 PM Pali Rohár <[email protected]> wrote:
>
> When bluetoohd daemon is starting, it prints following error messages:
>
> bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver initialization failed.
> bluetoothd[19117]: sap-server: Operation not permitted (1)
>
> Initialization is failing because sap server is enabled only when
> bluetoothd daemon is started with --experimental option.
>
> And "Operation not permitted" is result of returning error code -1.
>
> This patch improves error messages. When --experimental option is not used
> then bluetoothd prints more explaining error message. And in case function
> sap_init() fails then -EOPNOTSUPP "Operation not supported" is returned
> instead of -EPERM (-1).
> ---
> profiles/sap/server.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/sap/server.c b/profiles/sap/server.c
> index 5de682a33..99ff80297 100644
> --- a/profiles/sap/server.c
> +++ b/profiles/sap/server.c
> @@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
> GIOChannel *io;
> struct sap_server *server;
>
> + if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
> + error("Sap driver is disabled without --experimental option");
> + return -EOPNOTSUPP;
> + }
> +
> if (sap_init() < 0) {
> error("Sap driver initialization failed.");
> - return -1;
> + return -EOPNOTSUPP;
> }
>
> record = create_sap_record(SAP_SERVER_CHANNEL);
> --
> 2.20.1

We might as well introduce a experimental flag for the plugin so it
just don't load it if experimental flag is disabled.

--
Luiz Augusto von Dentz

2020-06-12 13:16:12

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] sap: Improve error messages

On Friday 05 June 2020 00:14:12 Luiz Augusto von Dentz wrote:
> Hi Pali,
>
> On Thu, Jun 4, 2020 at 4:27 PM Pali Rohár <[email protected]> wrote:
> >
> > When bluetoohd daemon is starting, it prints following error messages:
> >
> > bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver initialization failed.
> > bluetoothd[19117]: sap-server: Operation not permitted (1)
> >
> > Initialization is failing because sap server is enabled only when
> > bluetoothd daemon is started with --experimental option.
> >
> > And "Operation not permitted" is result of returning error code -1.
> >
> > This patch improves error messages. When --experimental option is not used
> > then bluetoothd prints more explaining error message. And in case function
> > sap_init() fails then -EOPNOTSUPP "Operation not supported" is returned
> > instead of -EPERM (-1).
> > ---
> > profiles/sap/server.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/profiles/sap/server.c b/profiles/sap/server.c
> > index 5de682a33..99ff80297 100644
> > --- a/profiles/sap/server.c
> > +++ b/profiles/sap/server.c
> > @@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
> > GIOChannel *io;
> > struct sap_server *server;
> >
> > + if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
> > + error("Sap driver is disabled without --experimental option");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > if (sap_init() < 0) {
> > error("Sap driver initialization failed.");
> > - return -1;
> > + return -EOPNOTSUPP;
> > }
> >
> > record = create_sap_record(SAP_SERVER_CHANNEL);
> > --
> > 2.20.1
>
> We might as well introduce a experimental flag for the plugin so it
> just don't load it if experimental flag is disabled.

Probably yet. But returning -1 (-EPERM) should be also fixed.

2020-06-15 09:49:13

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] sap: Improve error messages

Hi,

On Friday, 5 June 2020 01:24:33 CEST Pali Roh?r wrote:
> When bluetoohd daemon is starting, it prints following error messages:
>
> bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver
> initialization failed. bluetoothd[19117]: sap-server: Operation not
> permitted (1)
>
> Initialization is failing because sap server is enabled only when
> bluetoothd daemon is started with --experimental option.
>
> And "Operation not permitted" is result of returning error code -1.
>
> This patch improves error messages. When --experimental option is not used
> then bluetoothd prints more explaining error message. And in case function
> sap_init() fails then -EOPNOTSUPP "Operation not supported" is returned
> instead of -EPERM (-1).
> ---
> profiles/sap/server.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/sap/server.c b/profiles/sap/server.c
> index 5de682a33..99ff80297 100644
> --- a/profiles/sap/server.c
> +++ b/profiles/sap/server.c
> @@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
> GIOChannel *io;
> struct sap_server *server;
>
> + if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
> + error("Sap driver is disabled without --experimental
option");
> + return -EOPNOTSUPP;
> + }
> +

Maybe just make sap_init() fail if experimental is not enabled in sap-dummy.c?

This driver is usable only for profile qualification tests and nothing more.
And TBH I'm not sure why distros are enabling SAP in first place...

> if (sap_init() < 0) {
> error("Sap driver initialization failed.");
> - return -1;
> + return -EOPNOTSUPP;
> }
>
> record = create_sap_record(SAP_SERVER_CHANNEL);


--
pozdrawiam
Szymon Janc


2020-07-16 14:42:19

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] sap: Improve error messages

On Monday 15 June 2020 11:48:20 Szymon Janc wrote:
> Hi,
>
> On Friday, 5 June 2020 01:24:33 CEST Pali Rohár wrote:
> > When bluetoohd daemon is starting, it prints following error messages:
> >
> > bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver
> > initialization failed. bluetoothd[19117]: sap-server: Operation not
> > permitted (1)
> >
> > Initialization is failing because sap server is enabled only when
> > bluetoothd daemon is started with --experimental option.
> >
> > And "Operation not permitted" is result of returning error code -1.
> >
> > This patch improves error messages. When --experimental option is not used
> > then bluetoothd prints more explaining error message. And in case function
> > sap_init() fails then -EOPNOTSUPP "Operation not supported" is returned
> > instead of -EPERM (-1).
> > ---
> > profiles/sap/server.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/profiles/sap/server.c b/profiles/sap/server.c
> > index 5de682a33..99ff80297 100644
> > --- a/profiles/sap/server.c
> > +++ b/profiles/sap/server.c
> > @@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
> > GIOChannel *io;
> > struct sap_server *server;
> >
> > + if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
> > + error("Sap driver is disabled without --experimental
> option");
> > + return -EOPNOTSUPP;
> > + }
> > +
>
> Maybe just make sap_init() fail if experimental is not enabled in sap-dummy.c?

I guess this is what is already happening. But failure of sap_init()
means that bluetoothd daemon prints error message that initialization
failed as I wrote in commit message.

Therefore I added another check for experimental flag with printing
different error message which contains information why it failed.

> This driver is usable only for profile qualification tests and nothing more.
> And TBH I'm not sure why distros are enabling SAP in first place...
>
> > if (sap_init() < 0) {
> > error("Sap driver initialization failed.");
> > - return -1;
> > + return -EOPNOTSUPP;
> > }
> >
> > record = create_sap_record(SAP_SERVER_CHANNEL);
>
>
> --
> pozdrawiam
> Szymon Janc
>
>

2020-09-29 21:36:48

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] sap: Improve error messages

On Thursday 16 July 2020 16:40:08 Pali Rohár wrote:
> On Monday 15 June 2020 11:48:20 Szymon Janc wrote:
> > Hi,
> >
> > On Friday, 5 June 2020 01:24:33 CEST Pali Rohár wrote:
> > > When bluetoohd daemon is starting, it prints following error messages:
> > >
> > > bluetoothd[19117]: profiles/sap/server.c:sap_server_register() Sap driver
> > > initialization failed. bluetoothd[19117]: sap-server: Operation not
> > > permitted (1)
> > >
> > > Initialization is failing because sap server is enabled only when
> > > bluetoothd daemon is started with --experimental option.
> > >
> > > And "Operation not permitted" is result of returning error code -1.
> > >
> > > This patch improves error messages. When --experimental option is not used
> > > then bluetoothd prints more explaining error message. And in case function
> > > sap_init() fails then -EOPNOTSUPP "Operation not supported" is returned
> > > instead of -EPERM (-1).
> > > ---
> > > profiles/sap/server.c | 7 ++++++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/profiles/sap/server.c b/profiles/sap/server.c
> > > index 5de682a33..99ff80297 100644
> > > --- a/profiles/sap/server.c
> > > +++ b/profiles/sap/server.c
> > > @@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
> > > GIOChannel *io;
> > > struct sap_server *server;
> > >
> > > + if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
> > > + error("Sap driver is disabled without --experimental
> > option");
> > > + return -EOPNOTSUPP;
> > > + }
> > > +
> >
> > Maybe just make sap_init() fail if experimental is not enabled in sap-dummy.c?
>
> I guess this is what is already happening. But failure of sap_init()
> means that bluetoothd daemon prints error message that initialization
> failed as I wrote in commit message.
>
> Therefore I added another check for experimental flag with printing
> different error message which contains information why it failed.

Szymon, do you need something more?

> > This driver is usable only for profile qualification tests and nothing more.
> > And TBH I'm not sure why distros are enabling SAP in first place...
> >
> > > if (sap_init() < 0) {
> > > error("Sap driver initialization failed.");
> > > - return -1;
> > > + return -EOPNOTSUPP;
> > > }
> > >
> > > record = create_sap_record(SAP_SERVER_CHANNEL);
> >
> >
> > --
> > pozdrawiam
> > Szymon Janc
> >
> >