Skip to contentSkip to conversation

sap: Improve error messages

PATCH In review Pali Rohár 4 participants 8 messages Jun 4, 2020 – Sep 29, 2020 Bluetooth drivers

lore
Share & tools
lore

Pali Rohár

author
#0 lore raw

PATCH 1/1 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(-)
profiles/sap/server.c +6 -1
index 5de682a33..99ff80297 100644
@@ -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);
Signature · 1 line
2.20.1
#1 lore raw

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

#2 lore raw
In reply to Pali Rohár

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

maintainer
#3 lore raw
In reply to Pali Rohár

Hi Pali,

Quoted 40 lines · 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(-)
profiles/sap/server.c +6 -2
index 5de682a33..99ff80297 100644
@@ -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);
--
Quoted 1 line

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.

Signature · 1 line
Luiz Augusto von Dentz
author
#4 lore raw
Quoted 41 lines · On Friday 05 June 2020 00:14:12 Luiz Augusto von Dentz wrote:

Hi Pali,

Quoted 38 lines · 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(-)
profiles/sap/server.c +6 -1
index 5de682a33..99ff80297 100644
@@ -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;
        }
Quoted 6 lines
Quoted 3 lines
profiles/sap/server.c +0 -1
        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.

#5 lore raw
In reply to Pali Rohár

Hi,

Quoted 24 lines · 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(-)
profiles/sap/server.c +0 -0
index 5de682a33..99ff80297 100644

profiles/sap/server.c int sap_server_register(struct btd_adapter *adapter) · line 1357

Quoted 5 lines
profiles/sap/server.c +2 -0
@@ -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");

profiles/sap/server.c int sap_server_register(struct btd_adapter *adapter) · line 1360

Quoted 3 lines
profiles/sap/server.c +3 -0
+		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...

Quoted 7 lines
profiles/sap/server.c +1 -1
 	if (sap_init() < 0) {
 		error("Sap driver initialization failed.");
-		return -1;
+		return -EOPNOTSUPP;
 	}
 
 	record = create_sap_record(SAP_SERVER_CHANNEL);
Signature · 2 lines
pozdrawiam
Szymon Janc
author
#6 lore raw
Quoted 30 lines · On Monday 15 June 2020 11:48:20 Szymon Janc wrote:

Hi,

Quoted 27 lines · 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(-)
profiles/sap/server.c +0 -0
index 5de682a33..99ff80297 100644
@@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
 	GIOChannel *io;
 	struct sap_server *server;
Quoted 8 lines

profiles/sap/server.c int sap_server_register(struct btd_adapter *adapter) · line 1357

Quoted 2 lines
profiles/sap/server.c +2 -0
+	if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
+		error("Sap driver is disabled without --experimental

option");

profiles/sap/server.c int sap_server_register(struct btd_adapter *adapter) · line 1360

Quoted 3 lines
profiles/sap/server.c +3 -0
+		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.

Quoted 17 lines

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

Quoted 7 lines
profiles/sap/server.c +1 -1
 	if (sap_init() < 0) {
 		error("Sap driver initialization failed.");
-		return -1;
+		return -EOPNOTSUPP;
 	}
 
 	record = create_sap_record(SAP_SERVER_CHANNEL);
Signature · 2 lines
pozdrawiam
Szymon Janc
author
#7 lore raw
Quoted 40 lines · On Thursday 16 July 2020 16:40:08 Pali Rohár wrote:
Quoted 30 lines · On Monday 15 June 2020 11:48:20 Szymon Janc wrote:

Hi,

Quoted 27 lines · 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(-)
profiles/sap/server.c +0 -0
index 5de682a33..99ff80297 100644
@@ -1353,9 +1353,14 @@ int sap_server_register(struct btd_adapter *adapter)
 	GIOChannel *io;
 	struct sap_server *server;
Quoted 8 lines

profiles/sap/server.c int sap_server_register(struct btd_adapter *adapter) · line 1357

Quoted 2 lines
profiles/sap/server.c +2 -0
+	if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
+		error("Sap driver is disabled without --experimental

option");

profiles/sap/server.c int sap_server_register(struct btd_adapter *adapter) · line 1360

Quoted 3 lines
profiles/sap/server.c +3 -0
+		return -EOPNOTSUPP;
+	}
+

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

Quoted 6 lines

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?

Quoted 17 lines
Quoted 17 lines

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

Quoted 7 lines
profiles/sap/server.c +1 -1
 	if (sap_init() < 0) {
 		error("Sap driver initialization failed.");
-		return -1;
+		return -EOPNOTSUPP;
 	}
 
 	record = create_sap_record(SAP_SERVER_CHANNEL);
Signature · 2 lines
pozdrawiam
Szymon Janc