Maria - post-review fixes about my_sync_dir():

make it return an error (except if certain errno), test this error in
callers. Do a single my_sync_dir() in my_rename() if possible.


include/my_global.h:
  better have a symbol name talking about the feature, use it in the
  code of the feature, and define the symbol once depending on the
  platform, rather than have the platform "tested" in the code
  of the feature several times.
include/my_sys.h:
  my_sync_dir() now can return error
mysys/my_create.c:
  my_sync_dir() can now return an error
mysys/my_delete.c:
  my_sync_dir() can now return an error
mysys/my_rename.c:
  my_sync_dir() can now return an error.
  Do a single sync if "from" and "to" are the same directory.
  #ifdef here to not even compile dirname_part() if useless.
mysys/my_sync.c:
  more comments.
  A compilation error if no way to make my_sync() work (I guess
  we don't want to ship a binary which cannot do any sync at all;
  users of strange OSes compile from source and can remove
  the #error).
  my_sync_dir() now returns an error (except for certain errno values
  considered ok; EIO "input/output error" is not ok).
sql/unireg.cc:
  my_sync_dir() now returns an error which must be tested

include/my_global.h

@@ -1468,4 +1468,12 @@ do { doubleget_union _tmp; \
 #define dlerror() ""
 #endif
 
+/*
+  Only Linux is known to need an explicit sync of the directory to make sure a
+  file creation/deletion/renaming in(from,to) this directory durable.
+*/
+#ifdef TARGET_OS_LINUX
+#define NEED_EXPLICIT_SYNC_DIR 1
+#endif
+
 #endif /* my_global_h */

include/my_sys.h

@@ -623,8 +623,8 @@ extern FILE *my_fdopen(File Filedes,const char *name, int Flags,myf MyFlags);
 extern int my_fclose(FILE *fd,myf MyFlags);
 extern int my_chsize(File fd,my_off_t newlength, int filler, myf MyFlags);
 extern int my_sync(File fd, myf my_flags);
-extern void my_sync_dir(const char *dir_name, myf my_flags);
-extern void my_sync_dir_by_file(const char *file_name, myf my_flags);
+extern int my_sync_dir(const char *dir_name, myf my_flags);
+extern int my_sync_dir_by_file(const char *file_name, myf my_flags);
 extern int my_error _VARARGS((int nr,myf MyFlags, ...));
 extern int my_printf_error _VARARGS((uint my_err, const char *format,
 				     myf MyFlags, ...))

mysys/my_create.c

@@ -53,8 +53,12 @@ File my_create(const char *FileName, int CreateFlags, int access_flags,
   fd = open(FileName, access_flags);
 #endif
 
-  if ((MyFlags & MY_SYNC_DIR) && (fd >=0))
-    my_sync_dir_by_file(FileName, MyFlags);
+  if ((MyFlags & MY_SYNC_DIR) && (fd >=0) &&
+      my_sync_dir_by_file(FileName, MyFlags))
+  {
+    my_close(fd, MyFlags);
+    fd= -1;
+  }
 
   DBUG_RETURN(my_register_filename(fd, FileName, FILE_BY_CREATE,
 				   EE_CANTCREATEFILE, MyFlags));

mysys/my_delete.c

@@ -30,8 +30,9 @@ int my_delete(const char *name, myf MyFlags)
       my_error(EE_DELETE,MYF(ME_BELL+ME_WAITTANG+(MyFlags & ME_NOINPUT)),
 	       name,errno);
   }
-  else if (MyFlags & MY_SYNC_DIR)
-    my_sync_dir_by_file(name, MyFlags);
+  else if ((MyFlags & MY_SYNC_DIR) &&
+           my_sync_dir_by_file(name, MyFlags))
+    err= -1;
   DBUG_RETURN(err);
 } /* my_delete */
 

mysys/my_rename.c

@@ -63,8 +63,16 @@ int my_rename(const char *from, const char *to, myf MyFlags)
   }
   else if (MyFlags & MY_SYNC_DIR)
   {
-    my_sync_dir_by_file(from, MyFlags);
-    my_sync_dir_by_file(to, MyFlags);
+#ifdef NEED_EXPLICIT_SYNC_DIR
+    /* do only the needed amount of syncs: */
+    char dir_from[FN_REFLEN], dir_to[FN_REFLEN];
+    dirname_part(dir_from, from);
+    dirname_part(dir_to, to);
+    if (my_sync_dir(dir_from, MyFlags) ||
+        (strcmp(dir_from, dir_to) &&
+         my_sync_dir(dir_to, MyFlags)))
+      error= -1;
+#endif
   }
   DBUG_RETURN(error);
 } /* my_rename */

mysys/my_sync.c

@@ -50,10 +50,14 @@ int my_sync(File fd, myf my_flags)
   do
   {
 #if defined(F_FULLFSYNC)
-    /* Recent Mac OS X versions insist this call is safer than fsync() */
+    /*
+      In Mac OS X >= 10.3 this call is safer than fsync() (it forces the
+      disk's cache).
+    */
     if (!(res= fcntl(fd, F_FULLFSYNC, 0)))
       break; /* ok */
-    /* Some fs don't support F_FULLFSYNC and fail above, fallback: */
+    /* Some file systems don't support F_FULLFSYNC and fail above: */
+    DBUG_PRINT("info",("fcntl(F_FULLFSYNC) failed, falling back"));
 #endif
 #if defined(HAVE_FDATASYNC)
     res= fdatasync(fd);
@@ -62,7 +66,7 @@ int my_sync(File fd, myf my_flags)
 #elif defined(__WIN__)
     res= _commit(fd);
 #else
-#warning Cannot find a way to sync a file, durability in danger
+#error Cannot find a way to sync a file, durability in danger
     res= 0;					/* No sync (strange OS) */
 #endif
   } while (res == -1 && errno == EINTR);
@@ -74,7 +78,10 @@ int my_sync(File fd, myf my_flags)
       my_errno= -1;                             /* Unknown error */
     if ((my_flags & MY_IGNORE_BADFD) &&
         (er == EBADF || er == EINVAL || er == EROFS))
+    {
+      DBUG_PRINT("info", ("ignoring errno %d", er));
       res= 0;
+    }
     else if (my_flags & MY_WME)
       my_error(EE_SYNC, MYF(ME_BELL+ME_WAITTANG), my_filename(fd), my_errno);
   }
@@ -83,68 +90,62 @@ int my_sync(File fd, myf my_flags)
 
 
 /*
-  Force directory information to disk. Only Linux is known to need this to
-  make sure a file creation/deletion/renaming in(from,to) this directory
-  durable.
+  Force directory information to disk.
 
   SYNOPSIS
     my_sync_dir()
     dir_name             the name of the directory
-    my_flags             unused
+    my_flags             flags (MY_WME etc)
 
   RETURN
-    nothing (the sync may fail sometimes).
+    0 if ok, !=0 if error
 */
-void my_sync_dir(const char *dir_name, myf my_flags __attribute__((unused)))
+int my_sync_dir(const char *dir_name, myf my_flags)
 {
-#ifdef TARGET_OS_LINUX
+#ifdef NEED_EXPLICIT_SYNC_DIR
   DBUG_ENTER("my_sync_dir");
   DBUG_PRINT("my",("Dir: '%s'  my_flags: %d", dir_name, my_flags));
   File dir_fd;
-  int error= 0;
+  int res= 0;
   /*
-    Syncing a dir does not work on all filesystems (e.g. tmpfs->EINVAL) :
-    ignore errors. But print them to the debug log.
+    Syncing a dir may give EINVAL on tmpfs on Linux, which is ok.
+    EIO on the other hand is very important. Hence MY_IGNORE_BADFD.
   */
-  if (((dir_fd= my_open(dir_name, O_RDONLY, MYF(0))) >= 0))
-  {
-    if (my_sync(dir_fd, MYF(0)))
+  if ((dir_fd= my_open(dir_name, O_RDONLY, MYF(my_flags))) >= 0)
   {
-      error= errno;
-      DBUG_PRINT("info",("my_sync failed errno: %d", error));
-    }
-    my_close(dir_fd, MYF(0));
+    if (my_sync(dir_fd, MYF(my_flags | MY_IGNORE_BADFD)))
+      res= 2;
+    if (my_close(dir_fd, MYF(my_flags)))
+      res= 3;
   }
   else
-  {
-    error= errno;
-    DBUG_PRINT("info",("my_open failed errno: %d", error));
-  }
-  DBUG_VOID_RETURN;
+    res= 1;
+  DBUG_RETURN(res);
+#else
+  return 0;
 #endif
 }
 
 
 /*
-  Force directory information to disk. Only Linux is known to need this to
-  make sure a file creation/deletion/renaming in(from,to) this directory
-  durable.
+  Force directory information to disk.
 
   SYNOPSIS
     my_sync_dir_by_file()
     file_name            the name of a file in the directory
-    my_flags             unused
+    my_flags             flags (MY_WME etc)
 
   RETURN
-    nothing (the sync may fail sometimes).
+    0 if ok, !=0 if error
 */
-void my_sync_dir_by_file(const char *file_name,
-                         myf my_flags __attribute__((unused)))
+int my_sync_dir_by_file(const char *file_name, myf my_flags)
 {
-#ifdef TARGET_OS_LINUX
+#ifdef NEED_EXPLICIT_SYNC_DIR
   char dir_name[FN_REFLEN];
   dirname_part(dir_name, file_name);
   return my_sync_dir(dir_name, my_flags);
+#else
+  return 0;
 #endif
 }
 

sql/unireg.cc

@@ -285,12 +285,11 @@ bool mysql_create_frm(THD *thd, const char *file_name,
   my_free((gptr) screen_buff,MYF(0));
   my_free((gptr) keybuff, MYF(0));
 
-  if (opt_sync_frm && !(create_info->options & HA_LEX_CREATE_TMP_TABLE))
-  {
-    if (my_sync(file, MYF(MY_WME)))
+  if (opt_sync_frm && !(create_info->options & HA_LEX_CREATE_TMP_TABLE) &&
+      (my_sync(file, MYF(MY_WME)) ||
+       my_sync_dir_by_file(file_name, MYF(MY_WME))))
       goto err2;
-    my_sync_dir_by_file(file_name, MYF(0));
-  }
+
   if (my_close(file,MYF(MY_WME)))
     goto err3;