* [PATCH BlueZ] mesh/meshctl: Exit cleanly if start up fails
@ 2018-03-24 2:33 Inga Stotland
2018-03-26 13:31 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 4+ messages in thread
From: Inga Stotland @ 2018-03-24 2:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Inga Stotland
When failing to start meshctl due to invalid input configuration,
release bt_shell resources prior to exit.
---
mesh/main.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/mesh/main.c b/mesh/main.c
index d991c9f8c..d0f71c2d9 100644
--- a/mesh/main.c
+++ b/mesh/main.c
@@ -1930,11 +1930,11 @@ int main(int argc, char *argv[])
mesh_local_config_filename = g_malloc(len + strlen("local_node.json")
+ 2);
if (!mesh_local_config_filename)
- exit(1);
+ goto fail;
mesh_prov_db_filename = g_malloc(len + strlen("prov_db.json") + 2);
if (!mesh_prov_db_filename) {
- exit(1);
+ goto fail;
}
sprintf(mesh_local_config_filename, "%s", mesh_config_dir);
@@ -1950,7 +1950,7 @@ int main(int argc, char *argv[])
if (!prov_db_read_local_node(mesh_local_config_filename, true)) {
g_printerr("Failed to parse local node configuration file %s\n",
mesh_local_config_filename);
- exit(1);
+ goto fail;
}
sprintf(mesh_prov_db_filename, "%s", mesh_config_dir);
@@ -1965,7 +1965,7 @@ int main(int argc, char *argv[])
if (!prov_db_read(mesh_prov_db_filename)) {
g_printerr("Failed to parse provisioning database file %s\n",
mesh_prov_db_filename);
- exit(1);
+ goto fail;
}
dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
@@ -2001,5 +2001,9 @@ int main(int argc, char *argv[])
g_list_free(service_list);
g_list_free_full(ctrl_list, proxy_leak);
- return 0;
+ return EXIT_SUCCESS;
+
+fail:
+ bt_shell_cleanup();
+ return EXIT_FAILURE;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] mesh/meshctl: Exit cleanly if start up fails
2018-03-24 2:33 [PATCH BlueZ] mesh/meshctl: Exit cleanly if start up fails Inga Stotland
@ 2018-03-26 13:31 ` Luiz Augusto von Dentz
2018-03-26 15:23 ` Stotland, Inga
0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2018-03-26 13:31 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth@vger.kernel.org
Hi Inga,
On Sat, Mar 24, 2018 at 4:33 AM, Inga Stotland <inga.stotland@intel.com> wrote:
> When failing to start meshctl due to invalid input configuration,
> release bt_shell resources prior to exit.
Similar response as to Eramoto's patch, if the tool is going to exit
there is no need to free up the resources, the exit status is still
useful but that is a different matter. Or perhaps we just want to
avoid false positives with static analyzers?
> ---
> mesh/main.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/mesh/main.c b/mesh/main.c
> index d991c9f8c..d0f71c2d9 100644
> --- a/mesh/main.c
> +++ b/mesh/main.c
> @@ -1930,11 +1930,11 @@ int main(int argc, char *argv[])
> mesh_local_config_filename = g_malloc(len + strlen("local_node.json")
> + 2);
> if (!mesh_local_config_filename)
> - exit(1);
> + goto fail;
>
> mesh_prov_db_filename = g_malloc(len + strlen("prov_db.json") + 2);
> if (!mesh_prov_db_filename) {
> - exit(1);
> + goto fail;
> }
>
> sprintf(mesh_local_config_filename, "%s", mesh_config_dir);
> @@ -1950,7 +1950,7 @@ int main(int argc, char *argv[])
> if (!prov_db_read_local_node(mesh_local_config_filename, true)) {
> g_printerr("Failed to parse local node configuration file %s\n",
> mesh_local_config_filename);
> - exit(1);
> + goto fail;
> }
>
> sprintf(mesh_prov_db_filename, "%s", mesh_config_dir);
> @@ -1965,7 +1965,7 @@ int main(int argc, char *argv[])
> if (!prov_db_read(mesh_prov_db_filename)) {
> g_printerr("Failed to parse provisioning database file %s\n",
> mesh_prov_db_filename);
> - exit(1);
> + goto fail;
> }
>
> dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
> @@ -2001,5 +2001,9 @@ int main(int argc, char *argv[])
> g_list_free(service_list);
> g_list_free_full(ctrl_list, proxy_leak);
>
> - return 0;
> + return EXIT_SUCCESS;
> +
> +fail:
> + bt_shell_cleanup();
> + return EXIT_FAILURE;
> }
> --
> 2.13.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] mesh/meshctl: Exit cleanly if start up fails
2018-03-26 13:31 ` Luiz Augusto von Dentz
@ 2018-03-26 15:23 ` Stotland, Inga
2018-03-26 15:42 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 4+ messages in thread
From: Stotland, Inga @ 2018-03-26 15:23 UTC (permalink / raw)
To: luiz.dentz@gmail.com; +Cc: linux-bluetooth@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]
Hi Luiz,
On Mon, 2018-03-26 at 16:31 +0300, Luiz Augusto von Dentz wrote:
> Hi Inga,
>
> On Sat, Mar 24, 2018 at 4:33 AM, Inga Stotland <inga.stotland@intel.c
> om> wrote:
> > When failing to start meshctl due to invalid input configuration,
> > release bt_shell resources prior to exit.
>
>
> Similar response as to Eramoto's patch, if the tool is going to exit
> there is no need to free up the resources, the exit status is still
> useful but that is a different matter. Or perhaps we just want to
> avoid false positives with static analyzers?
>
My motivation for this patch was quite simple: if the program exits
after bt_shell_init() has ben called but prior to calling bt_shell_run,
bash shell is all messed up (at least on my system) after that.
>
> > ---
> > mesh/main.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/mesh/main.c b/mesh/main.c
> > index d991c9f8c..d0f71c2d9 100644
> > --- a/mesh/main.c
> > +++ b/mesh/main.c
> > @@ -1930,11 +1930,11 @@ int main(int argc, char *argv[])
> > mesh_local_config_filename = g_malloc(len +
> > strlen("local_node.json")
> >
> > + 2);
> > if (!mesh_local_config_filename)
> > - exit(1);
> > + goto fail;
> >
> > mesh_prov_db_filename = g_malloc(len +
> > strlen("prov_db.json") + 2);
> > if (!mesh_prov_db_filename) {
> > - exit(1);
> > + goto fail;
> > }
> >
> > sprintf(mesh_local_config_filename, "%s", mesh_config_dir);
> > @@ -1950,7 +1950,7 @@ int main(int argc, char *argv[])
> > if (!prov_db_read_local_node(mesh_local_config_filename,
> > true)) {
> > g_printerr("Failed to parse local node
> > configuration file %s\n",
> > mesh_local_config_filename);
> > - exit(1);
> > + goto fail;
> > }
> >
> > sprintf(mesh_prov_db_filename, "%s", mesh_config_dir);
> > @@ -1965,7 +1965,7 @@ int main(int argc, char *argv[])
> > if (!prov_db_read(mesh_prov_db_filename)) {
> > g_printerr("Failed to parse provisioning database
> > file %s\n",
> > mesh_prov_db_filename);
> > - exit(1);
> > + goto fail;
> > }
> >
> > dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
> > @@ -2001,5 +2001,9 @@ int main(int argc, char *argv[])
> > g_list_free(service_list);
> > g_list_free_full(ctrl_list, proxy_leak);
> >
> > - return 0;
> > + return EXIT_SUCCESS;
> > +
> > +fail:
> > + bt_shell_cleanup();
> > + return EXIT_FAILURE;
> > }
> > --
> > 2.13.6
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > bluetooth" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3266 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] mesh/meshctl: Exit cleanly if start up fails
2018-03-26 15:23 ` Stotland, Inga
@ 2018-03-26 15:42 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2018-03-26 15:42 UTC (permalink / raw)
To: Stotland, Inga; +Cc: linux-bluetooth@vger.kernel.org
Hi Inga,
On Mon, Mar 26, 2018 at 6:23 PM, Stotland, Inga <inga.stotland@intel.com> wrote:
> Hi Luiz,
>
> On Mon, 2018-03-26 at 16:31 +0300, Luiz Augusto von Dentz wrote:
>> Hi Inga,
>>
>> On Sat, Mar 24, 2018 at 4:33 AM, Inga Stotland <inga.stotland@intel.c
>> om> wrote:
>> > When failing to start meshctl due to invalid input configuration,
>> > release bt_shell resources prior to exit.
>>
>>
>> Similar response as to Eramoto's patch, if the tool is going to exit
>> there is no need to free up the resources, the exit status is still
>> useful but that is a different matter. Or perhaps we just want to
>> avoid false positives with static analyzers?
>>
>
> My motivation for this patch was quite simple: if the program exits
> after bt_shell_init() has ben called but prior to calling bt_shell_run,
> bash shell is all messed up (at least on my system) after that.
Then Im fine with the changes, but lets add this to the commit message
so it is clearer why we need to cleanup since it may affect the
original shell.
>>
>> > ---
>> > mesh/main.c | 14 +++++++++-----
>> > 1 file changed, 9 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/mesh/main.c b/mesh/main.c
>> > index d991c9f8c..d0f71c2d9 100644
>> > --- a/mesh/main.c
>> > +++ b/mesh/main.c
>> > @@ -1930,11 +1930,11 @@ int main(int argc, char *argv[])
>> > mesh_local_config_filename = g_malloc(len +
>> > strlen("local_node.json")
>> >
>> > + 2);
>> > if (!mesh_local_config_filename)
>> > - exit(1);
>> > + goto fail;
>> >
>> > mesh_prov_db_filename = g_malloc(len +
>> > strlen("prov_db.json") + 2);
>> > if (!mesh_prov_db_filename) {
>> > - exit(1);
>> > + goto fail;
>> > }
>> >
>> > sprintf(mesh_local_config_filename, "%s", mesh_config_dir);
>> > @@ -1950,7 +1950,7 @@ int main(int argc, char *argv[])
>> > if (!prov_db_read_local_node(mesh_local_config_filename,
>> > true)) {
>> > g_printerr("Failed to parse local node
>> > configuration file %s\n",
>> > mesh_local_config_filename);
>> > - exit(1);
>> > + goto fail;
>> > }
>> >
>> > sprintf(mesh_prov_db_filename, "%s", mesh_config_dir);
>> > @@ -1965,7 +1965,7 @@ int main(int argc, char *argv[])
>> > if (!prov_db_read(mesh_prov_db_filename)) {
>> > g_printerr("Failed to parse provisioning database
>> > file %s\n",
>> > mesh_prov_db_filename);
>> > - exit(1);
>> > + goto fail;
>> > }
>> >
>> > dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
>> > @@ -2001,5 +2001,9 @@ int main(int argc, char *argv[])
>> > g_list_free(service_list);
>> > g_list_free_full(ctrl_list, proxy_leak);
>> >
>> > - return 0;
>> > + return EXIT_SUCCESS;
>> > +
>> > +fail:
>> > + bt_shell_cleanup();
>> > + return EXIT_FAILURE;
>> > }
>> > --
>> > 2.13.6
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-
>> > bluetooth" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-26 15:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-24 2:33 [PATCH BlueZ] mesh/meshctl: Exit cleanly if start up fails Inga Stotland
2018-03-26 13:31 ` Luiz Augusto von Dentz
2018-03-26 15:23 ` Stotland, Inga
2018-03-26 15:42 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox