Fixed compilation failure on MacOSX

Due to a hack that has propagated to the maria storage engine, undefined
behaviour would result by bypassing the initialization code of variables
after my_thread_init().

By refactoring the nested logic into a separate function, this problem
is resolved.

include/myisam.h

@@ -331,7 +331,8 @@ typedef struct st_sort_info
   my_off_t filelength, dupp, buff_length;
   ha_rows max_records;
   uint current_key, total_keys;
-  uint got_error, threads_running;
+  volatile uint got_error;
+  uint threads_running;
   myf myf_rw;
   enum data_file_type new_data_file_type;
 } MI_SORT_INFO;

storage/maria/ma_sort.c

@@ -364,46 +364,41 @@ static ha_rows find_all_keys(MARIA_SORT_PARAM *info, ha_rows keys,
 } /* find_all_keys */
 
 
-/* Search after all keys and place them in a temp. file */
-
-pthread_handler_t _ma_thr_find_all_keys(void *arg)
+static my_bool _ma_thr_find_all_keys_exec(MARIA_SORT_PARAM* sort_param)
 {
-  MARIA_SORT_PARAM *sort_param= (MARIA_SORT_PARAM*) arg;
-  int error;
-  size_t memavl, old_memavl;
-  longlong sortbuff_size;
-  ha_keys UNINIT_VAR(keys), idx;
-  uint sort_length;
-  uint maxbuffer;
-  uchar **sort_keys=0;
-
-  error=1;
-
-  if (my_thread_init())
-    goto err;
-
-  { /* Add extra block since DBUG_ENTER declare variables */
   DBUG_ENTER("_ma_thr_find_all_keys");
   DBUG_PRINT("enter", ("master: %d", sort_param->master));
+
+  my_bool error= FALSE;
   if (sort_param->sort_info->got_error)
-      goto err;
+    error= TRUE;
+  if (error)
+    DBUG_RETURN(error);
 
   set_sort_param_read_write(sort_param);
 
+  ulonglong memavl, old_memavl, sortbuff_size;
+  ha_keys UNINIT_VAR(keys), idx;
+  uint sort_length;
+  uint maxbuffer;
+  uchar **sort_keys= NULL;
+
   my_b_clear(&sort_param->tempfile);
   my_b_clear(&sort_param->tempfile_for_exceptions);
-    bzero((char*) &sort_param->buffpek,sizeof(sort_param->buffpek));
+  bzero((char*) &sort_param->buffpek, sizeof(sort_param->buffpek));
   bzero((char*) &sort_param->unique,  sizeof(sort_param->unique));
 
+
   sortbuff_size= sort_param->sortbuff_size;
-    memavl=       MY_MAX(sortbuff_size, MIN_SORT_MEMORY);
+  memavl=        MY_MAX(sortbuff_size, MIN_SORT_BUFFER);
   idx=           (ha_keys) sort_param->sort_info->max_records;
   sort_length=   sort_param->key_length;
   maxbuffer=     1;
 
-    while (memavl >= MIN_SORT_MEMORY)
+  while (memavl >= MIN_SORT_BUFFER)
   {
-      if ((my_off_t) (idx+1)*(sort_length+sizeof(char*)) <= (my_off_t) memavl)
+    if ((my_off_t) (idx+1)*(sort_length+sizeof(char*)) <=
+        (my_off_t) memavl)
       keys= idx+1;
     else if ((sort_param->sort_info->param->testflag &
              (T_FORCE_SORT_MEMORY | T_CREATE_MISSING_KEYS)) ==
@@ -426,12 +421,16 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg)
         if (memavl < sizeof(BUFFPEK)*maxbuffer ||
             (keys=(memavl-sizeof(BUFFPEK)*maxbuffer)/
              (sort_length+sizeof(char*))) <= 1 ||
-              keys < maxbuffer)
+            keys < (uint) maxbuffer)
         {
+          mysql_mutex_lock(&sort_param->sort_info->mutex);
           _ma_check_print_error(sort_param->sort_info->param,
-                                  "aria_sort_buffer_size is too small. Current aria_sort_buffer_size: %llu  rows: %llu  sort_length: %u",
+                                "aria_sort_buffer_size is too small. Current "
+                                "aria_sort_buffer_size: %llu  rows: %llu  "
+                                "sort_length: %u",
                                 sortbuff_size, (ulonglong) idx, sort_length);
-            goto err;
+          mysql_mutex_unlock(&sort_param->sort_info->mutex);
+          DBUG_RETURN(TRUE);
         }
       }
       while ((maxbuffer= (uint) (idx/(keys-1)+1)) != maxbuffer_org);
@@ -445,7 +444,7 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg)
                                 maxbuffer, MY_MIN(maxbuffer/2, 1000), MYF(0)))
       {
         my_free(sort_keys);
-          sort_keys= (uchar **) NULL;            /* for err: label */
+        sort_keys= NULL;          /* Safety against double free on error. */
       }
       else
         break;
@@ -458,14 +457,17 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg)
   if (memavl < MIN_SORT_MEMORY)
   {
     /* purecov: begin inspected */
+    mysql_mutex_lock(&sort_param->sort_info->mutex);
     _ma_check_print_error(sort_param->sort_info->param,
-                            "aria_sort_buffer_size is too small. Current aria_sort_buffer_size: %llu  rows: %llu  sort_length: %u",
+                          "aria_sort_buffer_size is too small. Current "
+                          "aria_sort_buffer_size: %llu  rows: %llu  "
+                          "sort_length: %u",
                           sortbuff_size, (ulonglong) idx, sort_length);
     my_errno= ENOMEM;
-      goto err;
+    mysql_mutex_unlock(&sort_param->sort_info->mutex);
+    DBUG_RETURN(TRUE);
     /* purecov: end inspected */
   }
-
   if (sort_param->sort_info->param->testflag & T_VERBOSE)
     my_fprintf(stdout,
                "Key %d - Allocating buffer for %llu keys\n",
@@ -490,13 +492,11 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg)
     if (++idx == keys)
     {
       if (sort_param->write_keys(sort_param, sort_keys, idx - 1,
-                                   (BUFFPEK *)alloc_dynamic(&sort_param->
-                                                            buffpek),
+                                 (BUFFPEK *)alloc_dynamic(&sort_param->buffpek),
                                  &sort_param->tempfile))
         goto err;
       sort_keys[0]= (uchar*) (sort_keys+keys);
-        memcpy(sort_keys[0], sort_keys[idx - 1],
-               (size_t) sort_param->key_length);
+      memcpy(sort_keys[0], sort_keys[idx - 1], (size_t) sort_param->key_length);
       idx= 1;
     }
     sort_keys[idx]= sort_keys[idx - 1] + sort_param->key_length;
@@ -515,18 +515,35 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg)
   else
     sort_param->keys= idx;
 
-    goto ok;
+  DBUG_RETURN(FALSE);
 
 err:
   DBUG_PRINT("error", ("got some error"));
-    sort_param->sort_info->got_error= 1; /* no need to protect with a mutex */
   my_free(sort_keys);
   sort_param->sort_keys= 0;
   delete_dynamic(& sort_param->buffpek);
   close_cached_file(&sort_param->tempfile);
   close_cached_file(&sort_param->tempfile_for_exceptions);
 
-ok:
+  DBUG_RETURN(TRUE);
+}
+
+/* Search after all keys and place them in a temp. file */
+
+pthread_handler_t _ma_thr_find_all_keys(void *arg)
+{
+  MARIA_SORT_PARAM *sort_param= (MARIA_SORT_PARAM*) arg;
+  my_bool error= FALSE;
+  /* If my_thread_init fails */
+  if (my_thread_init())
+    error= TRUE;
+
+  if (error || _ma_thr_find_all_keys_exec(sort_param))
+    error= TRUE;
+
+  /*
+     Thread must clean up after itself.
+  */
   free_root(&sort_param->wordroot, MYF(0));
   /*
      Detach from the share if the writer is involved. Avoid others to
@@ -545,11 +562,13 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg)
     remove_io_thread(&sort_param->read_cache);
 
   mysql_mutex_lock(&sort_param->sort_info->mutex);
+  if (error)
+    sort_param->sort_info->got_error= 1;
+
   if (!--sort_param->sort_info->threads_running)
     mysql_cond_signal(&sort_param->sort_info->cond);
   mysql_mutex_unlock(&sort_param->sort_info->mutex);
-    DBUG_PRINT("exit", ("======== ending thread ========"));
-  }
+
   my_thread_end();
   return NULL;
 }

storage/maria/maria_def.h

@@ -67,7 +67,8 @@ typedef struct st_maria_sort_info
   pgcache_page_no_t page;
   ha_rows max_records;
   uint current_key, total_keys;
-  uint got_error, threads_running;
+  volatile uint got_error;
+  uint threads_running;
   myf myf_rw;
   enum data_file_type new_data_file_type, org_data_file_type;
 } MARIA_SORT_INFO;

storage/myisam/sort.c

@@ -344,36 +344,30 @@ static ha_rows find_all_keys(MI_SORT_PARAM *info, ha_rows keys,
   DBUG_RETURN((*maxbuffer)*(keys-1)+idx);
 } /* find_all_keys */
 
-/* Search after all keys and place them in a temp. file */
-
-pthread_handler_t thr_find_all_keys(void *arg)
+static my_bool thr_find_all_keys_exec(MI_SORT_PARAM *sort_param)
 {
-  MI_SORT_PARAM *sort_param= (MI_SORT_PARAM*) arg;
-  int error;
-  ulonglong memavl, old_memavl, sortbuff_size;
-  ha_keys UNINIT_VAR(keys), idx;
-  uint sort_length;
-  uint maxbuffer;
-  uchar **sort_keys=0;
-
-  error=1;
-
-  if (my_thread_init())
-    goto err;
-
-  { /* Add extra block since DBUG_ENTER declare variables */
   DBUG_ENTER("thr_find_all_keys");
   DBUG_PRINT("enter", ("master: %d", sort_param->master));
+
+  my_bool error= FALSE;
   if (sort_param->sort_info->got_error)
-      goto err;
+    error= TRUE;
+  if (error)
+    DBUG_RETURN(error);
 
   set_sort_param_read_write(sort_param);
 
+  ulonglong memavl, old_memavl, sortbuff_size;
+  ha_keys UNINIT_VAR(keys), idx;
+  uint sort_length;
+  uint maxbuffer;
+  uchar **sort_keys= NULL;
+
   my_b_clear(&sort_param->tempfile);
   my_b_clear(&sort_param->tempfile_for_exceptions);
   bzero((char*) &sort_param->buffpek, sizeof(sort_param->buffpek));
   bzero((char*) &sort_param->unique,  sizeof(sort_param->unique));
-    sort_keys= (uchar **) NULL;
+
 
   sortbuff_size= sort_param->sortbuff_size;
   memavl=        MY_MAX(sortbuff_size, MIN_SORT_BUFFER);
@@ -409,16 +403,18 @@ pthread_handler_t thr_find_all_keys(void *arg)
              (sort_length+sizeof(char*))) <= 1 ||
             keys < (uint) maxbuffer)
         {
+          mysql_mutex_lock(&sort_param->sort_info->mutex);
           mi_check_print_error(sort_param->sort_info->param,
-                                 "myisam_sort_buffer_size is too small. Current myisam_sort_buffer_size: %llu  rows: %llu  sort_length: %u",
+              "myisam_sort_buffer_size is too small. Current "
+              "myisam_sort_buffer_size: %llu  rows: %llu  sort_length: %u",
               sortbuff_size, (ulonglong) idx, sort_length);
-            goto err;
+          mysql_mutex_unlock(&sort_param->sort_info->mutex);
+          DBUG_RETURN(TRUE);
         }
       }
       while ((maxbuffer= (uint) (idx/(keys-1)+1)) != maxbuffer_org);
     }
-      if ((sort_keys= (uchar**)
-           my_malloc(keys*(sort_length+sizeof(char*))+
+    if ((sort_keys= my_malloc(keys * (sort_length + sizeof(char *)) +
                               ((sort_param->keyinfo->flag & HA_FULLTEXT) ?
                                  HA_FT_MAXBYTELEN : 0), MYF(0))))
     {
@@ -426,7 +422,7 @@ pthread_handler_t thr_find_all_keys(void *arg)
                                 maxbuffer, MY_MIN(maxbuffer/2, 1000), MYF(0)))
       {
         my_free(sort_keys);
-          sort_keys= (uchar **) NULL; /* for err: label */
+        sort_keys= NULL;          /* Safety against double free on error. */
       }
       else
         break;
@@ -439,11 +435,14 @@ pthread_handler_t thr_find_all_keys(void *arg)
   if (memavl < MIN_SORT_BUFFER)
   {
     /* purecov: begin inspected */
+    mysql_mutex_lock(&sort_param->sort_info->mutex);
     mi_check_print_error(sort_param->sort_info->param,
-                           "myisam_sort_buffer_size is too small. Current myisam_sort_buffer_size: %llu  rows: %llu  sort_length: %u",
+        "myisam_sort_buffer_size is too small. Current "
+        "myisam_sort_buffer_size: %llu  rows: %llu  sort_length: %u",
         sortbuff_size, (ulonglong) idx, sort_length);
     my_errno= ENOMEM;
-      goto err;
+    mysql_mutex_unlock(&sort_param->sort_info->mutex);
+    DBUG_RETURN(TRUE);
     /* purecov: end inspected */
   }
 
@@ -475,8 +474,7 @@ pthread_handler_t thr_find_all_keys(void *arg)
                                  &sort_param->tempfile))
         goto err;
       sort_keys[0]= (uchar*) (sort_keys+keys);
-        memcpy(sort_keys[0], sort_keys[idx - 1],
-               (size_t) sort_param->key_length);
+      memcpy(sort_keys[0], sort_keys[idx - 1], (size_t) sort_param->key_length);
       idx= 1;
     }
     sort_keys[idx]= sort_keys[idx - 1] + sort_param->key_length;
@@ -494,7 +492,7 @@ pthread_handler_t thr_find_all_keys(void *arg)
   else
     sort_param->keys= idx;
 
-    goto ok;
+  DBUG_RETURN(FALSE);
 
 err:
   DBUG_PRINT("error", ("got some error"));
@@ -505,7 +503,25 @@ pthread_handler_t thr_find_all_keys(void *arg)
   close_cached_file(&sort_param->tempfile);
   close_cached_file(&sort_param->tempfile_for_exceptions);
 
-ok:
+  DBUG_RETURN(TRUE);
+}
+
+/* Search after all keys and place them in a temp. file */
+
+pthread_handler_t thr_find_all_keys(void *arg)
+{
+  MI_SORT_PARAM *sort_param= (MI_SORT_PARAM*) arg;
+  my_bool error= FALSE;
+  /* If my_thread_init fails */
+  if (my_thread_init())
+    error= TRUE;
+
+  if (error || thr_find_all_keys_exec(sort_param))
+    error= TRUE;
+
+  /*
+     Thread must clean up after itself.
+  */
   free_root(&sort_param->wordroot, MYF(0));
   /*
      Detach from the share if the writer is involved. Avoid others to
@@ -524,12 +540,12 @@ pthread_handler_t thr_find_all_keys(void *arg)
     remove_io_thread(&sort_param->read_cache);
 
   mysql_mutex_lock(&sort_param->sort_info->mutex);
+  if (error)
+    sort_param->sort_info->got_error= 1;
+
   if (!--sort_param->sort_info->threads_running)
     mysql_cond_signal(&sort_param->sort_info->cond);
   mysql_mutex_unlock(&sort_param->sort_info->mutex);
-    DBUG_PRINT("exit", ("======== ending thread ========"));
-    DBUG_LEAVE;
-  }
   my_thread_end();
   return NULL;
 }