Bug#35132: MySQLadmin --wait ping always crashes on Windows systems

Failing to connect would release parts of the MYSQL struct.
We would then proceed to try again to connect without re-
initializing the struct.

We prevent the unwanted freeing of data we'll still need now.


client/mysqladmin.cc:
  Losing a connection (or not even getting on in the first place) should
  not trash the MYSQL-struct.
  
  Add a lot of comments.
  
  Rewrite re-connection fu.
sql-common/client.c:
  Assert against bad parameters usually caused by de-initing a
  MYSQL-struct without re-initing it again before re-use.

client/mysqladmin.cc

@@ -22,6 +22,7 @@
 #endif
 #include <sys/stat.h>
 #include <mysql.h>
+#include <sql_common.h>
 
 #ifdef LATER_HAVE_NDBCLUSTER_DB
 #include "../ndb/src/mgmclient/ndb_mgmclient.h"
@@ -358,6 +359,11 @@ int main(int argc,char *argv[])
     mysql_options(&mysql, MYSQL_SET_CHARSET_NAME, default_charset);
   if (sql_connect(&mysql, option_wait))
   {
+    /*
+      We couldn't get an initial connection and will definitely exit.
+      The following just determines the exit-code we'll give.
+    */
+
     unsigned int err= mysql_errno(&mysql);
     if (err >= CR_MIN_ERROR && err <= CR_MAX_ERROR)
       error= 1;
@@ -376,30 +382,69 @@ int main(int argc,char *argv[])
   }
   else
   {
+    /*
+      --count=0 aborts right here. Otherwise iff --sleep=t ("interval")
+      is given a t!=0, we get an endless loop, or n iterations if --count=n
+      was given an n!=0. If --sleep wasn't given, we get one iteration.
+
+      To wit, --wait loops the connection-attempts, while --sleep loops
+      the command execution (endlessly if no --count is given).
+    */
+
     while (!interrupted && (!opt_count_iterations || nr_iterations))
     {
       new_line = 0;
-      if ((error=execute_commands(&mysql,argc,commands)))
+
+      if ((error= execute_commands(&mysql,argc,commands)))
       {
+        /*
+          Unknown/malformed command always aborts and can't be --forced.
+          If the user got confused about the syntax, proceeding would be
+          dangerous ...
+        */
 	if (error > 0)
-	  break;				/* Wrong command error */
-	if (!option_force)
+	  break;
+
+        /*
+          Command was well-formed, but failed on the server. Might succeed
+          on retry (if conditions on server change etc.), but needs --force
+          to retry.
+        */
+        if (!option_force)
+          break;
+      }                                         /* if((error= ... */
+
+      if (interval)                             /* --sleep=interval given */
+      {
+        /*
+          If connection was dropped (unintentionally, or due to SHUTDOWN),
+          re-establish it if --wait ("retry-connect") was given and user
+          didn't signal for us to die. Otherwise, signal failure.
+        */
+
+	if (mysql.net.vio == 0)
 	{
 	  if (option_wait && !interrupted)
 	  {
-	    mysql_close(&mysql);
-	    if (!sql_connect(&mysql, option_wait))
-	    {
-	      sleep(1);				/* Don't retry too rapidly */
-	      continue;				/* Retry */
-	    }
+	    sleep(1);
+	    sql_connect(&mysql, option_wait);
+	    /*
+	      continue normally and decrease counters so that
+	      "mysqladmin --count=1 --wait=1 shutdown"
+	      cannot loop endlessly.
+	    */
 	  }
-	  error=1;
-	  break;
-	}
-      }
-      if (interval)
-      {
+	  else
+	  {
+	    /*
+	      connexion broke, and we have no order to re-establish it. fail.
+	    */
+	    if (!option_force)
+	      error= 1;
+	    break;
+	  }
+	}                                       /* lost connection */
+
 	sleep(interval);
 	if (new_line)
 	  puts("");
@@ -407,10 +452,11 @@ int main(int argc,char *argv[])
 	  nr_iterations--;
       }
       else
-	break;
-    }
-    mysql_close(&mysql);
-  }
+        break;                                  /* no --sleep, done looping */
+    }                                           /* command-loop */
+  }                                             /* got connection */
+
+  mysql_close(&mysql);
   my_free(opt_password,MYF(MY_ALLOW_ZERO_PTR));
   my_free(user,MYF(MY_ALLOW_ZERO_PTR));
 #ifdef HAVE_SMEM
@@ -428,6 +474,17 @@ static sig_handler endprog(int signal_number __attribute__((unused)))
   interrupted=1;
 }
 
+/**
+   @brief connect to server, optionally waiting for same to come up
+
+   @param  mysql     connection struct
+   @param  wait      wait for server to come up?
+                     (0: no, ~0: forever, n: cycles)
+
+   @return Operation result
+   @retval 0         success
+   @retval 1         failure
+*/
 
 static my_bool sql_connect(MYSQL *mysql, uint wait)
 {
@@ -436,7 +493,7 @@ static my_bool sql_connect(MYSQL *mysql, uint wait)
   for (;;)
   {
     if (mysql_real_connect(mysql,host,user,opt_password,NullS,tcp_port,
-			   unix_port, 0))
+			   unix_port, CLIENT_REMEMBER_OPTIONS))
     {
       mysql->reconnect= 1;
       if (info)
@@ -447,9 +504,9 @@ static my_bool sql_connect(MYSQL *mysql, uint wait)
       return 0;
     }
 
-    if (!wait)
+    if (!wait)                                  // was or reached 0, fail
     {
-      if (!option_silent)
+      if (!option_silent)                       // print diagnostics
       {
 	if (!host)
 	  host= (char*) LOCAL_HOST;
@@ -473,11 +530,18 @@ static my_bool sql_connect(MYSQL *mysql, uint wait)
       }
       return 1;
     }
+
     if (wait != (uint) ~0)
-      wait--;				/* One less retry */
+      wait--;				/* count down, one less retry */
+
     if ((mysql_errno(mysql) != CR_CONN_HOST_ERROR) &&
 	(mysql_errno(mysql) != CR_CONNECTION_ERROR))
     {
+      /*
+        Error is worse than "server doesn't answer (yet?)";
+        fail even if we still have "wait-coins" unless --force
+        was also given.
+      */
       fprintf(stderr,"Got error: %s\n", mysql_error(mysql));
       if (!option_force)
 	return 1;
@@ -501,11 +565,18 @@ static my_bool sql_connect(MYSQL *mysql, uint wait)
 }
 
 
-/*
-  Execute a command.
-  Return 0 on ok
-	 -1 on retryable error
-	 1 on fatal error
+/**
+   @brief Execute all commands
+
+   @details We try to execute all commands we were given, in the order
+            given, but return with non-zero as soon as we encounter trouble.
+            By that token, individual commands can be considered a conjunction
+            with boolean short-cut.
+
+   @return success?
+   @retval 0       Yes!  ALL commands worked!
+   @retval 1       No, one failed and will never work (malformed): fatal error!
+   @retval -1      No, one failed on the server, may work next time!
 */
 
 static int execute_commands(MYSQL *mysql,int argc, char **argv)
@@ -575,7 +646,6 @@ static int execute_commands(MYSQL *mysql,int argc, char **argv)
 			mysql_error(mysql));
 	return -1;
       }
-      mysql_close(mysql);	/* Close connection to avoid error messages */
       argc=1;                   /* force SHUTDOWN to be the last command    */
       if (got_pidfile)
       {

sql-common/client.c

@@ -416,6 +416,15 @@ HANDLE create_shared_memory(MYSQL *mysql,NET *net, uint connect_timeout)
   const char *prefix;
   int i;
 
+  /*
+    If this is NULL, somebody freed the MYSQL* options.  mysql_close()
+    is a good candidate.  We don't just silently (re)set it to
+    def_shared_memory_base_name as that would create really confusing/buggy
+    behavior if the user passed in a different name on the command-line or
+    in a my.cnf.
+  */
+  DBUG_ASSERT(shared_memory_base_name != NULL);
+
   /*
      get enough space base-name + '_' + longest suffix we might ever send
    */