post-review fixes

server-tools/instance-manager/commands.cc:
  remove commented out code
server-tools/instance-manager/instance.cc:
  use flag instead of int variable
server-tools/instance-manager/instance.h:
  no more default values
server-tools/instance-manager/instance_map.cc:
  use flag to be more verbose
server-tools/instance-manager/instance_options.cc:
  don't read options when looking for an option, use strmake instead of strchr
server-tools/instance-manager/instance_options.h:
  fix comment, use flag instead of int value
server-tools/instance-manager/listener.cc:
  don't like c++ comments
server-tools/instance-manager/log.cc:
  safety: strmake adds trailing zero to the string
server-tools/instance-manager/parse_output.cc:
  use strmake instead of strncpy, renamed varianles to make code more readable
server-tools/instance-manager/parse_output.h:
  get rid of default value

server-tools/instance-manager/commands.cc

@@ -644,10 +644,6 @@ Set_option::Set_option(Instance_map *instance_map_arg,
     {
       strmake(option, option_arg, option_len_arg);
       strmake(option_value, option_value_arg, option_value_len_arg);
-/*    strncpy(option, option_arg, option_len_arg);
-      option[option_len_arg]= 0;
-      strncpy(option_value, option_value_arg, option_value_len_arg);
-      option_value[option_value_len_arg]= 0; */
     }
     else
     {

server-tools/instance-manager/instance.cc

@@ -326,8 +326,8 @@ int Instance::init(const char *name_arg)
 
 int Instance::complete_initialization(Instance_map *instance_map_arg,
                                       const char *mysqld_path,
-                                      int only_instance)
+                                      uint instance_type)
 {
   instance_map= instance_map_arg;
-  return options.complete_initialization(mysqld_path, only_instance);
+  return options.complete_initialization(mysqld_path, instance_type);
 }

server-tools/instance-manager/instance.h

@@ -33,7 +33,7 @@ class Instance
   ~Instance();
   int init(const char *name);
   int complete_initialization(Instance_map *instance_map_arg,
-                              const char *mysqld_path, int only_instance= 0);
+                              const char *mysqld_path, uint instance_type);
 
   bool is_running();
   int start();

server-tools/instance-manager/instance_map.cc

@@ -202,14 +202,14 @@ int Instance_map::complete_initialization()
       hash_free should handle it's deletion => goto err, not
       err_instance.
     */
-    if (instance->complete_initialization(this, mysqld_path, 1))
+    if (instance->complete_initialization(this, mysqld_path, DEFAULT_SINGLE_INSTANCE))
       goto err;
   }
   else
     while (i < hash.records)
     {
       instance= (Instance *) hash_element(&hash, i);
-      if (instance->complete_initialization(this, mysqld_path))
+      if (instance->complete_initialization(this, mysqld_path, USUAL_INSTANCE))
         goto err;
       i++;
     }

server-tools/instance-manager/instance_options.cc

@@ -95,7 +95,7 @@ int Instance_options::get_default_option(char *result, size_t result_len,
 
   /* +2 eats first "--" from the option string (E.g. "--datadir") */
   rc= parse_output_and_get_value(cmd.buffer, option_name + 2,
-                                   result, result_len);
+                                 result, result_len, GET_VALUE);
 
   return rc;
 err:
@@ -121,9 +121,8 @@ int Instance_options::get_default_option(char *result, size_t result_len,
 int Instance_options::fill_instance_version()
 {
   enum { MAX_VERSION_STRING_LENGTH= 160 };
-  enum { RETURN_LINE= 1 };
   char result[MAX_VERSION_STRING_LENGTH];
-  char version_option[]= " --version";
+  char version_option[]= " --no-defaults --version";
   int rc= 1;
   Buffer cmd(mysqld_path_len + sizeof(version_option));
 
@@ -133,7 +132,7 @@ int Instance_options::fill_instance_version()
 
   rc= parse_output_and_get_value(cmd.buffer, mysqld_path,
                                  result, MAX_VERSION_STRING_LENGTH,
-                                 RETURN_LINE);
+                                 GET_LINE);
 
   if (*result != '\0')
   {
@@ -198,8 +197,8 @@ int Instance_options::fill_log_options()
       goto err;
   }
   else           /* below is safe, as --datadir always has a value */
-    strncpy(datadir, strchr(mysqld_datadir, '=') + 1,
-            MAX_LOG_OPTION_LENGTH);
+    strmake(datadir, strchr(mysqld_datadir, '=') + 1,
+            MAX_LOG_OPTION_LENGTH - 1);
 
   if (gethostname(hostname,sizeof(hostname)-1) < 0)
     strmov(hostname, "mysql");
@@ -232,7 +231,7 @@ int Instance_options::fill_log_options()
           if ((MAX_LOG_OPTION_LENGTH - strlen(full_name)) >
               strlen(log_files->default_suffix))
           {
-            strcpy(full_name + strlen(full_name),
+            strmov(full_name + strlen(full_name),
                    log_files->default_suffix);
           }
           else
@@ -338,7 +337,7 @@ pid_t Instance_options::get_pid()
 
 
 int Instance_options::complete_initialization(const char *default_path,
-                                              int only_instance)
+                                              uint instance_type)
 {
   const char *tmp;
 
@@ -369,18 +368,23 @@ int Instance_options::complete_initialization(const char *default_path,
       found, we would like to model mysqld pid file values.
     */
     if (!gethostname(hostname, sizeof(hostname) - 1))
-      (only_instance == 0) ?
-      strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", instance_name, "-",
-               hostname, ".pid", NullS):
-      strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", hostname,
-               ".pid", NullS);
-
+    {
+      if (instance_type & DEFAULT_SINGLE_INSTANCE)
+        strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", instance_name, "-",
+                 hostname, ".pid", NullS);
+      else
+        strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", hostname,
+                 ".pid", NullS);
+    }
     else
-      (only_instance == 0) ?
-      strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", instance_name,
-               ".pid", NullS):
-      strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", "mysql",
-               ".pid", NullS);
+    {
+      if (instance_type & DEFAULT_SINGLE_INSTANCE)
+        strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", instance_name,
+                 ".pid", NullS);
+      else
+        strxnmov(pidfilename, MAX_PATH_LEN - 1, "--pid-file=", "mysql",
+                 ".pid", NullS);
+    }
 
     add_option(pidfilename);
   }

server-tools/instance-manager/instance_options.h

@@ -34,6 +34,8 @@
   don't have to synchronize between threads.
 */
 
+enum { USUAL_INSTANCE= 0, DEFAULT_SINGLE_INSTANCE };
+
 class Instance_options
 {
 public:
@@ -45,7 +47,7 @@ class Instance_options
   {}
   ~Instance_options();
   /* fills in argv */
-  int complete_initialization(const char *default_path, int only_instance);
+  int complete_initialization(const char *default_path, uint instance_type);
 
   int add_option(const char* option);
   int init(const char *instance_name_arg);
@@ -66,7 +68,7 @@ class Instance_options
   char **argv;
   /*
     Here we cache the version string, obtained from mysqld --version.
-    In the case when mysqld binary is not found we get "unknown" here.
+    In the case when mysqld binary is not found we get NULL here.
   */
   const char *mysqld_version;
   /* We need the some options, so we store them as a separate pointers */

server-tools/instance-manager/listener.cc

@@ -163,7 +163,7 @@ void Listener_thread::run()
   unix_socket_address.sun_family= AF_UNIX;
   strmake(unix_socket_address.sun_path, options.socket_file_name,
           sizeof(unix_socket_address.sun_path));
-  unlink(unix_socket_address.sun_path); // in case we have stale socket file
+  unlink(unix_socket_address.sun_path); /* in case we have stale socket file */
 
   {
     /*

server-tools/instance-manager/log.cc

@@ -76,7 +76,7 @@ static inline void log(FILE *file,  const char *format, va_list args)
       if (buff_msg == 0)
       {
         strmake(buff_stack, "log(): message is too big, my_malloc() failed",
-                sizeof(buff_stack));
+                sizeof(buff_stack) - 1);
         buff_msg= buff_stack;
         break;
       }

server-tools/instance-manager/parse_output.cc

@@ -16,10 +16,11 @@
 
 #include <my_global.h>
 #include "parse.h"
+#include "parse_output.h"
 
 #include <stdio.h>
 #include <my_sys.h>
-#include <string.h>
+#include <m_string.h>
 
 
 /*
@@ -28,17 +29,17 @@
   SYNOPSYS
     parse_output_and_get_value()
 
-    command      the command to execue with popen.
-    word         the word to look for (usually an option name)
-    result       the buffer to store the next word (option value)
-    result_len   self-explanatory
-    get_all_line flag, which is set if we want to get all the line after
-                 the matched word.
+    command          the command to execue with popen.
+    word             the word to look for (usually an option name)
+    result           the buffer to store the next word (option value)
+    input_buffer_len self-explanatory
+    flag             this equals to GET_LINE if we want to get all the line after
+                     the matched word and GET_VALUE otherwise.
 
   DESCRIPTION
 
     Parse output of the "command". Find the "word" and return the next one
-    if get_all_line is 0. Return the rest of the parsed string otherwise.
+    if flag is GET_VALUE. Return the rest of the parsed string otherwise.
 
   RETURN
     0 - ok
@@ -46,8 +47,8 @@
 */
 
 int parse_output_and_get_value(const char *command, const char *word,
-                               char *result, size_t result_len,
-                               int get_all_line)
+                               char *result, size_t input_buffer_len,
+                               uint flag)
 {
   FILE *output;
   uint wordlen;
@@ -68,7 +69,7 @@ int parse_output_and_get_value(const char *command, const char *word,
 
   while (fgets(linebuf, sizeof(linebuf) - 1, output))
   {
-    uint lineword_len= 0;
+    uint found_word_len= 0;
     char *linep= linebuf;
 
     linebuf[sizeof(linebuf) - 1]= '\0';        /* safety */
@@ -77,26 +78,24 @@ int parse_output_and_get_value(const char *command, const char *word,
       Get the word, which might contain non-alphanumeric characters. (Usually
       these are '/', '-' and '.' in the path expressions and filenames)
     */
-    get_word((const char **) &linep, &lineword_len, NONSPACE);
+    get_word((const char **) &linep, &found_word_len, NONSPACE);
     if (!strncmp(word, linep, wordlen))
     {
       /*
-        If we have found the word, return the next one. This is usually
-        an option value.
+        If we have found the word, return the next one (this is usually
+        an option value) or the whole line (if flag)
       */
-      linep+= lineword_len;                     /* swallow the previous one */
-      if (!get_all_line)
+      linep+= found_word_len;                     /* swallow the previous one */
+      if (flag & GET_VALUE)    /* not GET_LINE */
       {
-        get_word((const char **) &linep, &lineword_len, NONSPACE);
-        if (result_len <= lineword_len)
+        get_word((const char **) &linep, &found_word_len, NONSPACE);
+        if (input_buffer_len <= found_word_len)
           goto err;
-        strncpy(result, linep, lineword_len);
-        result[lineword_len]= '\0';
+        strmake(result, linep, found_word_len);
       }
-      else
+      else         /* currently there are only two options */
       {
-        strncpy(result, linep, result_len);
-        result[result_len]= '\0'; /* safety */
+        strmake(result, linep, input_buffer_len - 1);
       }
       goto pclose;
     }

server-tools/instance-manager/parse_output.h

@@ -16,8 +16,10 @@
    along with this program; if not, write to the Free Software
    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
 
+enum { GET_VALUE= 1, GET_LINE };
+
 int parse_output_and_get_value(const char *command, const char *word,
-                               char *result, size_t result_len,
-                               int get_all_line= 0);
+                               char *result, size_t input_buffer_len,
+                               uint flag);
 
 #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_PARSE_OUTPUT_H */