[PATCH] sap: Improve error messages

[PATCH] sap: Improve error messages

[Pali Rohár]

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

RE: sap: Improve error messages

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]


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

RE: sap: Improve error messages

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]


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

Re: [PATCH] sap: Improve error messages

[Luiz Augusto von Dentz]

Hi Pali,

On Thu, Jun 4, 2020 at 4:27 PM Pali Rohár <pali@kernel.org> 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

Re: [PATCH] sap: Improve error messages

[Pali Rohár]

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 <pali@kernel.org> 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.

Re: [PATCH] sap: Improve error messages

[Szymon Janc]

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

Re: [PATCH] sap: Improve error messages

[Pali Rohár]

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
> 
> 

Re: [PATCH] sap: Improve error messages

[Pali Rohár]

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
> > 
> >