Bug #25074 mysql_upgrade test can't give consistent results

 - Tell the programs executed by mysql_upgrade to only read
  from the defaults file generated by msyql_upgrade
 - Fix memory leaks


client/mysql_upgrade.c:
  - Always generate defaults file used for the programs executed
    by mysql_upgrade.
  - Set the executed programs to read options _only_ from the
    generated defaults file 
  - Add DBUG printouts
  - Add comments
  - Fix memory leaks
  - Change one 'my_delete'(delete a file) to 'my_free'(free memory)
  - Free memory allocated by 'load_default'

client/mysql_upgrade.c

@@ -54,6 +54,8 @@ static char *default_dbug_option= (char*) "d:t:O,/tmp/mysql_upgrade.trace";
 #endif
 static my_bool info_flag= 0, tty_password= 0;
 
+static char **defaults_argv;
+
 static struct my_option my_long_options[]=
 {
   {"help", '?', "Display this help message and exit.", 0, 0, 0, GET_NO_ARG,
@@ -282,6 +284,10 @@ static int create_defaults_file(const char *path, const char *forced_path)
   DYNAMIC_STRING buf;
   extra_default_t *d;
 
+  DBUG_ENTER("create_defaults_file");
+  DBUG_PRINT("enter", ("path: %s, forced_path: %s", path, forced_path));
+
+  /* Delete any previous defaults file generated by mysql_upgrade */
   my_delete(path, MYF(0));
   
   defaults_file= my_open(path, O_BINARY | O_CREAT | O_WRONLY | O_EXCL,
@@ -298,6 +304,7 @@ static int create_defaults_file(const char *path, const char *forced_path)
     goto error;
   }
 
+  /* Copy forced_path file into the defaults_file being generated */
   if (forced_path)
   {
     forced_file= my_open(forced_path, O_RDONLY, MYF(MY_FAE | MY_WME));
@@ -306,6 +313,7 @@ static int create_defaults_file(const char *path, const char *forced_path)
       ret= 1;
       goto error;
     }
+    DBUG_PRINT("info", ("Copying from %s to %s", forced_path, path));
     do
     {
       cnt= my_read(forced_file, buf.str, buf.max_length, MYF(MY_WME));
@@ -316,10 +324,12 @@ static int create_defaults_file(const char *path, const char *forced_path)
         my_close(forced_file, MYF(0));
         goto error;
       }
+      DBUG_PRINT("info", ("%s", buf.str));
     } while (cnt == buf.max_length);
     my_close(forced_file, MYF(0));
   }
-  
+
+  /* Write all extra_default options into the [client] section */
   dynstr_set(&buf, "\n[client]");
   if (opt_password) 
   {
@@ -330,6 +340,7 @@ static int create_defaults_file(const char *path, const char *forced_path)
       goto error;
     }
   }
+  DBUG_PRINT("info", ("Writing extra_defaults to file"));
   while (extra_defaults) 
   {
     int len;
@@ -338,6 +349,7 @@ static int create_defaults_file(const char *path, const char *forced_path)
     len= d->n_len + d->v_len + 1; 
     if (buf.length + len >= buf.max_length)     /* to avoid realloc() */
     {
+      
       if (my_write(defaults_file, buf.str, buf.length, MYF(MY_FNABP | MY_WME)))
       {
         ret= 1;
@@ -345,15 +357,16 @@ static int create_defaults_file(const char *path, const char *forced_path)
       }
       dynstr_set(&buf, NULL);
     }
-    if (dynstr_append_mem(&buf, "\n", 1)
-       || dynstr_append_mem(&buf, d->name, d->n_len)
-       || (d->v_len && (dynstr_append_mem(&buf, "=", 1)
-       || dynstr_append_mem(&buf, d->value, d->v_len))))
+    if (dynstr_append_mem(&buf, "\n", 1) ||
+        dynstr_append_mem(&buf, d->name, d->n_len) ||
+        (d->v_len && (dynstr_append_mem(&buf, "=", 1) ||
+                      dynstr_append_mem(&buf, d->value, d->v_len))))
     {
       ret= 1;
       goto error;
     }
-    my_delete((gptr)d, MYF(0));
+    DBUG_PRINT("info", ("%s", buf.str));
+    my_free((gptr)d, MYF(0));
     list_pop(extra_defaults);                   /* pop off the head */
   }
   if (my_write(defaults_file, buf.str, buf.length, MYF(MY_FNABP | MY_WME)))
@@ -373,7 +386,7 @@ static int create_defaults_file(const char *path, const char *forced_path)
     my_delete(path, MYF(0));
   
 out:
-  return ret;
+  DBUG_RETURN(ret);
 }
 
 
@@ -449,13 +462,9 @@ int main(int argc, char **argv)
   
   char *forced_defaults_file;
   char *forced_extra_defaults;
-  char *defaults_group_suffix;
-  const char *script_line;
-  char *upgrade_defaults_path; 
-  char *defaults_to_use= NULL;
-  int upgrade_defaults_created= 0;
-  
-  char path[FN_REFLEN];
+  char *local_defaults_group_suffix;
+
+  char path[FN_REFLEN], upgrade_defaults_path[FN_REFLEN];
   DYNAMIC_STRING cmdline;
 
   MY_INIT(argv[0]);
@@ -466,9 +475,10 @@ int main(int argc, char **argv)
   /* Check if we are forced to use specific defaults */
   get_defaults_options(argc, argv,
                        &forced_defaults_file, &forced_extra_defaults,
-                       &defaults_group_suffix);
+                       &local_defaults_group_suffix);
   
   load_defaults("my", load_default_groups, &argc, &argv);
+  defaults_argv= argv;
 
   /* 
      Must init_dynamic_string before handle_options because string is freed
@@ -516,25 +526,19 @@ int main(int argc, char **argv)
     goto error;
   }
 
-  /* 
-     Create the modified defaults file to be used by mysqlcheck 
-     and mysql tools                                            
+  /*
+     Create the modified defaults file to be used by mysqlcheck
+     and mysql command line client
    */
-  fn_format(path, UPGRADE_DEFAULTS_NAME, datadir, "", MYF(0));
-  upgrade_defaults_path= my_strdup(path, MYF(0));
-  
-  if (extra_defaults) 
-  {
-    ret= create_defaults_file(upgrade_defaults_path, forced_extra_defaults);
-    if (ret)
-      goto error;
-    
-    defaults_to_use= upgrade_defaults_path;
-    upgrade_defaults_created= 1;
-  } 
-  else
-    defaults_to_use= forced_extra_defaults;
+  fn_format(upgrade_defaults_path, UPGRADE_DEFAULTS_NAME, datadir, "", MYF(0));
+  create_defaults_file(upgrade_defaults_path, forced_extra_defaults);
+
 
+  /*
+    Read the mysql_upgrade_info file to check if mysql_upgrade
+    already has been done
+    Maybe this could be done a little earlier?
+  */
   if (!find_file(MYSQL_UPGRADE_INFO_NAME, datadir, MY_SEARCH_SELF, 
                           path, sizeof(path), NULL, NullS)
      && !opt_force)
@@ -553,7 +557,9 @@ int main(int argc, char **argv)
       goto fix_priv_tables;
     }
   }
-  
+
+
+  /* Find mysqlcheck */
   if (find_file(mysqlcheck_name, basedir, MYF(0), path, sizeof(path),
                 "bin", EXTRA_CLIENT_PATHS, NullS))
   {
@@ -575,13 +581,13 @@ int main(int argc, char **argv)
     dynstr_append_os_quoted(&cmdline, path, NullS);
   }
 
-  if (defaults_to_use)
-  {
-    dynstr_append(&cmdline, " ");
-    dynstr_append_os_quoted(&cmdline, "--defaults-extra-file=", 
-                            defaults_to_use, NullS);
-  }
-
+  /*
+    All settings have been written to the "upgrade_defaults_path"
+    instruct mysqlcheck to only read options from that file
+  */
+  dynstr_append(&cmdline, " ");
+  dynstr_append_os_quoted(&cmdline, "--defaults-file=",
+                          upgrade_defaults_path, NullS);
   dynstr_append(&cmdline, " ");
   dynstr_append_os_quoted(&cmdline, "--check-upgrade", NullS);
   dynstr_append(&cmdline, " ");
@@ -594,9 +600,10 @@ int main(int argc, char **argv)
   dynstr_append(&cmdline, "\"");
 #endif /* __WIN__ */
 
+  /* Execute mysqlcheck */
   if (opt_verbose)
     printf("Running %s\n", cmdline.str);
-
+  DBUG_PRINT("info", ("Running: %s", cmdline.str));
   ret= system(cmdline.str);
   if (ret)
   {
@@ -610,6 +617,7 @@ int main(int argc, char **argv)
     goto error;
 
 fix_priv_tables:
+  /* Find mysql */
   if (find_file(mysql_name, basedir, MYF(0), path, sizeof(path), 
                 "bin", EXTRA_CLIENT_PATHS, NullS))
   {
@@ -631,6 +639,7 @@ int main(int argc, char **argv)
     dynstr_append_os_quoted(&cmdline, path, NullS);
   }
 
+  /* Find mysql_fix_privililege_tables.sql */
   if (find_file(MYSQL_FIX_PRIV_TABLES_NAME, basedir, MYF(0), 
                           path, sizeof(path), 
                           "support_files", "share", "share/mysql", "scripts",
@@ -646,15 +655,14 @@ int main(int argc, char **argv)
            " where MySQL is installed");
     goto error;
   }
-  else
-    script_line= my_strdup(path, MYF(0));
 
-  if (defaults_to_use)
-  {
-    dynstr_append(&cmdline, " ");
-    dynstr_append_os_quoted(&cmdline, "--defaults-extra-file=", 
-                            defaults_to_use, NullS);
-  }
+  /*
+    All settings have been written to the "upgrade_defaults_path",
+    instruct mysql to only read options from that file
+  */
+  dynstr_append(&cmdline, " ");
+  dynstr_append_os_quoted(&cmdline, "--defaults-file=",
+                          upgrade_defaults_path, NullS);
   dynstr_append(&cmdline, " ");
   dynstr_append_os_quoted(&cmdline, "--force", NullS);
   dynstr_append(&cmdline, " ");
@@ -666,14 +674,15 @@ int main(int argc, char **argv)
   dynstr_append(&cmdline, " ");
   dynstr_append_os_quoted(&cmdline, "--database=mysql", NullS);
   dynstr_append(&cmdline, " < ");
-  dynstr_append_os_quoted(&cmdline, script_line, NullS);
+  dynstr_append_os_quoted(&cmdline, path, NullS);
 #ifdef __WIN__
   dynstr_append(&cmdline, "\"");
 #endif /* __WIN__ */
 
+  /* Execute "mysql --force < mysql_fix_privilege_tables.sql" */
   if (opt_verbose)
     printf("Running %s\n", cmdline.str);
-
+  DBUG_PRINT("info", ("Running: %s", cmdline.str));
   ret= system(cmdline.str);
   if (ret)
     fprintf(stderr, "Error executing '%s'\n", cmdline.str);
@@ -681,10 +690,11 @@ int main(int argc, char **argv)
 error:
   dynstr_free(&cmdline);
 
-  if (upgrade_defaults_created)
-    my_delete(upgrade_defaults_path, MYF(0));
-  
-  my_end(info_flag ? MY_CHECK_ERROR | MY_GIVE_INFO : 0);
+  /* Delete the generated defaults file */
+  my_delete(upgrade_defaults_path, MYF(0));
+
+  free_defaults(defaults_argv);
+  my_end(info_flag ? MY_CHECK_ERROR : 0);
   return ret;
 }