minor post review fixes

server-tools/instance-manager/buffer.cc

@@ -26,15 +26,15 @@
   Puts the given string to the buffer.
 
   SYNOPSYS
-    put_to_buffer()
-    start_pos         start position in the buffer
+    append()
+    position         start position in the buffer
     string            string to be put in the buffer
     len_arg           the length of the string. This way we can avoid some
                       strlens.
 
   DESCRIPTION
 
-    The method puts a string into the buffer, starting from start_pos .
+    The method puts a string into the buffer, starting from position .
     In the case when the buffer is too small it reallocs the buffer. The
     total size of the buffer is restricted with 16.
 
@@ -43,12 +43,12 @@
     1 - The buffer came to 16Mb barrier
 */
 
-int Buffer::put_to_buffer(char *start_pos, const char *string, uint len_arg)
+int Buffer::append(char *position, const char *string, uint len_arg)
 {
-  if (check_and_add(start_pos - buffer, len_arg))
+  if (reserve(position - buffer, len_arg))
     return 1;
 
-  strnmov(start_pos, string, len_arg);
+  strnmov(position, string, len_arg);
   return 0;
 }
 
@@ -58,7 +58,7 @@ int Buffer::put_to_buffer(char *start_pos, const char *string, uint len_arg)
   "len_arg" starting from "position" and reallocs it if no.
 
   SYNOPSYS
-    check_and_add()
+    reserve()
     position          the number starting byte on the buffer to store a buffer
     len_arg           the length of the string.
 
@@ -74,7 +74,7 @@ int Buffer::put_to_buffer(char *start_pos, const char *string, uint len_arg)
     1 - The buffer came to 16Mb barrier
 */
 
-int Buffer::check_and_add(uint position, uint len_arg)
+int Buffer::reserve(uint position, uint len_arg)
 {
   if (position + len_arg >= MAX_BUFFER_SIZE)
     return 1;
@@ -83,9 +83,9 @@ int Buffer::check_and_add(uint position, uint len_arg)
   {
     buffer= (char *) realloc(buffer,
                              min(MAX_BUFFER_SIZE,
-                                 max((uint) buffer_size*1.5,
+                                 max((uint) (buffer_size*1.5),
                                      position + len_arg)));
-    buffer_size= (uint) buffer_size*1.5;
+    buffer_size= (uint) (buffer_size*1.5);
   }
   return 0;
 }

server-tools/instance-manager/buffer.h

@@ -50,8 +50,8 @@ public:
 
 public:
   char *buffer;
-  int put_to_buffer(char *start_pos, const char *string, uint len_arg);
-  int check_and_add(uint position, uint len_arg);
+  int append(char *start_pos, const char *string, uint len_arg);
+  int reserve(uint position, uint len_arg);
 };
 
 #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_BUFFER_H */

server-tools/instance-manager/command.cc

@@ -20,16 +20,6 @@
 
 #include "command.h"
 
-#include <my_global.h>
-#include <my_sys.h>
-#include <m_ctype.h>
-#include <m_string.h>
-#include <mysql_com.h>
-#include <mysqld_error.h>
-
-#include "log.h"
-#include "protocol.h"
-#include "instance_map.h"
 
 Command::Command(Command_factory *factory_arg)
   :factory(factory_arg)

server-tools/instance-manager/command.h

@@ -22,7 +22,7 @@
 
 #include <my_global.h>
 
-/* Class responsible for allocation and deallocation of im classes. */
+/* Class responsible for allocation of im commands. */
 
 class Command_factory;
 
@@ -44,6 +44,4 @@ protected:
   Command_factory *factory;
 };
 
-#define CONST_STR(a) String(a,sizeof(a),&my_charset_latin1)
-
 #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_COMMAND_H */

server-tools/instance-manager/commands.cc

@@ -54,11 +54,12 @@ Show_instance_status::Show_instance_status(Command_factory *factory,
   Instance *instance;
 
   /* we make a search here, since we don't want t store the name */
-  if (instance= (factory->instance_map).find(name, len))
+  if (instance= factory->instance_map.find(name, len))
   {
     instance_name= instance->options.instance_name;
   }
-  else instance_name= NULL;
+  else
+    instance_name= NULL;
 }
 
 
@@ -90,7 +91,8 @@ Show_instance_options::Show_instance_options(Command_factory *factory,
   {
     instance_name= instance->options.instance_name;
   }
-  else instance_name= NULL;
+  else
+    instance_name= NULL;
 }
 
 
@@ -116,7 +118,7 @@ Start_instance::Start_instance(Command_factory *factory,
   :Command(factory)
 {
   /* we make a search here, since we don't want t store the name */
-  if (instance= (factory->instance_map).find(name, len))
+  if (instance= factory->instance_map.find(name, len))
     instance_name= instance->options.instance_name;
 }
 
@@ -150,7 +152,7 @@ Stop_instance::Stop_instance(Command_factory *factory,
   :Command(factory)
 {
   /* we make a search here, since we don't want t store the name */
-  if (instance= (factory->instance_map).find(name, len))
+  if (instance= factory->instance_map.find(name, len))
     instance_name= instance->options.instance_name;
 }
 

server-tools/instance-manager/commands.h

@@ -120,9 +120,6 @@ public:
 class Syntax_error : public Command
 {
 public:
-  Syntax_error()
-  {}
-
   int execute(struct st_net *net, ulong connection_id);
 };
 

server-tools/instance-manager/guardian.cc

@@ -70,16 +70,13 @@ Guardian_thread::~Guardian_thread()
 
     Check for all guarded instances and restart them if needed. If everything
     is fine go and sleep for some time.
-
-  RETURN
-    The function return no value
 */
 
 void Guardian_thread::run()
 {
   Instance *instance;
   LIST *loop;
-  int i=0;
+  int i= 0;
 
   my_thread_init();
 
@@ -90,11 +87,8 @@ void Guardian_thread::run()
     while (loop != NULL)
     {
       instance= (Instance *) loop->data;
-      if (instance != NULL)
-      {
-        if (!instance->is_running())
-          instance->start();
-      }
+      /* instance-> start already checks whether instance is running */
+      instance->start();
       loop= loop->next;
     }
     pthread_mutex_unlock(&LOCK_guardian);
@@ -124,17 +118,18 @@ void Guardian_thread::run()
 
 int Guardian_thread::guard(const char *instance_name, uint name_len)
 {
-  LIST *lst;
+  LIST *node;
   Instance *instance;
 
-  lst= (LIST *) alloc_root(&alloc, sizeof(LIST));
-  if (lst == NULL) return 1;
+  node= (LIST *) alloc_root(&alloc, sizeof(LIST));
+  if (node == NULL)
+    return 1;
   instance= instance_map->find(instance_name, name_len);
   /* we store the pointers to instances from the instance_map's MEM_ROOT */
-  lst->data= (void *) instance;
+  node->data= (void *) instance;
 
   pthread_mutex_lock(&LOCK_guardian);
-  guarded_instances= list_add(guarded_instances, lst);
+  guarded_instances= list_add(guarded_instances, node);
   pthread_mutex_unlock(&LOCK_guardian);
 
   return 0;
@@ -150,28 +145,28 @@ int Guardian_thread::guard(const char *instance_name, uint name_len)
 
 int Guardian_thread::stop_guard(const char *instance_name, uint name_len)
 {
-  LIST *lst;
+  LIST *node;
   Instance *instance;
 
   instance= instance_map->find(instance_name, name_len);
 
-  lst= guarded_instances;
-  if (lst == NULL) return 1;
-
   pthread_mutex_lock(&LOCK_guardian);
-  while (lst != NULL)
+  node= guarded_instances;
+
+  while (node != NULL)
   {
     /*
       We compare only pointers, as we always use pointers from the
       instance_map's MEM_ROOT.
     */
-    if ((Instance *) lst->data == instance)
+    if ((Instance *) node->data == instance)
     {
-      guarded_instances= list_delete(guarded_instances, lst);
+      guarded_instances= list_delete(guarded_instances, node);
       pthread_mutex_unlock(&LOCK_guardian);
       return 0;
     }
-    else lst= lst->next;
+    else
+      node= node->next;
   }
   pthread_mutex_unlock(&LOCK_guardian);
   /* if there is nothing to delete it is also fine */

server-tools/instance-manager/instance.cc

@@ -49,8 +49,8 @@ int Instance::start()
          exit(0); /* parent goes bye-bye */
        else
        {
-        execv(options.mysqld_path, options.argv);
-        exit(1);
+         execv(options.mysqld_path, options.argv);
+         exit(1);
        }
     case -1:
       return ER_CANNOT_START_INSTANCE;

server-tools/instance-manager/instance_options.cc

@@ -172,7 +172,7 @@ err:
 
 int Instance_options::add_to_argv(const char* option)
 {
-  DBUG_ASSERT(filled_default_options < (MAX_NUMBER_OF_DEFAULT_OPTIONS + 1));
+  DBUG_ASSERT(filled_default_options < MAX_NUMBER_OF_DEFAULT_OPTIONS);
 
   if (option != NULL)
     argv[filled_default_options++]= (char *) option;

server-tools/instance-manager/instance_options.h

@@ -36,9 +36,10 @@
 class Instance_options
 {
 public:
-  Instance_options() : mysqld_socket(0), mysqld_datadir(0),
-  mysqld_bind_address(0), mysqld_pid_file(0), mysqld_port(0), mysqld_path(0),
-  mysqld_user(0), mysqld_password(0), is_guarded(0), filled_default_options(0)
+  Instance_options() :
+    mysqld_socket(0), mysqld_datadir(0), mysqld_bind_address(0),
+    mysqld_pid_file(0), mysqld_port(0), mysqld_path(0), mysqld_user(0),
+    mysqld_password(0), is_guarded(0), filled_default_options(0)
   {}
   ~Instance_options();
   /* fills in argv */
@@ -50,7 +51,7 @@ public:
   int init(const char *instance_name_arg);
 
 public:
-  enum { MAX_NUMBER_OF_DEFAULT_OPTIONS= 3 };
+  enum { MAX_NUMBER_OF_DEFAULT_OPTIONS= 1 };
   enum { MEM_ROOT_BLOCK_SIZE= 512 };
   char **argv;
   /* We need the some options, so we store them as a separate pointers */

server-tools/instance-manager/listener.cc

@@ -78,9 +78,9 @@ Listener_thread::~Listener_thread()
 
 void Listener_thread::run()
 {
-  enum { LISTEN_BACK_LOG_SIZE = 5 };          // standard backlog size
+  enum { LISTEN_BACK_LOG_SIZE = 5 };            // standard backlog size
   int flags;
-  int arg= 1;                                   /* value to be set by setsockopt */
+  int arg= 1;                             /* value to be set by setsockopt */
   /* I. prepare 'listen' sockets */
 
   int ip_socket= socket(AF_INET, SOCK_STREAM, 0);
@@ -93,7 +93,7 @@ void Listener_thread::run()
   }
 
   struct sockaddr_in ip_socket_address;
-  memset(&ip_socket_address, 0, sizeof(ip_socket_address));
+  bzero(&ip_socket_address, sizeof(ip_socket_address));
 
   ulong im_bind_addr;
   if (options.bind_address != 0)
@@ -101,7 +101,8 @@ void Listener_thread::run()
     if ((im_bind_addr= (ulong) inet_addr(options.bind_address)) == INADDR_NONE)
       im_bind_addr= htonl(INADDR_ANY);
   }
-  else im_bind_addr= htonl(INADDR_ANY);
+  else
+    im_bind_addr= htonl(INADDR_ANY);
   uint im_port= options.port_number;
 
   ip_socket_address.sin_family= AF_INET;
@@ -144,7 +145,7 @@ void Listener_thread::run()
   }
 
   struct sockaddr_un unix_socket_address;
-  memset(&unix_socket_address, 0, sizeof(unix_socket_address));
+  bzero(&unix_socket_address, sizeof(unix_socket_address));
 
   unix_socket_address.sun_family= AF_UNIX;
   strmake(unix_socket_address.sun_path, options.socket_file_name,

server-tools/instance-manager/manager.cc

@@ -30,6 +30,23 @@
 #include "log.h"
 #include "guardian.h"
 
+static int create_pid_file(const char *pid_file_name)
+{
+  if (FILE *pid_file= my_fopen(pid_file_name,
+                               O_WRONLY | O_CREAT | O_BINARY, MYF(0)))
+    {
+      fprintf(pid_file, "%d\n", (int) getpid());
+      my_fclose(pid_file, MYF(0));
+    }
+    else
+    {
+      log_error("can't create pid file %s: errno=%d, %s",
+                pid_file_name, errno, strerror(errno));
+      return 1;
+    }
+  return 0;
+}
+
 
 /*
   manager - entry point to the main instance manager process: start
@@ -53,32 +70,24 @@ void manager(const Options &options)
                                   &instance_map,
                                   options.monitoring_interval);
 
+  Listener_thread_args listener_args(thread_registry, options, user_map,
+                                     instance_map);
+
   instance_map.mysqld_path= options.default_mysqld_path;
   instance_map.user= options.default_admin_user;
   instance_map.password= options.default_admin_password;
   instance_map.guardian= &guardian_thread;
-  instance_map.load();
 
-  Listener_thread_args listener_args(thread_registry, options, user_map,
-                                     instance_map);
 
+  if (instance_map.load())
+    return;
 
   if (user_map.load(options.password_file_name))
     return;
 
   /* write pid file */
-  if (FILE *pid_file= my_fopen(options.pid_file_name,
-                               O_WRONLY | O_CREAT | O_BINARY, MYF(0)))
-  {
-    fprintf(pid_file, "%d\n", (int) getpid());
-    my_fclose(pid_file, MYF(0));
-  }
-  else
-  {
-    log_error("can't create pid file %s: errno=%d, %s",
-              options.pid_file_name, errno, strerror(errno));
+  if (create_pid_file(options.pid_file_name))
     return;
-  }
 
   /* block signals */
   sigset_t mask;

server-tools/instance-manager/protocol.cc

@@ -105,9 +105,9 @@ void store_to_string(Buffer *buf, const char *string, uint *position)
   uint string_len;
 
   string_len= strlen(string);
-  buf->check_and_add(*position, 2);
+  buf->reserve(*position, 2);
   currpos= net_store_length(buf->buffer + *position, string_len);
-  buf->put_to_buffer(currpos, string, string_len);
+  buf->append(currpos, string, string_len);
   *position= *position + string_len + (currpos - buf->buffer - *position);
 }
 
@@ -147,7 +147,7 @@ int send_fields(struct st_net *net, LIST *fields)
     store_to_string(&send_buff, (char *) "", &position); /* table name alias */
     store_to_string(&send_buff, field->name, &position); /* column name */
     store_to_string(&send_buff, field->name, &position); /* column name alias */
-    send_buff.check_and_add(position, 12);
+    send_buff.reserve(position, 12);
     send_buff.buffer[position++]= 12;
     int2store(send_buff.buffer + position, 1); /* charsetnr */
     int4store(send_buff.buffer + position + 2, field->length); /* field length */