Re: [Maria-developers] [Commits] 732d45e: MDEV-8378 - Debian: the Lintian complains about many "shlib-calls-exit" in many
Hi, Sergey! On Dec 14, Sergey Vojtovich wrote:
revision-id: 732d45e93b81a104e3f3931e4908e22167a54622 (mariadb-10.0.22-57-g732d45e) parent(s): 3e206a518dec400e084451165f633b78eb2e7fee committer: Sergey Vojtovich timestamp: 2015-12-14 15:27:09 +0400 message:
MDEV-8378 - Debian: the Lintian complains about many "shlib-calls-exit" in many of the plugins
Fixed exit/_exit calls from libdaemon_example.so, libmysqlclient.so, libmysqlclient_r.so, libmysqld.so.
Note: this is just rough prototype, not to be pushed. libmysqld.so still has a bunch of exit/_exit calls from InnoDB.
diff --git a/mysys/my_addr_resolve.c b/mysys/my_addr_resolve.c index 90e6f43..276d5aa 100644 --- a/mysys/my_addr_resolve.c +++ b/mysys/my_addr_resolve.c @@ -190,7 +190,7 @@ const char *my_addr_resolve_init() close(out[0]); close(out[1]); execlp("addr2line", "addr2line", "-C", "-f", "-e", my_progname, NULL); - exit(1); + abort();
ok
}
close(in[0]); diff --git a/mysys/my_default.c b/mysys/my_default.c index 8faee00..7bf86f2 100644 --- a/mysys/my_default.c +++ b/mysys/my_default.c @@ -631,7 +631,7 @@ int my_load_defaults(const char *conf_file, const char **groups, if (!my_getopt_is_args_separator((*argv)[i])) /* skip arguments separator */ printf("%s ", (*argv)[i]); puts(""); - exit(0); + abort(); /* !!! */
not ok. --print-defaults means exit(0), not abort()
}
if (default_directories) @@ -641,7 +641,7 @@ int my_load_defaults(const char *conf_file, const char **groups,
err: fprintf(stderr,"Fatal error in defaults handling. Program aborted\n"); - exit(1); + abort();
ok
return 0; /* Keep compiler happy */ }
diff --git a/mysys/my_malloc.c b/mysys/my_malloc.c index e5332301..111bc7f 100644 --- a/mysys/my_malloc.c +++ b/mysys/my_malloc.c @@ -108,7 +108,7 @@ void *my_malloc(size_t size, myf my_flags) my_error(EE_OUTOFMEMORY, MYF(ME_BELL + ME_WAITTANG + ME_NOREFRESH + ME_FATALERROR),size); if (my_flags & MY_FAE) - exit(1); + abort();
no, MY_FAE means exit(1) not abort()
} else { diff --git a/mysys/my_static.c b/mysys/my_static.c index 84d2dc6..4272c24 100644 --- a/mysys/my_static.c +++ b/mysys/my_static.c @@ -72,7 +72,8 @@ ulong my_time_to_wait_for_lock=2; /* In seconds */ #ifdef SHARED_LIBRARY const char *globerrs[GLOBERRS]; /* my_error_messages is here */ #endif -void (*my_abort_hook)(int) = (void(*)(int)) exit; +void my_abort_wrapper(int a __attribute__((unused))) { abort(); } +void (*my_abort_hook)(int)= my_abort_wrapper;
whatever, it's never used, as far as I can see. It can as well be removed.
void (*error_handler_hook)(uint error, const char *str, myf MyFlags)= my_message_stderr; void (*fatal_error_handler_hook)(uint error, const char *str, myf MyFlags)= diff --git a/mysys/typelib.c b/mysys/typelib.c index 75744a6..ec1e8fb 100644 --- a/mysys/typelib.c +++ b/mysys/typelib.c @@ -51,7 +51,7 @@ int find_type_or_exit(const char *x, TYPELIB *typelib, const char *option) if ((res= find_type_with_warning(x, typelib, option)) <= 0) { sf_leaking_memory= 1; /* no memory leak reports here */ - exit(1); + abort();
no, it's find_type_or_exit, not find_type_or_abort. Incorrect option on the command line should not cause SIGABRT.
} return res; } diff --git a/plugin/daemon_example/daemon_example.cc b/plugin/daemon_example/daemon_example.cc index dbc3c5c..cdea632 100644 --- a/plugin/daemon_example/daemon_example.cc +++ b/plugin/daemon_example/daemon_example.cc @@ -129,7 +129,7 @@ static int daemon_example_plugin_init(void *p __attribute__ ((unused))) (void *)con) != 0) { fprintf(stderr,"Could not create heartbeat thread!\n"); - exit(0); + DBUG_RETURN(1);
ok
} plugin->data= (void *)con;
diff --git a/sql-common/client.c b/sql-common/client.c index d20759c..7a4efc3 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -1219,7 +1219,7 @@ void mysql_read_default_options(struct st_mysql_options *options, FIND_TYPE_BASIC)) <= 0) { fprintf(stderr, "Unknown option to protocol: %s\n", opt_arg); - exit(1); + abort();
No way. An application should not SIGABRT on the invalid values in the my.cnf file. It should not exit either, in fact, but it's a different issue.
} break; case OPT_shared_memory_base_name: diff --git a/sql/signal_handler.cc b/sql/signal_handler.cc index 3fadbcd..b301b4c 100644 --- a/sql/signal_handler.cc +++ b/sql/signal_handler.cc @@ -69,7 +69,7 @@ extern "C" sig_handler handle_fatal_signal(int sig) if (segfaulted) { my_safe_printf_stderr("Fatal " SIGNAL_FMT " while backtracing\n", sig); - _exit(1); /* Quit without running destructors */ + abort();
Heh, no. You don't want to send another signal when you've encountered an fatal error inside the signal handler. ... etc ... Are you sure we want to fix that at all? And if yes - is it something we need to bother fixing now? How comes it's "critical"? Regards, Sergei Chief Architect MariaDB and security@mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/maria... https://www.percona.com/live/data-performance-conference-2016/sessions/maria...
Hi Sergei, On Tue, Dec 15, 2015 at 10:49:37PM +0100, Sergei Golubchik wrote:
Hi, Sergey!
On Dec 14, Sergey Vojtovich wrote:
revision-id: 732d45e93b81a104e3f3931e4908e22167a54622 (mariadb-10.0.22-57-g732d45e) parent(s): 3e206a518dec400e084451165f633b78eb2e7fee committer: Sergey Vojtovich timestamp: 2015-12-14 15:27:09 +0400 message:
MDEV-8378 - Debian: the Lintian complains about many "shlib-calls-exit" in many of the plugins
Fixed exit/_exit calls from libdaemon_example.so, libmysqlclient.so, libmysqlclient_r.so, libmysqld.so.
Note: this is just rough prototype, not to be pushed. libmysqld.so still has a bunch of exit/_exit calls from InnoDB. ...skip...
Are you sure we want to fix that at all? Definitely daemon_example. The rest is nice to have (e.g. I wouldn't want my app to exit if embedded gets --print-defaults).
But the fix should be different: change of abort to exit makes little sense. Either we should pass through error code, or disable code which is never executed by embedded/libmysqlclient with ifdefs.
And if yes - is it something we need to bother fixing now? The right fix as described above may become complex, so I'd rather postpone it for 10.2. But Debian is suffering from this now with 10.0.
I don't know.
How comes it's "critical"? I think the reason behind "critical" is our willing to please Debian.
If you don't mind I'll push daemon_example part of this fix and remove my_abort_hook: one line less in lintian report is a progress still. Thanks, Sergey
Hi, Sergey! On Dec 16, Sergey Vojtovich wrote:
Are you sure we want to fix that at all? Definitely daemon_example.
fine
The rest is nice to have (e.g. I wouldn't want my app to exit if embedded gets --print-defaults).
agree
But the fix should be different: change of abort to exit makes little sense. Either we should pass through error code, or disable code which is never executed by embedded/libmysqlclient with ifdefs.
okay. for mysys that means error codes, meaning API changes.
If you don't mind I'll push daemon_example part of this fix and remove my_abort_hook: one line less in lintian report is a progress still.
sure Regards, Sergei Chief Architect MariaDB and security@mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/maria... https://www.percona.com/live/data-performance-conference-2016/sessions/maria...
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich