| Message ID | 20200604232433.4951-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sap: Improve error messages | expand |
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
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
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.
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.
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);
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 > >
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 > > > >
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);