Fixed Bug#2123, mysqld segmentation faulted when it tried to

open a file that already existed. The problem was that end_io_cache()
was called even if init_io_cache() was not. This affected both
OUTFILE and DUMPFILE (both fixed). Sometimes wrongly aligned pointer was freed,
sometimes mysqld core dumped.

Other problem was that select_dump::send_error removed the dumpfile,
even if it was created by an earlier run, or by some other program, if
the file permissions just permitted it. Fixed it so that the file will
only be deleted, if an error occurred, but the file was created by mysqld
just a moment ago, in that thread.

On the other hand, select_export did not handle the corresponding garbage
file at all. Both fixed.

After these fixes, a big part of the select_export::prepare and select_dump::prepare
code became identical. Merged the code into a new function called create_file(),
which is now called by the two latter functions.

Regards,
Jani


mysys/mf_iocache.c:
  Fixed a bug in comment.

2 files changed, 34 insertions, 47 deletions

diff --git a/mysys/mf_iocache.c b/mysys/mf_iocache.c
index b5c80d9482f..1dd3108e151 100644
--- a/mysys/mf_iocache.c
+++ b/mysys/mf_iocache.c
@@ -1175,8 +1175,8 @@ int _flush_io_cache(IO_CACHE *info, int need_append_buffer_lock)
     info		IO_CACHE Handle to free
 
   NOTES
-    It's currently safe to call this if one has called io_cache_init()
-    on the 'info' object, even if io_cache_init() failed.
+    It's currently safe to call this if one has called init_io_cache()
+    on the 'info' object, even if init_io_cache() failed.
     This function is also safe to call twice with the same handle.
 
   RETURN
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 89b812eb205..696744953dc 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -695,39 +695,53 @@ select_export::~select_export()
   thd->sent_row_count=row_count;
 }
 
-int
-select_export::prepare(List<Item> &list, SELECT_LEX_UNIT *u)
+
+static int create_file(THD *thd, char *path, sql_exchange *exchange,
+		       File *file, IO_CACHE *cache)
 {
-  char path[FN_REFLEN];
-  uint option=4;
-  bool blob_flag=0;
-  unit= u;
+  uint option= 4;
+
 #ifdef DONT_ALLOW_FULL_LOAD_DATA_PATHS
-  option|=1;					// Force use of db directory
+  option|= 1;					// Force use of db directory
 #endif
-  if ((uint) strlen(exchange->file_name) + NAME_LEN >= FN_REFLEN)
-    strmake(path,exchange->file_name,FN_REFLEN-1);
-  (void) fn_format(path,exchange->file_name, thd->db ? thd->db : "", "",
+  (void) fn_format(path, exchange->file_name, thd->db ? thd->db : "", "",
 		   option);
-  if (!access(path,F_OK))
+  if (!access(path, F_OK))
   {
     my_error(ER_FILE_EXISTS_ERROR, MYF(0), exchange->file_name);
     return 1;
   }
   /* Create the file world readable */
-  if ((file=my_create(path, 0666, O_WRONLY, MYF(MY_WME))) < 0)
+  if ((*file= my_create(path, 0666, O_WRONLY, MYF(MY_WME))) < 0)
     return 1;
 #ifdef HAVE_FCHMOD
-  (void) fchmod(file,0666);			// Because of umask()
+  (void) fchmod(*file, 0666);			// Because of umask()
 #else
-  (void) chmod(path,0666);
+  (void) chmod(path, 0666);
 #endif
-  if (init_io_cache(&cache,file,0L,WRITE_CACHE,0L,1,MYF(MY_WME)))
+  if (init_io_cache(cache, *file, 0L, WRITE_CACHE, 0L, 1, MYF(MY_WME)))
   {
-    my_close(file,MYF(0));
-    file= -1;
+    my_close(*file, MYF(0));
+    my_delete(path, MYF(0));  // Delete file on error, it was just created 
+    *file= -1;
+    end_io_cache(cache);
     return 1;
   }
+  return 0;
+}
+
+
+int
+select_export::prepare(List<Item> &list, SELECT_LEX_UNIT *u)
+{
+  char path[FN_REFLEN];
+  bool blob_flag=0;
+  unit= u;
+  if ((uint) strlen(exchange->file_name) + NAME_LEN >= FN_REFLEN)
+    strmake(path,exchange->file_name,FN_REFLEN-1);
+
+  if (create_file(thd, path, exchange, &file, &cache))
+    return 1;
   /* Check if there is any blobs in data */
   {
     List_iterator_fast<Item> li(list);
@@ -901,7 +915,6 @@ err:
 void select_export::send_error(uint errcode, const char *err)
 {
   ::send_error(thd,errcode,err);
-  (void) end_io_cache(&cache);
   (void) my_close(file,MYF(0));
   file= -1;
 }
@@ -938,33 +951,9 @@ int
 select_dump::prepare(List<Item> &list __attribute__((unused)),
 		     SELECT_LEX_UNIT *u)
 {
-  uint option=4;
   unit= u;
-#ifdef DONT_ALLOW_FULL_LOAD_DATA_PATHS
-  option|=1;					// Force use of db directory
-#endif
-  (void) fn_format(path,exchange->file_name, thd->db ? thd->db : "", "",
-		   option);
-  if (!access(path,F_OK))
-  {
-    my_error(ER_FILE_EXISTS_ERROR,MYF(0),exchange->file_name);
+  if (create_file(thd, path, exchange, &file, &cache))
     return 1;
-  }
-  /* Create the file world readable */
-  if ((file=my_create(path, 0666, O_WRONLY, MYF(MY_WME))) < 0)
-    return 1;
-#ifdef HAVE_FCHMOD
-  (void) fchmod(file,0666);			// Because of umask()
-#else
-  (void) chmod(path,0666);
-#endif
-  if (init_io_cache(&cache,file,0L,WRITE_CACHE,0L,1,MYF(MY_WME)))
-  {
-    my_close(file,MYF(0));
-    my_delete(path,MYF(0));
-    file= -1;
-    return 1;
-  }
   return 0;
 }
 
@@ -1011,9 +1000,7 @@ err:
 void select_dump::send_error(uint errcode,const char *err)
 {
   ::send_error(thd,errcode,err);
-  (void) end_io_cache(&cache);
   (void) my_close(file,MYF(0));
-  (void) my_delete(path,MYF(0));		// Delete file on error
   file= -1;
 }