Hi, Alexey!
On Oct 27, Alexey Botchkov wrote:
> #At file:///home/hf/work/mysql_common/39289/
>
> 2664 Alexey Botchkov 2008-10-27
> Bug#39289 libmysqld.a calls exit() upon error
>
> Several functions (mostly in mysqld.cc) directly call
> exit() function in case of errors, which is not a desired
> behaviour expecially in the embedded-server library.
>
> Fixed by making these functions return error sign instead
> of exiting.
looks good.
suggestion: to prevent exit() to reapper (unintentinally) you can add to
my_global.h something like
#define exit(X) do not use exit in the server it is bad for embedded
and in mysqld.cc, in one place where it's really necessary:
#undef exit
exit(0);
see other comments below. Ok to push after fixing them (implementing the
"suggestion" above is optional, as you like).
> @@ -7481,7 +7485,10 @@ static void mysql_init_variables(void)
> delay_key_write_options= (uint) DELAY_KEY_WRITE_ON;
> slave_exec_mode_options= 0;
> slave_exec_mode_options= (uint)
> - find_bit_type_or_exit(slave_exec_mode_str, &slave_exec_mode_typelib, NULL);
> + find_bit_type_or_exit(slave_exec_mode_str, &slave_exec_mode_typelib, NULL,
rename the function to, say, find_bit_type_or_complain (..._or_print_error,
whatever).
> + &error);
> + if (error)
> + return 1;
> opt_specialflag= SPECIAL_ENGLISH;
> unix_sock= ip_sock= INVALID_SOCKET;
> mysql_home_ptr= mysql_home;
> @@ -8167,13 +8185,17 @@ mysqld_get_one_option(int optid,
>
> /** Handle arguments for multiple key caches. */
>
> -extern "C" uchar **mysql_getopt_value(const char *keyname, uint key_length,
> - const struct my_option *option);
> +extern "C" int mysql_getopt_value(uchar **value,
why int ?
> + const char *keyname, uint key_length,
> + const struct my_option *option,
> + int *error);
>
> -uchar* *
> +static uchar* *
and why static ?
> mysql_getopt_value(const char *keyname, uint key_length,
> - const struct my_option *option)
> + const struct my_option *option, int *error)
> {
> + if (error)
> + *error= 0;
> switch (option->id) {
> case OPT_KEY_BUFFER_SIZE:
> case OPT_KEY_CACHE_BLOCK_SIZE:
> @@ -8441,30 +8469,37 @@ static void fix_paths(void)
> my_free(opt_secure_file_priv, MYF(0));
> opt_secure_file_priv= my_strdup(buff, MYF(MY_FAE));
> }
> + return 0;
> }
>
>
> static ulong find_bit_type_or_exit(const char *x, TYPELIB *bit_lib,
> - const char *option)
> + const char *option, int *error)
> {
> - ulong res;
> -
> + ulong result;
> const char **ptr;
>
> - if ((res= find_bit_type(x, bit_lib)) == ~(ulong) 0)
> + *error= 0;
> + if ((result= find_bit_type(x, bit_lib)) == ~(ulong) 0)
> {
> + char *buff= (char *) my_alloca(2048);
> + char *cbuf;
> ptr= bit_lib->type_names;
> - if (!*x)
> - fprintf(stderr, "No option given to %s\n", option);
> - else
> - fprintf(stderr, "Wrong option to %s. Option(s) given: %s\n", option, x);
> - fprintf(stderr, "Alternatives are: '%s'", *ptr);
> + cbuf= buff + ((!*x) ?
> + my_snprintf(buff, 2048, "No option given to %s\n", option) :
> + my_snprintf(buff, 2048, "Wrong option to %s. Option(s) given: %s\n",
> + option, x));
> + cbuf+= my_snprintf(cbuf, 2048 - (cbuf-buff), "Alternatives are: '%s'", *ptr);
> while (*++ptr)
> - fprintf(stderr, ",'%s'", *ptr);
> - fprintf(stderr, "\n");
> - exit(1);
> + cbuf+= my_snprintf(cbuf, 2048 - (cbuf-buff), ",'%s'", *ptr);
> + my_snprintf(cbuf, 2048 - (cbuf-buff), "\n");
> + sql_perror(buff);
1. Why perror ? there's no meaningful errno here. Use sql_print_error().
2. why do you assemble a complete message in a buffer,
instead of simply replacing every fprintf with sql_print_error ?
> + *error= 1;
> + my_afree(buff);
> + return 0;
> }
> - return res;
> +
> + return result;
> }
Regards / Mit vielen Grüßen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB München 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring
