maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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@xxxxxxxxxxx
-- 
Vote for my Percona Live 2016 talks:
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-connectors-fast-and-smart-new-protocol-optimizations#community-voting
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-101-security-validation-authentication-encryption#community-voting