Great work.
Please find my review comments below.
STATUS
------
Approved.
REQUIRED CHANGES
----------------
No
REQUESTS
--------
No
SUGGESTIONS
-----------
SU1. Could you please modify the code to fulfill the coding guideline.
Please check the detail comments.
DETAILS
-------
> === 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 */
The line is too long. Please make it short than 80 chars.
> puts(*argument);
> my_free(load_default_groups);
> free_defaults(arguments);
>
[snip]
> @@ -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;
Please add a space after '='.
> /* End pointer */
[snip]
> @@ -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 */
>
Please add a space after '='.
> - (*argc)+=args.elements+1;
> + (*argc)+=args.elements+args_sep;
Same to above.
> *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);
>
--
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer
Email : Li-Bing.Song@stripped
Skype : libing.song
MSN : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================
