Commands always return 0 as exit status in non-interactive mode. This
patch set makes them return the exit_status variable as exit status.
Note:
This patch set does not include the following fix for meshctl, due to
conflict with Inga's patch ("mesh/meshctl: Exit cleanly if start up
fails")[1]. I will send a rebased fix after merging Inga's patch to
BlueZ master branch.
[1]: https://www.spinics.net/lists/linux-bluetooth/msg75020.html
ERAMOTO Masaya (6):
shared/shell: Return exit status to caller
client: Return exit status for non-interactive
tools/bluetooth-player: Return exit status for non-interactive
tools/obexctl: Return exit status for non-interactive
tools/btmgmt: Return exit status for non-interactive
shared/mainloop: Fix overwriting exit status
client/main.c | 5 +++--
src/shared/mainloop.c | 4 +---
src/shared/shell.c | 7 +++++--
src/shared/shell.h | 2 +-
tools/bluetooth-player.c | 5 +++--
tools/btmgmt.c | 2 +-
tools/obexctl.c | 5 +++--
7 files changed, 17 insertions(+), 13 deletions(-)
---
mesh/main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mesh/main.c b/mesh/main.c
index d991c9f8c..c79c44bd8 100644
--- a/mesh/main.c
+++ b/mesh/main.c
@@ -1904,6 +1904,7 @@ static void client_ready(GDBusClient *client, void *user_data)
int main(int argc, char *argv[])
{
GDBusClient *client;
+ int status;
int len;
int extra;
@@ -1989,7 +1990,7 @@ int main(int argc, char *argv[])
if (!onoff_client_init(PRIMARY_ELEMENT_IDX))
g_printerr("Failed to initialize mesh generic On/Off client\n");
- bt_shell_run();
+ status = bt_shell_run();
g_dbus_client_unref(client);
@@ -2001,5 +2002,5 @@ int main(int argc, char *argv[])
g_list_free(service_list);
g_list_free_full(ctrl_list, proxy_leak);
- return 0;
+ return status;
}
--
2.14.1
Hi Eramoto,
On Tue, Mar 27, 2018 at 8:38 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Hi Luiz,
>
> On 03/26/2018 10:26 PM, Luiz Augusto von Dentz wrote:
>> Hi Eramoto,
>>
>> On Mon, Mar 26, 2018 at 11:25 AM, ERAMOTO Masaya
>> <[email protected]> wrote:
>>> Commands always return 0 as exit status in non-interactive mode. This
>>> patch set makes them return the exit_status variable as exit status.
>>>
>>> Note:
>>> This patch set does not include the following fix for meshctl, due to
>>> conflict with Inga's patch ("mesh/meshctl: Exit cleanly if start up
>>> fails")[1]. I will send a rebased fix after merging Inga's patch to
>>> BlueZ master branch.
>>>
>>> [1]: https://www.spinics.net/lists/linux-bluetooth/msg75020.html
>>
>> So here is the concern the exit status or the memory leaks, because
>> the latter is not really a problem if the process will be terminated
>> all the resources will be freed anyway so calling bt_shell_cleanup
>> seems pointless in this regard. The status seems alright as I can see
>> it being used with scripts that may parse the exit status.
>
> bluetoothctl etc. always return 0 except calling exit(), since
> bt_shell_run() does not return value even if a subcommand fails.
> Because it is hard for scripts to handle them whose exit status is 0
> if they hit any unexpected problem, they are not convenient on
> non-interactive mode.
>
> This patch set makes bt_shell_run() return EXIT_SUCCESS/EXIT_FAILURE or
> error code, and bluetoothctl etc. be able to pass the status to scripts.
Applied, thanks.
Hi Luiz,
On 03/26/2018 10:26 PM, Luiz Augusto von Dentz wrote:
> Hi Eramoto,
>
> On Mon, Mar 26, 2018 at 11:25 AM, ERAMOTO Masaya
> <[email protected]> wrote:
>> Commands always return 0 as exit status in non-interactive mode. This
>> patch set makes them return the exit_status variable as exit status.
>>
>> Note:
>> This patch set does not include the following fix for meshctl, due to
>> conflict with Inga's patch ("mesh/meshctl: Exit cleanly if start up
>> fails")[1]. I will send a rebased fix after merging Inga's patch to
>> BlueZ master branch.
>>
>> [1]: https://www.spinics.net/lists/linux-bluetooth/msg75020.html
>
> So here is the concern the exit status or the memory leaks, because
> the latter is not really a problem if the process will be terminated
> all the resources will be freed anyway so calling bt_shell_cleanup
> seems pointless in this regard. The status seems alright as I can see
> it being used with scripts that may parse the exit status.
bluetoothctl etc. always return 0 except calling exit(), since
bt_shell_run() does not return value even if a subcommand fails.
Because it is hard for scripts to handle them whose exit status is 0
if they hit any unexpected problem, they are not convenient on
non-interactive mode.
This patch set makes bt_shell_run() return EXIT_SUCCESS/EXIT_FAILURE or
error code, and bluetoothctl etc. be able to pass the status to scripts.
Regards,
Eramoto
>
>>
>> ERAMOTO Masaya (6):
>> shared/shell: Return exit status to caller
>> client: Return exit status for non-interactive
>> tools/bluetooth-player: Return exit status for non-interactive
>> tools/obexctl: Return exit status for non-interactive
>> tools/btmgmt: Return exit status for non-interactive
>> shared/mainloop: Fix overwriting exit status
>>
>> client/main.c | 5 +++--
>> src/shared/mainloop.c | 4 +---
>> src/shared/shell.c | 7 +++++--
>> src/shared/shell.h | 2 +-
>> tools/bluetooth-player.c | 5 +++--
>> tools/btmgmt.c | 2 +-
>> tools/obexctl.c | 5 +++--
>> 7 files changed, 17 insertions(+), 13 deletions(-)
>>
>>
>> ---
>> mesh/main.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/mesh/main.c b/mesh/main.c
>> index d991c9f8c..c79c44bd8 100644
>> --- a/mesh/main.c
>> +++ b/mesh/main.c
>> @@ -1904,6 +1904,7 @@ static void client_ready(GDBusClient *client, void *user_data)
>> int main(int argc, char *argv[])
>> {
>> GDBusClient *client;
>> + int status;
>> int len;
>> int extra;
>>
>> @@ -1989,7 +1990,7 @@ int main(int argc, char *argv[])
>> if (!onoff_client_init(PRIMARY_ELEMENT_IDX))
>> g_printerr("Failed to initialize mesh generic On/Off client\n");
>>
>> - bt_shell_run();
>> + status = bt_shell_run();
>>
>> g_dbus_client_unref(client);
>>
>> @@ -2001,5 +2002,5 @@ int main(int argc, char *argv[])
>> g_list_free(service_list);
>> g_list_free_full(ctrl_list, proxy_leak);
>>
>> - return 0;
>> + return status;
>> }
>> --
>> 2.14.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
Hi Eramoto,
On Mon, Mar 26, 2018 at 11:25 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Commands always return 0 as exit status in non-interactive mode. This
> patch set makes them return the exit_status variable as exit status.
>
> Note:
> This patch set does not include the following fix for meshctl, due to
> conflict with Inga's patch ("mesh/meshctl: Exit cleanly if start up
> fails")[1]. I will send a rebased fix after merging Inga's patch to
> BlueZ master branch.
>
> [1]: https://www.spinics.net/lists/linux-bluetooth/msg75020.html
So here is the concern the exit status or the memory leaks, because
the latter is not really a problem if the process will be terminated
all the resources will be freed anyway so calling bt_shell_cleanup
seems pointless in this regard. The status seems alright as I can see
it being used with scripts that may parse the exit status.
>
> ERAMOTO Masaya (6):
> shared/shell: Return exit status to caller
> client: Return exit status for non-interactive
> tools/bluetooth-player: Return exit status for non-interactive
> tools/obexctl: Return exit status for non-interactive
> tools/btmgmt: Return exit status for non-interactive
> shared/mainloop: Fix overwriting exit status
>
> client/main.c | 5 +++--
> src/shared/mainloop.c | 4 +---
> src/shared/shell.c | 7 +++++--
> src/shared/shell.h | 2 +-
> tools/bluetooth-player.c | 5 +++--
> tools/btmgmt.c | 2 +-
> tools/obexctl.c | 5 +++--
> 7 files changed, 17 insertions(+), 13 deletions(-)
>
>
> ---
> mesh/main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mesh/main.c b/mesh/main.c
> index d991c9f8c..c79c44bd8 100644
> --- a/mesh/main.c
> +++ b/mesh/main.c
> @@ -1904,6 +1904,7 @@ static void client_ready(GDBusClient *client, void *user_data)
> int main(int argc, char *argv[])
> {
> GDBusClient *client;
> + int status;
> int len;
> int extra;
>
> @@ -1989,7 +1990,7 @@ int main(int argc, char *argv[])
> if (!onoff_client_init(PRIMARY_ELEMENT_IDX))
> g_printerr("Failed to initialize mesh generic On/Off client\n");
>
> - bt_shell_run();
> + status = bt_shell_run();
>
> g_dbus_client_unref(client);
>
> @@ -2001,5 +2002,5 @@ int main(int argc, char *argv[])
> g_list_free(service_list);
> g_list_free_full(ctrl_list, proxy_leak);
>
> - return 0;
> + return status;
> }
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Since mainloop_run() overwrites the exit_status variable even if having
called mainloop_{quit,exit_*}(), such as in case executing btmgmt with
an invalid command, it is initialized in declaration.
---
src/shared/mainloop.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/shared/mainloop.c b/src/shared/mainloop.c
index 09c46a79a..e6ab9c43d 100644
--- a/src/shared/mainloop.c
+++ b/src/shared/mainloop.c
@@ -42,7 +42,7 @@
static int epoll_fd;
static int epoll_terminate;
-static int exit_status;
+static int exit_status = EXIT_SUCCESS;
struct mainloop_data {
int fd;
@@ -141,8 +141,6 @@ int mainloop_run(void)
}
}
- exit_status = EXIT_SUCCESS;
-
while (!epoll_terminate) {
struct epoll_event events[MAX_EPOLL_EVENTS];
int n, nfds;
--
2.14.1
---
tools/btmgmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/btmgmt.c b/tools/btmgmt.c
index 122c46d0d..f60bb8b53 100644
--- a/tools/btmgmt.c
+++ b/tools/btmgmt.c
@@ -4416,7 +4416,7 @@ int main(int argc, char *argv[])
bt_shell_attach(fileno(stdin));
update_prompt(mgmt_index);
- bt_shell_run();
+ status = bt_shell_run();
mgmt_cancel_all(mgmt);
mgmt_unregister_all(mgmt);
--
2.14.1
---
tools/obexctl.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/obexctl.c b/tools/obexctl.c
index 3360972bd..f839ab0bd 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -2145,6 +2145,7 @@ static void property_changed(GDBusProxy *proxy, const char *name,
int main(int argc, char *argv[])
{
GDBusClient *client;
+ int status;
bt_shell_init(argc, argv, NULL);
bt_shell_set_menu(&main_menu);
@@ -2161,11 +2162,11 @@ int main(int argc, char *argv[])
g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
property_changed, NULL);
- bt_shell_run();
+ status = bt_shell_run();
g_dbus_client_unref(client);
dbus_connection_unref(dbus_conn);
- return 0;
+ return status;
}
--
2.14.1
---
tools/bluetooth-player.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/bluetooth-player.c b/tools/bluetooth-player.c
index 441a934cc..ccb0b5ee5 100644
--- a/tools/bluetooth-player.c
+++ b/tools/bluetooth-player.c
@@ -1119,6 +1119,7 @@ static void property_changed(GDBusProxy *proxy, const char *name,
int main(int argc, char *argv[])
{
GDBusClient *client;
+ int status;
bt_shell_init(argc, argv, NULL);
bt_shell_set_menu(&main_menu);
@@ -1134,11 +1135,11 @@ int main(int argc, char *argv[])
g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
property_changed, NULL);
- bt_shell_run();
+ status = bt_shell_run();
g_dbus_client_unref(client);
dbus_connection_unref(dbus_conn);
- return 0;
+ return status;
}
--
2.14.1
---
client/main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/client/main.c b/client/main.c
index 584217a82..7a100b552 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2532,6 +2532,7 @@ static void client_ready(GDBusClient *client, void *user_data)
int main(int argc, char *argv[])
{
GDBusClient *client;
+ int status;
bt_shell_init(argc, argv, &opt);
bt_shell_set_menu(&main_menu);
@@ -2559,7 +2560,7 @@ int main(int argc, char *argv[])
g_dbus_client_set_ready_watch(client, client_ready, NULL);
- bt_shell_run();
+ status = bt_shell_run();
g_dbus_client_unref(client);
@@ -2569,5 +2570,5 @@ int main(int argc, char *argv[])
g_free(auto_register_agent);
- return 0;
+ return status;
}
--
2.14.1
---
src/shared/shell.c | 7 +++++--
src/shared/shell.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/shared/shell.c b/src/shared/shell.c
index 33bc0d980..b503c6628 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -993,17 +993,20 @@ static void env_destroy(void *data)
free(env);
}
-void bt_shell_run(void)
+int bt_shell_run(void)
{
struct io *signal;
+ int status;
signal = setup_signalfd();
- mainloop_run();
+ status = mainloop_run();
io_destroy(signal);
bt_shell_cleanup();
+
+ return status;
}
void bt_shell_cleanup(void)
diff --git a/src/shared/shell.h b/src/shared/shell.h
index 33c31f35b..8b7cb7f30 100644
--- a/src/shared/shell.h
+++ b/src/shared/shell.h
@@ -66,7 +66,7 @@ struct bt_shell_opt {
void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt);
-void bt_shell_run(void);
+int bt_shell_run(void);
void bt_shell_quit(int status);
void bt_shell_noninteractive_quit(int status);
--
2.14.1