Hi, Rucha! On Sep 08, Rucha Deodhar wrote: > Hi, Sergei ! > Thanks for the review. > > > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > > > index b7f52382721..e2ec0b00923 100644 > > > --- a/extra/my_print_defaults.c > > > +++ b/extra/my_print_defaults.c > > > @@ -48,20 +48,6 @@ static struct my_option my_long_options[] = > > > > you forgot the "config-file", which is here, just about "debug" > > I am sorry, I don't understand.
Oops. Sorry. I looked at my_print_defaults.c in the checked out branch and I saw: {"config-file", 'c', "Deprecated, please use --defaults-file instead. " "Name of config file to read; if no extension is given, default " "extension (e.g., .ini or .cnf) will be added", (char**) &config_file, (char**) &config_file, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, #ifdef DBUG_OFF {"debug", '#', "This is a non-debug version. Catch this and exit", 0,0, 0, GET_DISABLED, OPT_ARG, 0, 0, 0, 0, 0, 0}, #else so I wrote that "config-file" should also be removed. But now I see that it's present in 10.2, but not in 10.4, it was already removed. So, ignore that my comment, you didn't forget config-file, it was removed in 10.4, I looked at the wrong branch. > > > {"debug", '#', "Output debug log", (char**) &default_dbug_option, > > > (char**) &default_dbug_option, 0, GET_STR, OPT_ARG, 0, 0, 0, 0, 0, 0}, > > > #endif > > > - {"defaults-file", 'c', > > > - "Read this file only, do not read global or per-user config " > > > - "files; should be the first option", > > > - (char**) &config_file, (char*) &config_file, 0, GET_STR, REQUIRED_ARG, > > > @@ -141,49 +124,44 @@ static int get_options(int *argc,char ***argv) > > > return 0; > > > } > > > > > > -static char *make_args(const char *s1, const char *s2) > > > -{ > > > - char *s= malloc(strlen(s1) + strlen(s2) + 1); > > > - strmov(strmov(s, s1), s2); > > > - return s; > > > -} > > > - > > > 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; > > > + char **argument, **arguments, **org_argv, **new_argv, **pos; > > > + int nargs, i= 0, index=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; > > > > > > - /* Check out the args */ > > > - if (get_options(&argc,&argv)) > > > - cleanup_and_exit(1); > > > - > > > - if (!no_defaults) > > > + /* > > > + We already process --defaults* options at the beginning in > > > + get_defaults_options(). So, now we we make a new commandline which > > > + has all options except --defaults* options and pass it to > > > handle_options() > > > + instead of original commandline. > > > + */ > > > + new_argv= (char**)malloc(argc*sizeof(*org_argv)); > > > + for (pos= org_argv; *pos; pos++) > > > { > > > + if (!is_prefix(*pos, "--defaults-extra-file=") && > > > + !is_prefix(*pos, "--defaults-file=") && > > > + !is_prefix(*pos, "--defaults-group-suffix=")) > > > + new_argv[index++]= *(pos); > > > > Why are you doing it? > > After get_defaults_options, you know how many arguments were default, > > just skip args_used, and invoke handle_options on the rest. > > I did it to filter all the --defaults* options, anywhere on the command > line. > > get_defaults_options() counts the program name in addition to --defaults* > options, so args_used will at least be 1. So skipping argv using agrs_used > will at least skip the program name even before it is passed to > handle_options(). But argv is also skipped in handle_options() to skip the > program name. > So one extra argument will get skipped too. correct, you need to skip args_used-1, good point. > Say --defaults* options are somewhere at the end, they will not get counted > in get_defaults_options(). So --defaults* will be passed to > handle_options() without getting skipped. And handle_options() will give an > error even if the options are valid because > it will not find --defaults* options in my_long_options[]. This is what we want, right? That's why you're doing it. Try `mariadb` client or `mariadb-dump` or `mariadbd` server, anything, and put --defaults* option not at the beginning. It'll be "unknown option". The idea is to make my_print_defaults *not special*, to behave like any other executable. Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp