List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:January 14 2011 2:33pm
Subject:Re: bzr commit into mysql-5.5-bugteam branch (zhenxing.he:3180) Bug#57953
View as plain text  
Hi Zhenxing,

Nice work, I can't think of anything to improve. Patch approved.

/Sven


On 12/20/2010 05:31 AM, He Zhenxing wrote:
> #At file:///media/sdb2/hezx/work/mysql/bzr/b57953/5.5-bugteam/ based on
> revid:georgi.kodinov@stripped
>
>   3180 He Zhenxing	2010-12-20
>        BUG#57953 my_load_defaults return junk argument ----args-separator---- to
> caller
>
>        After fix of bug#25192, load_defaults() will add an args separator
>        to distinguish options loaded from configure files from that provided
>        in the command line. One problem of this is that the args separator
>        would be added no matter the application need it or not.
>
>        Fixed the problem by adding an option:
>          bool my_getopt_use_args_separator;
>        to control whether the separator will be added or not. And also
>        added functions:
>          bool my_getopt_is_args_separator(const char* arg);
>        to check if the argument is the separator or not.
>
>      modified:
>        extra/my_print_defaults.c
>        include/my_sys.h
>        mysys/default.c
>        mysys/my_getopt.c
>        sql-common/client.c
>        sql/mysqld.cc
>        storage/ndb/test/run-test/setup.cpp
> === modified file 'extra/my_print_defaults.c'
> --- a/extra/my_print_defaults.c	2010-07-15 11:13:30 +0000
> +++ b/extra/my_print_defaults.c	2010-12-20 04:30:59 +0000
> @@ -193,7 +193,7 @@ int main(int argc, char **argv)
>     }
>
>     for (argument= arguments+1 ; *argument ; argument++)
> -    if (*argument != args_separator)           /* skip arguments separator */
> +    if (!my_getopt_is_args_separator(*argument))           /* skip arguments
> separator */
>         puts(*argument);
>     my_free(load_default_groups);
>     free_defaults(arguments);
>
> === modified file 'include/my_sys.h'
> --- a/include/my_sys.h	2010-12-03 01:06:56 +0000
> +++ b/include/my_sys.h	2010-12-20 04:30:59 +0000
> @@ -834,7 +834,8 @@ extern void *memdup_root(MEM_ROOT *root,
>   extern int get_defaults_options(int argc, char **argv,
>                                   char **defaults, char **extra_defaults,
>                                   char **group_suffix);
> -extern const char *args_separator;
> +extern my_bool my_getopt_use_args_separator;
> +extern my_bool my_getopt_is_args_separator(const char* arg);
>   extern int my_load_defaults(const char *conf_file, const char **groups,
>                               int *argc, char ***argv, const char ***);
>   extern int load_defaults(const char *conf_file, const char **groups,
>
> === modified file 'mysys/default.c'
> --- a/mysys/default.c	2010-11-16 10:58:39 +0000
> +++ b/mysys/default.c	2010-12-20 04:30:59 +0000
> @@ -61,9 +61,23 @@
>      check the pointer, use "----args-separator----" here to ease debug
>      if someone misused it.
>
> +   The args seprator will only be added when
> +   my_getopt_use_args_seprator is set to TRUE before calling
> +   load_defaults();
> +
>      See BUG#25192
>   */
> -const char *args_separator= "----args-separator----";
> +static const char *args_separator= "----args-separator----";
> +inline static void set_args_separator(char** arg)
> +{
> +  DBUG_ASSERT(my_getopt_use_args_separator);
> +  *arg= (char*)args_separator;
> +}
> +my_bool my_getopt_use_args_separator= FALSE;
> +my_bool my_getopt_is_args_separator(const char* arg)
> +{
> +  return (arg == args_separator);
> +}
>   const char *my_defaults_file=0;
>   const char *my_defaults_group_suffix=0;
>   const char *my_defaults_extra_file=0;
> @@ -503,6 +517,7 @@ int my_load_defaults(const char *conf_fi
>     char *ptr,**res;
>     struct handle_option_ctx ctx;
>     const char **dirs;
> +  uint args_sep= my_getopt_use_args_separator ? 1 : 0;
>     DBUG_ENTER("load_defaults");
>
>     init_alloc_root(&alloc,512,0);
> @@ -515,17 +530,28 @@ int my_load_defaults(const char *conf_fi
>     if (*argc>= 2&&  !strcmp(argv[0][1],"--no-defaults"))
>     {
>       /* remove the --no-defaults argument and return only the other arguments */
> -    uint i;
> +    uint i, j;
>       if (!(ptr=(char*) alloc_root(&alloc,sizeof(alloc)+
>   				 (*argc + 1)*sizeof(char*))))
>         goto err;
>       res= (char**) (ptr+sizeof(alloc));
>       res[0]= **argv;				/* Copy program name */
> -    /* set arguments separator */
> -    res[1]= (char *)args_separator;
> -    for (i=2 ; i<  (uint) *argc ; i++)
> -      res[i]=argv[0][i];
> -    res[i]=0;					/* End pointer */
> +    j= 1;                 /* Start from 1 for the reset result args */
> +    if (my_getopt_use_args_separator)
> +    {
> +      /* set arguments separator */
> +      set_args_separator(&res[1]);
> +      j++;
> +    }
> +    for (i=2 ; i<  (uint) *argc ; i++, j++)
> +      res[j]=argv[0][i];
> +    res[j]=0;					/* End pointer */
> +    /*
> +      Update the argc, if have not added args separator, then we have
> +      to decrease argc because we have removed the "--no-defaults".
> +    */
> +    if (!my_getopt_use_args_separator)
> +      (*argc)--;
>       *argv=res;
>       *(MEM_ROOT*) ptr= alloc;			/* Save alloc root for free */
>       if (default_directories)
> @@ -559,7 +585,7 @@ int my_load_defaults(const char *conf_fi
>       or a forced default file
>     */
>     if (!(ptr=(char*) alloc_root(&alloc,sizeof(alloc)+
> -			       (args.elements + *argc + 1 + 1) *sizeof(char*))))
> +			       (args.elements + *argc + 1 + args_sep) *sizeof(char*))))
>       goto err;
>     res= (char**) (ptr+sizeof(alloc));
>
> @@ -580,16 +606,19 @@ int my_load_defaults(const char *conf_fi
>       --*argc; ++*argv;				/* skip argument */
>     }
>
> -  /* set arguments separator for arguments from config file and
> -     command line */
> -  res[args.elements+1]= (char *)args_separator;
> +  if (my_getopt_use_args_separator)
> +  {
> +    /* set arguments separator for arguments from config file and
> +       command line */
> +    set_args_separator(&res[args.elements+1]);
> +  }
>
>     if (*argc)
> -    memcpy((uchar*) (res+1+args.elements+1), (char*) ((*argv)+1),
> +    memcpy((uchar*) (res+1+args.elements+args_sep), (char*) ((*argv)+1),
>   	   (*argc-1)*sizeof(char*));
> -  res[args.elements+ *argc+1]=0;                /* last null */
> +  res[args.elements+ *argc+args_sep]=0;                /* last null */
>
> -  (*argc)+=args.elements+1;
> +  (*argc)+=args.elements+args_sep;
>     *argv= (char**) res;
>     *(MEM_ROOT*) ptr= alloc;			/* Save alloc root for free */
>     delete_dynamic(&args);
> @@ -599,7 +628,7 @@ int my_load_defaults(const char *conf_fi
>       printf("%s would have been started with the following arguments:\n",
>   	   **argv);
>       for (i=1 ; i<  *argc ; i++)
> -      if ((*argv)[i] != args_separator) /* skip arguments separator */
> +      if (!my_getopt_is_args_separator((*argv)[i])) /* skip arguments separator */
>           printf("%s ", (*argv)[i]);
>       puts("");
>       exit(0);
>
> === modified file 'mysys/my_getopt.c'
> --- a/mysys/my_getopt.c	2010-10-25 12:30:07 +0000
> +++ b/mysys/my_getopt.c	2010-12-20 04:30:59 +0000
> @@ -176,7 +176,7 @@ int handle_options(int *argc, char ***ar
>     */
>     for (pos= *argv, pos_end=pos+ *argc; pos != pos_end ; pos++)
>     {
> -    if (*pos == args_separator)
> +    if (my_getopt_is_args_separator(*pos))
>       {
>         is_cmdline_arg= 0;
>         break;
> @@ -188,7 +188,7 @@ int handle_options(int *argc, char ***ar
>       char **first= pos;
>       char *cur_arg= *pos;
>       opt_found= 0;
> -    if (!is_cmdline_arg&&  (cur_arg == args_separator))
> +    if (!is_cmdline_arg&&  (my_getopt_is_args_separator(cur_arg)))
>       {
>         is_cmdline_arg= 1;
>
>
> === modified file 'sql-common/client.c'
> --- a/sql-common/client.c	2010-11-10 15:21:51 +0000
> +++ b/sql-common/client.c	2010-12-20 04:30:59 +0000
> @@ -1220,7 +1220,7 @@ void mysql_read_default_options(struct s
>       char **option=argv;
>       while (*++option)
>       {
> -      if (option[0] == args_separator)          /* skip arguments separator */
> +      if (my_getopt_is_args_separator(option[0]))          /* skip arguments
> separator */
>           continue;
>         /* DBUG_PRINT("info",("option: %s",option[0])); */
>         if (option[0][0] == '-'&&  option[0][1] == '-')
>
> === modified file 'sql/mysqld.cc'
> --- a/sql/mysqld.cc	2010-12-07 17:08:54 +0000
> +++ b/sql/mysqld.cc	2010-12-20 04:30:59 +0000
> @@ -4216,8 +4216,10 @@ int mysqld_main(int argc, char **argv)
>
>     orig_argc= argc;
>     orig_argv= argv;
> +  my_getopt_use_args_separator= TRUE;
>     if (load_defaults(MYSQL_CONFIG_NAME, load_default_groups,&argc,&argv))
>       return 1;
> +  my_getopt_use_args_separator= FALSE;
>     defaults_argc= argc;
>     defaults_argv= argv;
>     remaining_argc= argc;
>
> === modified file 'storage/ndb/test/run-test/setup.cpp'
> --- a/storage/ndb/test/run-test/setup.cpp	2009-10-02 08:25:53 +0000
> +++ b/storage/ndb/test/run-test/setup.cpp	2010-12-20 04:30:59 +0000
> @@ -105,7 +105,7 @@ setup_config(atrt_config&  config)
>        */
>       for (j = 0; j<(size_t)argc; j++)
>       {
> -      if (tmp[j] == args_separator)             /* skip arguments separator */
> +      if (my_getopt_is_args_separator(tmp[j])) /* skip arguments separator */
>           continue;
>         for (k = 0; proc_args[k].name; k++)
>         {
> @@ -375,7 +375,7 @@ load_options(int argc, char** argv, int
>        *  Skip the separator for arguments from config file and command
>        *  line
>        */
> -    if (argv[i] == args_separator)
> +    if (my_getopt_is_args_separator(argv[i]))
>         continue;
>       for (size_t j = 0; f_options[j].name; j++)
>       {
>
>
>
>
>

Thread
bzr commit into mysql-5.5-bugteam branch (zhenxing.he:3180) Bug#57953He Zhenxing20 Dec
  • Re: bzr commit into mysql-5.5-bugteam branch (zhenxing.he:3180)Bug#57953Libing Song22 Dec
  • Re: bzr commit into mysql-5.5-bugteam branch (zhenxing.he:3180) Bug#57953Sven Sandberg14 Jan