maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Rucha!

On Nov 04, Rucha Deodhar wrote:
> revision-id: 80749146b7d (mariadb-10.6.1-77-g80749146b7d)
> parent(s): 76149650764
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2021-09-12 02:11:19 +0530
> message:
> 
> MDEV-26238: Remove inconsistent behaviour of --default-* options
> in my_print_defaults
> 
> Analysis: --defaults* option is recognized anywhere in the commandline
> instead of only at the beginning because handle_options() recognizes
> options in any order.
> Fix: use get_defaults_options() which recognizes --defaults* options only at
> the beginning. After this is done, we only want to recognize other options
> given in any order which can be done using handle_options(). So only skip
> --defaults* options and pass rest of them to handle_options().
> Also, removed -e, -g and -c because only my_print_defaults supports them.
> 
> diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c
> index b7f52382721..607509a309e 100644
> --- a/extra/my_print_defaults.c
> +++ b/extra/my_print_defaults.c
> @@ -141,50 +124,39 @@ static int get_options(int *argc,char ***argv)
...
>  int main(int argc, char **argv)
>  {
> -  int count= 0, error, no_defaults= 0;
> +  int count, error, args_used;
>    char **load_default_groups= 0, *tmp_arguments[6];
>    char **argument, **arguments, **org_argv;
>    int nargs, i= 0;
>    MY_INIT(argv[0]);
>  
>    org_argv= argv;
> -  if (*argv && !strcmp(*argv, "--no-defaults"))
> -  {
> -    argv++;
> -    ++count;
> -    no_defaults= 1;
> -  }
> -  /* Copy program name and --no-defaults if present*/
> +  args_used= get_defaults_options(argv);
> +
> +  /* Copy defaults-xxx arguments & program name */
> +  count= args_used;
>    arguments= tmp_arguments;
> -  memcpy((char*) arguments, (char*) org_argv, (++count)*sizeof(*org_argv));
> +  memcpy((char*) arguments, (char*) org_argv, count*sizeof(*org_argv));
>    arguments[count]= 0;

I still cannot understand why you're doing it. Why do you need a copy of
argv?

>  
> +  /*
> +     We already process --defaults* options at the beginning in
> +     get_defaults_options(). So skip --defaults* options and
> +     pass remaining options to handle_options().
> +  */
> +  if (is_prefix(org_argv[1], "--no-defaults"))
> +    args_used--;

I'm sorry, I don't understand. Why do you need that?
all *defaults* options were handled in get_defaults_options().

> +

why do you need org_argv? First you do

   org_argv= argv;

then after some code that doesn't change either argv or org_argv, you
copy the vallue back:

> +  argv=org_argv;
> +  argv+=args_used-1;
> +  argc-=args_used-1;
> +
>    /* Check out the args */
>    if (get_options(&argc,&argv))
>      cleanup_and_exit(1);
>  
> -  if (!no_defaults)
> -  {
> -    if (opt_defaults_file_used)
> -     arguments[count++]= make_args("--defaults-file=", config_file);
> -    if (my_defaults_extra_file)
> -      arguments[count++]= make_args("--defaults-extra-file=",
> -                                  my_defaults_extra_file);
> -    if (my_defaults_group_suffix)
> -      arguments[count++]= make_args("--defaults-group-suffix=",
> -                                  my_defaults_group_suffix);
> -    arguments[count]= 0;
> -  }
> -
>    nargs= argc + 1;
>    if (opt_mysqld)
>      nargs+= array_elements(mysqld_groups);

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx