gfileutils: make g_file_set_contents() always fsync()1302-file-set-contents-fsync

Previously, this function only called fsync() if @filename exists and is
non-empty. This behaviour was introduced when the function was first
written (6cff88ba18b3bc0d118308f109840cb163dcea03) and shortly
afterwards (d20a188b1250ab3cf211d684429127d99378e886) respectively, with
the latter justified as a performance optimisation.

This meant that g_file_set_contents() does not provide the guarantee
that developers assume it has, namely that after a call and a crash,
@filename will either contain its previous contents or its new
@contents. In practice, when it was previously non-existent or empty on
a bog-standard ext4 filesystem, it would often contain NUL bytes
matching the @length of @contents, requiring application developers to
explicitly handle this third case.

Given the documentation includes the word "atomic", we make this
function provide the guarantee that was previously implied but untrue,
and document it. If applications require higher performance at the cost
of correctness, they can open-code the old behaviour, or we can add a
new function to glib providing weaker guarantees.

https://gitlab.gnome.org/GNOME/glib/issues/1302

1 files changed, 7 insertions, 14 deletions

diff --git a/glib/gfileutils.c b/glib/gfileutils.c
index 1e7a771a9..cf0a8bc3f 100644
--- a/glib/gfileutils.c
+++ b/glib/gfileutils.c
@@ -1111,16 +1111,14 @@ write_to_temp_file (const gchar  *contents,
 
 #ifdef HAVE_FSYNC
   {
-    struct stat statbuf;
-
     errno = 0;
-    /* If the final destination exists and is > 0 bytes, we want to sync the
+    /* We want to sync the
      * newly written file to ensure the data is on disk when we rename over
      * the destination. Otherwise if we get a system crash we can lose both
      * the new and the old file on some filesystems. (I.E. those that don't
      * guarantee the data is written to the disk before the metadata.)
      */
-    if (g_lstat (dest_file, &statbuf) == 0 && statbuf.st_size > 0 && fsync (fd) != 0)
+    if (fsync (fd) != 0)
       {
         int saved_errno = errno;
         set_file_error (err,
@@ -1173,16 +1171,11 @@ write_to_temp_file (const gchar  *contents,
  *   lists, metadata etc. may be lost. If @filename is a symbolic link,
  *   the link itself will be replaced, not the linked file.
  *
- * - On UNIX, if @filename already exists and is non-empty, and if the system
- *   supports it (via a journalling filesystem or equivalent), the fsync()
- *   call (or equivalent) will be used to ensure atomic replacement: @filename
- *   will contain either its old contents or @contents, even in the face of
- *   system power loss, the disk being unsafely removed, etc.
- *
- * - On UNIX, if @filename does not already exist or is empty, there is a
- *   possibility that system power loss etc. after calling this function will
- *   leave @filename empty or full of NUL bytes, depending on the underlying
- *   filesystem.
+ * - On UNIX, if the filesystem is uncleanly unmounted after a successful call
+ *   to this function, it is guaranteed that @filename will contain either its
+ *   old contents, or @contents. In particular, if @filename did not previously
+ *   exist, following a crash it will either not exist or contain its new
+ *   @contents.
  *
  * - On Windows renaming a file will not remove an existing file with the
  *   new name, so on Windows there is a race condition between the existing