diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 8e3aefd16..3cefbe010 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -1613,7 +1613,9 @@ G_FILE_ERROR GFileTest g_file_error_from_errno g_file_get_contents +GFileSetContentsFlags g_file_set_contents +g_file_set_contents_full g_file_test g_mkstemp g_mkstemp_full diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c index 5e3e93d13..416d8cc32 100644 --- a/gio/gdbusauthmechanismsha1.c +++ b/gio/gdbusauthmechanismsha1.c @@ -830,10 +830,12 @@ keyring_generate_entry (const gchar *cookie_context, /* and now actually write the cookie file if there are changes (this is atomic) */ if (changed_file) { - if (!g_file_set_contents (path, - new_contents->str, - -1, - error)) + if (!g_file_set_contents_full (path, + new_contents->str, + -1, + G_FILE_SET_CONTENTS_CONSISTENT, + 0600, + error)) { *out_id = 0; *out_cookie = 0; diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index e6bca46fe..ed98570fe 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -3630,7 +3630,9 @@ update_mimeapps_list (const char *desktop_id, data = g_key_file_to_data (key_file, &data_size, error); g_key_file_free (key_file); - res = g_file_set_contents (filename, data, data_size, error); + res = g_file_set_contents_full (filename, data, data_size, + G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING, + 0600, error); desktop_file_dirs_invalidate_user_config (); @@ -3774,7 +3776,9 @@ g_desktop_app_info_set_as_default_for_extension (GAppInfo *appinfo, " \n" "\n", mimetype, extension, extension); - g_file_set_contents (filename, contents, -1, NULL); + g_file_set_contents_full (filename, contents, -1, + G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING, + 0600, NULL); g_free (contents); run_update_command ("update-mime-database", "mime"); @@ -3923,7 +3927,9 @@ g_desktop_app_info_ensure_saved (GDesktopAppInfo *info, /* FIXME - actually handle error */ (void) g_close (fd, NULL); - res = g_file_set_contents (filename, data, data_size, error); + res = g_file_set_contents_full (filename, data, data_size, + G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING, + 0600, error); g_free (data); if (!res) { diff --git a/gio/glocalfile.c b/gio/glocalfile.c index a92c7b28b..f7ae5d496 100644 --- a/gio/glocalfile.c +++ b/gio/glocalfile.c @@ -2211,7 +2211,9 @@ g_local_file_trash (GFile *file, original_name_escaped, delete_time); g_free (delete_time); - g_file_set_contents (infofile, data, -1, NULL); + g_file_set_contents_full (infofile, data, -1, + G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING, + 0600, NULL); /* TODO: Maybe we should verify that you can delete the file from the trash * before moving it? OTOH, that is hard, as it needs a recursive scan diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c index a8161bd52..136d98241 100644 --- a/gio/glocalfileoutputstream.c +++ b/gio/glocalfileoutputstream.c @@ -313,9 +313,8 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file, { GLocalFileStat final_stat; -#ifdef HAVE_FSYNC if (file->priv->sync_on_close && - fsync (file->priv->fd) != 0) + g_fsync (file->priv->fd) != 0) { int errsv = errno; @@ -325,8 +324,7 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file, g_strerror (errsv)); goto err_out; } -#endif - + #ifdef G_OS_WIN32 /* Must close before renaming on Windows, so just do the close first diff --git a/gio/gtestdbus.c b/gio/gtestdbus.c index 633c93534..96da52c68 100644 --- a/gio/gtestdbus.c +++ b/gio/gtestdbus.c @@ -573,7 +573,9 @@ write_config_file (GTestDBus *self) "\n"); close (fd); - g_file_set_contents (path, contents->str, contents->len, &error); + g_file_set_contents_full (path, contents->str, contents->len, + G_FILE_SET_CONTENTS_NONE, + 0600, &error); g_assert_no_error (error); g_string_free (contents, TRUE); diff --git a/glib/gfileutils.c b/glib/gfileutils.c index f0799e212..8d80cd50a 100644 --- a/glib/gfileutils.c +++ b/glib/gfileutils.c @@ -46,6 +46,10 @@ #define O_BINARY 0 #endif +#ifndef O_CLOEXEC +#define O_CLOEXEC 0 +#endif + #include "gfileutils.h" #include "gstdio.h" @@ -1023,8 +1027,9 @@ g_file_get_contents (const gchar *filename, static gboolean rename_file (const char *old_name, - const char *new_name, - GError **err) + const char *new_name, + gboolean do_fsync, + GError **err) { errno = 0; if (g_rename (old_name, new_name) == -1) @@ -1046,36 +1051,99 @@ rename_file (const char *old_name, return FALSE; } - + + /* In order to guarantee that the *new* contents of the file are seen in + * future, fsync() the directory containing the file. Otherwise if the file + * system was unmounted cleanly now, it would be undefined whether the old + * or new contents of the file were visible after recovery. + * + * This assumes the @old_name and @new_name are in the same directory. */ +#ifdef HAVE_FSYNC + if (do_fsync) + { + gchar *dir = g_path_get_dirname (new_name); + int dir_fd = g_open (dir, O_RDONLY, 0); + + if (dir_fd >= 0) + { + g_fsync (dir_fd); + g_close (dir_fd, NULL); + } + + g_free (dir); + } +#endif /* HAVE_FSYNC */ + return TRUE; } -static gchar * -write_to_temp_file (const gchar *contents, - gssize length, - const gchar *dest_file, - GError **err) +static gboolean +fd_should_be_fsynced (int fd, + const gchar *test_file, + GFileSetContentsFlags flags) { - gchar *tmp_name; - gchar *retval; - gint fd; +#ifdef HAVE_FSYNC + struct stat statbuf; - retval = NULL; +#ifdef BTRFS_SUPER_MAGIC + { + struct statfs buf; - tmp_name = g_strdup_printf ("%s.XXXXXX", dest_file); + /* On Linux, on btrfs, skip the fsync since rename-over-existing is + * guaranteed to be atomic and this is the only case in which we + * would fsync() anyway. + * + * See https://btrfs.wiki.kernel.org/index.php/FAQ#What_are_the_crash_guarantees_of_overwrite-by-rename.3F + */ - errno = 0; - fd = g_mkstemp_full (tmp_name, O_RDWR | O_BINARY, 0666); + if ((flags & G_FILE_SET_CONTENTS_CONSISTENT) && + fstatfs (fd, &buf) == 0 && buf.f_type == BTRFS_SUPER_MAGIC) + return FALSE; + } +#endif /* BTRFS_SUPER_MAGIC */ - if (fd == -1) + /* If the final destination exists and is > 0 bytes, 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.) + * + * There is no difference (in file system terms) if the old file doesn’t + * already exist, apart from the fact that if the system crashes and the new + * data hasn’t been fsync()ed, there is only one bit of old data to lose (that + * the file didn’t exist in the first place). In some situations, such as + * trashing files, the old file never exists, so it seems reasonable to avoid + * the fsync(). This is not a widely applicable optimisation though. + */ + if ((flags & (G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_DURABLE)) && + (flags & G_FILE_SET_CONTENTS_ONLY_EXISTING)) { - int saved_errno = errno; - set_file_error (err, - tmp_name, _("Failed to create file “%s”: %s"), - saved_errno); - goto out; + errno = 0; + if (g_lstat (test_file, &statbuf) == 0) + return (statbuf.st_size > 0); + else if (errno == ENOENT) + return FALSE; + else + return TRUE; /* lstat() failed; be cautious */ } + else + { + return (flags & (G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_DURABLE)); + } +#else /* if !HAVE_FSYNC */ + return FALSE; +#endif /* !HAVE_FSYNC */ +} +/* closes @fd once it’s finished (on success or error) */ +static gboolean +write_to_file (const gchar *contents, + gsize length, + int fd, + const gchar *dest_file, + gboolean do_fsync, + GError **err) +{ #ifdef HAVE_FALLOCATE if (length > 0) { @@ -1098,12 +1166,11 @@ write_to_temp_file (const gchar *contents, continue; set_file_error (err, - tmp_name, _("Failed to write file “%s”: write() failed: %s"), + dest_file, _("Failed to write file “%s”: write() failed: %s"), saved_errno); close (fd); - g_unlink (tmp_name); - goto out; + return FALSE; } g_assert (s <= length); @@ -1112,63 +1179,34 @@ write_to_temp_file (const gchar *contents, length -= s; } -#ifdef BTRFS_SUPER_MAGIC - { - struct statfs buf; - - /* On Linux, on btrfs, skip the fsync since rename-over-existing is - * guaranteed to be atomic and this is the only case in which we - * would fsync() anyway. - */ - - if (fstatfs (fd, &buf) == 0 && buf.f_type == BTRFS_SUPER_MAGIC) - goto no_fsync; - } -#endif #ifdef HAVE_FSYNC - { - struct stat statbuf; + errno = 0; + if (do_fsync && g_fsync (fd) != 0) + { + int saved_errno = errno; + set_file_error (err, + dest_file, _("Failed to write file “%s”: fsync() failed: %s"), + saved_errno); + close (fd); - errno = 0; - /* If the final destination exists and is > 0 bytes, 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) - { - int saved_errno = errno; - set_file_error (err, - tmp_name, _("Failed to write file “%s”: fsync() failed: %s"), - saved_errno); - close (fd); - g_unlink (tmp_name); - - goto out; - } - } -#endif - -#ifdef BTRFS_SUPER_MAGIC - no_fsync: + return FALSE; + } #endif errno = 0; if (!g_close (fd, err)) - { - g_unlink (tmp_name); + return FALSE; - goto out; - } + return TRUE; +} - retval = g_strdup (tmp_name); - - out: - g_free (tmp_name); - - return retval; +static inline int +steal_fd (int *fd_ptr) +{ + int fd = *fd_ptr; + *fd_ptr = -1; + return fd; } /** @@ -1179,11 +1217,54 @@ write_to_temp_file (const gchar *contents, * @length: length of @contents, or -1 if @contents is a nul-terminated string * @error: return location for a #GError, or %NULL * + * Writes all of @contents to a file named @filename. This is a convenience + * wrapper around calling g_file_set_contents() with `flags` set to + * `G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING` and + * `mode` set to `0666`. + * + * Returns: %TRUE on success, %FALSE if an error occurred + * + * Since: 2.8 + */ +gboolean +g_file_set_contents (const gchar *filename, + const gchar *contents, + gssize length, + GError **error) +{ + return g_file_set_contents_full (filename, contents, length, + G_FILE_SET_CONTENTS_CONSISTENT | + G_FILE_SET_CONTENTS_ONLY_EXISTING, + 0666, error); +} + +/** + * g_file_set_contents_full: + * @filename: (type filename): name of a file to write @contents to, in the GLib file name + * encoding + * @contents: (array length=length) (element-type guint8): string to write to the file + * @length: length of @contents, or -1 if @contents is a nul-terminated string + * @flags: flags controlling the safety vs speed of the operation + * @mode: file mode, as passed to `open()`; typically this will be `0666` + * @error: return location for a #GError, or %NULL + * * Writes all of @contents to a file named @filename, with good error checking. * If a file called @filename already exists it will be overwritten. * - * This write is atomic in the sense that it is first written to a temporary - * file which is then renamed to the final name. Notes: + * @flags control the properties of the write operation: whether it’s atomic, + * and what the tradeoff is between returning quickly or being resilient to + * system crashes. + * + * As this function performs file I/O, it is recommended to not call it anywhere + * where blocking would cause problems, such as in the main loop of a graphical + * application. In particular, if @flags has any value other than + * %G_FILE_SET_CONTENTS_NONE then this function may call `fsync()`. + * + * If %G_FILE_SET_CONTENTS_CONSISTENT is set in @flags, the operation is atomic + * in the sense that it is first written to a temporary file which is then + * renamed to the final name. + * + * Notes: * * - On UNIX, if @filename already exists hard links to @filename will break. * Also since the file is recreated, existing permissions, access control @@ -1191,15 +1272,17 @@ write_to_temp_file (const gchar *contents, * 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 + * supports it (via a journalling filesystem or equivalent), and if + * %G_FILE_SET_CONTENTS_CONSISTENT is set in @flags, 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. + * filesystem, unless %G_FILE_SET_CONTENTS_DURABLE and + * %G_FILE_SET_CONTENTS_CONSISTENT are set in @flags. * * - 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 @@ -1216,88 +1299,171 @@ write_to_temp_file (const gchar *contents, * Note that the name for the temporary file is constructed by appending up * to 7 characters to @filename. * + * If the file didn’t exist before and is created, it will be given the + * permissions from @mode. Otherwise, the permissions of the existing file may + * be changed to @mode depending on @flags, or they may remain unchanged. + * * Returns: %TRUE on success, %FALSE if an error occurred * - * Since: 2.8 + * Since: 2.66 */ gboolean -g_file_set_contents (const gchar *filename, - const gchar *contents, - gssize length, - GError **error) +g_file_set_contents_full (const gchar *filename, + const gchar *contents, + gssize length, + GFileSetContentsFlags flags, + int mode, + GError **error) { - gchar *tmp_filename; - gboolean retval; - GError *rename_error = NULL; - g_return_val_if_fail (filename != NULL, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); g_return_val_if_fail (contents != NULL || length == 0, FALSE); g_return_val_if_fail (length >= -1, FALSE); - - if (length == -1) + + /* @flags are handled as follows: + * - %G_FILE_SET_CONTENTS_NONE: write directly to @filename, no fsync()s + * - %G_FILE_SET_CONTENTS_CONSISTENT: write to temp file, fsync() it, rename() + * - %G_FILE_SET_CONTENTS_CONSISTENT | ONLY_EXISTING: as above, but skip the + * fsync() if @filename doesn’t exist or is empty + * - %G_FILE_SET_CONTENTS_DURABLE: write directly to @filename, fsync() it + * - %G_FILE_SET_CONTENTS_DURABLE | ONLY_EXISTING: as above, but skip the + * fsync() if @filename doesn’t exist or is empty + * - %G_FILE_SET_CONTENTS_CONSISTENT | DURABLE: write to temp file, fsync() + * it, rename(), fsync() containing directory + * - %G_FILE_SET_CONTENTS_CONSISTENT | DURABLE | ONLY_EXISTING: as above, but + * skip both fsync()s if @filename doesn’t exist or is empty + */ + + if (length < 0) length = strlen (contents); - tmp_filename = write_to_temp_file (contents, length, filename, error); - - if (!tmp_filename) + if (flags & G_FILE_SET_CONTENTS_CONSISTENT) { - retval = FALSE; - goto out; - } + gchar *tmp_filename = NULL; + GError *rename_error = NULL; + gboolean retval; + int fd; + gboolean do_fsync; - if (!rename_file (tmp_filename, filename, &rename_error)) - { -#ifndef G_OS_WIN32 + tmp_filename = g_strdup_printf ("%s.XXXXXX", filename); - g_unlink (tmp_filename); - g_propagate_error (error, rename_error); - retval = FALSE; - goto out; + errno = 0; + fd = g_mkstemp_full (tmp_filename, O_RDWR | O_BINARY, mode); -#else /* G_OS_WIN32 */ - - /* Renaming failed, but on Windows this may just mean - * the file already exists. So if the target file - * exists, try deleting it and do the rename again. - */ - if (!g_file_test (filename, G_FILE_TEST_EXISTS)) - { - g_unlink (tmp_filename); - g_propagate_error (error, rename_error); - retval = FALSE; - goto out; - } - - g_error_free (rename_error); - - if (g_unlink (filename) == -1) - { + if (fd == -1) + { int saved_errno = errno; set_file_error (error, - filename, - _("Existing file “%s” could not be removed: g_unlink() failed: %s"), + tmp_filename, _("Failed to create file “%s”: %s"), saved_errno); - g_unlink (tmp_filename); - retval = FALSE; - goto out; - } - - if (!rename_file (tmp_filename, filename, error)) - { - g_unlink (tmp_filename); - retval = FALSE; - goto out; - } + retval = FALSE; + goto consistent_out; + } + do_fsync = fd_should_be_fsynced (fd, filename, flags); + if (!write_to_file (contents, length, steal_fd (&fd), tmp_filename, do_fsync, error)) + { + g_unlink (tmp_filename); + retval = FALSE; + goto consistent_out; + } + + if (!rename_file (tmp_filename, filename, do_fsync, &rename_error)) + { +#ifndef G_OS_WIN32 + + g_unlink (tmp_filename); + g_propagate_error (error, rename_error); + retval = FALSE; + goto consistent_out; + +#else /* G_OS_WIN32 */ + + /* Renaming failed, but on Windows this may just mean + * the file already exists. So if the target file + * exists, try deleting it and do the rename again. + */ + if (!g_file_test (filename, G_FILE_TEST_EXISTS)) + { + g_unlink (tmp_filename); + g_propagate_error (error, rename_error); + retval = FALSE; + goto consistent_out; + } + + g_error_free (rename_error); + + if (g_unlink (filename) == -1) + { + int saved_errno = errno; + set_file_error (error, + filename, + _("Existing file “%s” could not be removed: g_unlink() failed: %s"), + saved_errno); + g_unlink (tmp_filename); + retval = FALSE; + goto consistent_out; + } + + if (!rename_file (tmp_filename, filename, flags, error)) + { + g_unlink (tmp_filename); + retval = FALSE; + goto consistent_out; + } + +#endif /* G_OS_WIN32 */ + } + + retval = TRUE; + +consistent_out: + g_free (tmp_filename); + return retval; + } + else + { + int direct_fd; + int open_flags; + gboolean do_fsync; + + open_flags = O_RDWR | O_BINARY | O_CREAT | O_CLOEXEC; +#ifdef O_NOFOLLOW + /* Windows doesn’t have symlinks, so O_NOFOLLOW is unnecessary there. */ + open_flags |= O_NOFOLLOW; #endif + + errno = 0; + direct_fd = g_open (filename, open_flags, mode); + + if (direct_fd < 0) + { + int saved_errno = errno; + +#ifdef O_NOFOLLOW + /* ELOOP indicates that @filename is a symlink, since we used + * O_NOFOLLOW (alternately it could indicate that @filename contains + * looping or too many symlinks). In either case, try again on the + * %G_FILE_SET_CONTENTS_CONSISTENT code path. */ + if (saved_errno == ELOOP) + return g_file_set_contents_full (filename, contents, length, + flags | G_FILE_SET_CONTENTS_CONSISTENT, + mode, error); +#endif + + set_file_error (error, + filename, _("Failed to open file “%s”: %s"), + saved_errno); + return FALSE; + } + + do_fsync = fd_should_be_fsynced (direct_fd, filename, flags); + if (!write_to_file (contents, length, steal_fd (&direct_fd), filename, + do_fsync, error)) + return FALSE; } - retval = TRUE; - - out: - g_free (tmp_filename); - return retval; + return TRUE; } /* diff --git a/glib/gfileutils.h b/glib/gfileutils.h index f60fad858..d6b1d9eec 100644 --- a/glib/gfileutils.h +++ b/glib/gfileutils.h @@ -72,6 +72,39 @@ typedef enum G_FILE_TEST_EXISTS = 1 << 4 } GFileTest; +/** + * GFileSetContentsFlags: + * @G_FILE_SET_CONTENTS_NONE: No guarantees about file consistency or durability. + * The most dangerous setting, which is slightly faster than other settings. + * @G_FILE_SET_CONTENTS_CONSISTENT: Guarantee file consistency: after a crash, + * either the old version of the file or the new version of the file will be + * available, but not a mixture. On Unix systems this equates to an `fsync()` + * on the file and use of an atomic `rename()` of the new version of the file + * over the old. + * @G_FILE_SET_CONTENTS_DURABLE: Guarantee file durability: after a crash, the + * new version of the file will be available. On Unix systems this equates to + * an `fsync()` on the file (if %G_FILE_SET_CONTENTS_CONSISTENT is unset), or + * the effects of %G_FILE_SET_CONTENTS_CONSISTENT plus an `fsync()` on the + * directory containing the file after calling `rename()`. + * @G_FILE_SET_CONTENTS_ONLY_EXISTING: Only apply consistency and durability + * guarantees if the file already exists. This may speed up file operations + * if the file doesn’t currently exist, but may result in a corrupted version + * of the new file if the system crashes while writing it. + * + * Flags to pass to g_file_set_contents_full() to affect its safety and + * performance. + * + * Since: 2.66 + */ +typedef enum +{ + G_FILE_SET_CONTENTS_NONE = 0, + G_FILE_SET_CONTENTS_CONSISTENT = 1 << 0, + G_FILE_SET_CONTENTS_DURABLE = 1 << 1, + G_FILE_SET_CONTENTS_ONLY_EXISTING = 1 << 2 +} GFileSetContentsFlags +GLIB_AVAILABLE_ENUMERATOR_IN_2_66; + GLIB_AVAILABLE_IN_ALL GQuark g_file_error_quark (void); /* So other code can generate a GFileError */ @@ -91,6 +124,15 @@ gboolean g_file_set_contents (const gchar *filename, const gchar *contents, gssize length, GError **error); +G_GNUC_BEGIN_IGNORE_DEPRECATIONS +GLIB_AVAILABLE_IN_2_66 +gboolean g_file_set_contents_full (const gchar *filename, + const gchar *contents, + gssize length, + GFileSetContentsFlags flags, + int mode, + GError **error); +G_GNUC_END_IGNORE_DEPRECATIONS GLIB_AVAILABLE_IN_ALL gchar *g_file_read_link (const gchar *filename, GError **error); diff --git a/glib/gkeyfile.c b/glib/gkeyfile.c index 8572711f8..7c81bf070 100644 --- a/glib/gkeyfile.c +++ b/glib/gkeyfile.c @@ -4611,7 +4611,9 @@ g_key_file_parse_comment_as_value (GKeyFile *key_file, * @error: a pointer to a %NULL #GError, or %NULL * * Writes the contents of @key_file to @filename using - * g_file_set_contents(). + * g_file_set_contents(). If you need stricter guarantees about durability of + * the written file than are provided by g_file_set_contents(), use + * g_file_set_contents_full() with the return value of g_key_file_to_data(). * * This function can fail for any of the reasons that * g_file_set_contents() may fail. diff --git a/glib/gstdio.c b/glib/gstdio.c index 88d030621..b570f9176 100644 --- a/glib/gstdio.c +++ b/glib/gstdio.c @@ -1552,7 +1552,7 @@ g_rmdir (const gchar *filename) * * As `close()` and `fclose()` are part of the C library, this implies that it is * currently impossible to close a file if the application C library and the C library - * used by GLib are different. Convenience functions like g_file_set_contents() + * used by GLib are different. Convenience functions like g_file_set_contents_full() * avoid this problem. * * Returns: A `FILE*` if the file was successfully opened, or %NULL if @@ -1660,10 +1660,13 @@ g_freopen (const gchar *filename, * g_fsync: * @fd: a file descriptor * - * A wrapper for the POSIX fsync() function (_commit() on Windows). - * The fsync() function is used to synchronize a file's in-core + * A wrapper for the POSIX `fsync()` function. On Windows, `_commit()` will be + * used. On macOS, `fcntl(F_FULLFSYNC)` will be used. + * The `fsync()` function is used to synchronize a file's in-core * state with that of the disk. * + * This wrapper will handle retrying on `EINTR`. + * * See the C library manual for more details about fsync(). * * Returns: 0 on success, or -1 if an error occurred. @@ -1676,8 +1679,18 @@ g_fsync (gint fd) { #ifdef G_OS_WIN32 return _commit (fd); +#elif defined(HAVE_FSYNC) || defined(HAVE_FCNTL_F_FULLFSYNC) + int retval; + do +#ifdef HAVE_FCNTL_F_FULLFSYNC + retval = fcntl (fd, F_FULLFSYNC, 0); #else - return fsync (fd); + retval = fsync (fd); +#endif + while (G_UNLIKELY (retval < 0 && errno == EINTR)); + return retval; +#else + return 0; #endif } diff --git a/glib/tests/fileutils.c b/glib/tests/fileutils.c index 46c54e805..a3aa084f6 100644 --- a/glib/tests/fileutils.c +++ b/glib/tests/fileutils.c @@ -950,6 +950,257 @@ test_set_contents (void) g_free (name); } +static void +test_set_contents_full (void) +{ + GFileSetContentsFlags flags_mask = + G_FILE_SET_CONTENTS_ONLY_EXISTING | + G_FILE_SET_CONTENTS_DURABLE | + G_FILE_SET_CONTENTS_CONSISTENT; + gint flags; + const struct + { + enum + { + EXISTING_FILE_NONE, + EXISTING_FILE_REGULAR, +#ifndef G_OS_WIN32 + EXISTING_FILE_SYMLINK, +#endif + EXISTING_FILE_DIRECTORY, + } + existing_file; + int new_mode; /* only relevant if @existing_file is %EXISTING_FILE_NONE */ + gboolean use_strlen; + + gboolean expected_success; + GFileError expected_error; + } + tests[] = + { + { EXISTING_FILE_NONE, 0644, FALSE, TRUE, 0 }, + { EXISTING_FILE_NONE, 0644, TRUE, TRUE, 0 }, + { EXISTING_FILE_NONE, 0600, FALSE, TRUE, 0 }, + { EXISTING_FILE_REGULAR, 0644, FALSE, TRUE, 0 }, +#ifndef G_OS_WIN32 + { EXISTING_FILE_SYMLINK, 0644, FALSE, TRUE, 0 }, +#endif + { EXISTING_FILE_DIRECTORY, 0644, FALSE, FALSE, G_FILE_ERROR_ISDIR }, + }; + gsize i; + + g_test_summary ("Test g_file_set_contents_full() with various flags"); + + for (flags = 0; flags < (gint) flags_mask; flags++) + { + for (i = 0; i < G_N_ELEMENTS (tests); i++) + { + GError *error = NULL; + gchar *file_name = NULL, *link_name = NULL, *dir_name = NULL; + const gchar *set_contents_name; + gchar *buf = NULL; + gsize len; + gboolean ret; + GStatBuf statbuf; + + g_test_message ("Flags %d and test %" G_GSIZE_FORMAT, flags, i); + + switch (tests[i].existing_file) + { + case EXISTING_FILE_REGULAR: +#ifndef G_OS_WIN32 + case EXISTING_FILE_SYMLINK: +#endif + { + gint fd; + + fd = g_file_open_tmp (NULL, &file_name, &error); + g_assert_no_error (error); + write (fd, "a", 1); + g_assert_no_errno (g_fsync (fd)); + close (fd); + +#ifndef G_OS_WIN32 + /* Pass an existing symlink to g_file_set_contents_full() to see + * what it does. */ + if (tests[i].existing_file == EXISTING_FILE_SYMLINK) + { + link_name = g_strconcat (file_name, ".link", NULL); + g_assert_no_errno (symlink (file_name, link_name)); + + set_contents_name = link_name; + } + else +#endif /* !G_OS_WIN32 */ + { + set_contents_name = file_name; + } + break; + } + case EXISTING_FILE_DIRECTORY: + { + dir_name = g_dir_make_tmp ("glib-fileutils-set-contents-full-XXXXXX", &error); + g_assert_no_error (error); + + set_contents_name = dir_name; + break; + } + case EXISTING_FILE_NONE: + { + file_name = g_build_filename (g_get_tmp_dir (), "glib-file-set-contents-full-test", NULL); + g_remove (file_name); + g_assert_false (g_file_test (file_name, G_FILE_TEST_EXISTS)); + + set_contents_name = file_name; + break; + } + default: + { + g_assert_not_reached (); + } + } + + /* Set the file contents */ + ret = g_file_set_contents_full (set_contents_name, "b", + tests[i].use_strlen ? -1 : 1, + flags, tests[i].new_mode, &error); + + if (!tests[i].expected_success) + { + g_assert_error (error, G_FILE_ERROR, tests[i].expected_error); + g_assert_false (ret); + g_clear_error (&error); + } + else + { + g_assert_no_error (error); + g_assert_true (ret); + + /* Check the contents and mode were set correctly. The mode isn’t + * changed on existing files. */ + ret = g_file_get_contents (set_contents_name, &buf, &len, &error); + g_assert_no_error (error); + g_assert_true (ret); + g_assert_cmpstr (buf, ==, "b"); + g_assert_cmpuint (len, ==, 1); + g_free (buf); + + g_assert_no_errno (g_lstat (set_contents_name, &statbuf)); + + if (tests[i].existing_file == EXISTING_FILE_NONE) + g_assert_cmpint (statbuf.st_mode & ~S_IFMT, ==, tests[i].new_mode); + +#ifndef G_OS_WIN32 + if (tests[i].existing_file == EXISTING_FILE_SYMLINK) + { + gchar *target_contents = NULL; + + /* If the @set_contents_name was a symlink, it should now be a + * regular file, and the file it pointed to should not have + * changed. */ + g_assert_cmpint (statbuf.st_mode & S_IFMT, ==, S_IFREG); + + g_file_get_contents (file_name, &target_contents, NULL, &error); + g_assert_no_error (error); + g_assert_cmpstr (target_contents, ==, "a"); + + g_free (target_contents); + } +#endif /* !G_OS_WIN32 */ + } + + if (dir_name != NULL) + g_rmdir (dir_name); + if (link_name != NULL) + g_remove (link_name); + if (file_name != NULL) + g_remove (file_name); + + g_free (dir_name); + g_free (link_name); + g_free (file_name); + } + } +} + +static void +test_set_contents_full_read_only_file (void) +{ + gint fd; + GError *error = NULL; + gchar *file_name = NULL; + gboolean ret; + + g_test_summary ("Test g_file_set_contents_full() on a read-only file"); + + /* Can’t test this with different #GFileSetContentsFlags as they all have + * different behaviours wrt replacing the file while noticing/ignoring the + * existing file permissions. */ + fd = g_file_open_tmp (NULL, &file_name, &error); + g_assert_no_error (error); + write (fd, "a", 1); + g_assert_no_errno (g_fsync (fd)); + close (fd); + g_assert_no_errno (chmod (file_name, 0200)); + + /* Set the file contents */ + ret = g_file_set_contents_full (file_name, "b", 1, G_FILE_SET_CONTENTS_NONE, 0644, &error); + g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_ACCES); + g_assert_false (ret); + g_clear_error (&error); + + g_remove (file_name); + + g_free (file_name); +} + +static void +test_set_contents_full_read_only_directory (void) +{ + GFileSetContentsFlags flags_mask = + G_FILE_SET_CONTENTS_ONLY_EXISTING | + G_FILE_SET_CONTENTS_DURABLE | + G_FILE_SET_CONTENTS_CONSISTENT; + gint flags; + + g_test_summary ("Test g_file_set_contents_full() on a file in a read-only directory"); + + for (flags = 0; flags < (gint) flags_mask; flags++) + { + gint fd; + GError *error = NULL; + gchar *dir_name = NULL; + gchar *file_name = NULL; + gboolean ret; + + g_test_message ("Flags %d", flags); + + dir_name = g_dir_make_tmp ("glib-file-set-contents-full-rodir-XXXXXX", &error); + g_assert_no_error (error); + + file_name = g_build_filename (dir_name, "file", NULL); + fd = g_open (file_name, O_CREAT | O_RDWR, 0644); + g_assert_cmpint (fd, >=, 0); + write (fd, "a", 1); + g_assert_no_errno (g_fsync (fd)); + close (fd); + + g_assert_no_errno (chmod (dir_name, 0)); + + /* Set the file contents */ + ret = g_file_set_contents_full (file_name, "b", 1, flags, 0644, &error); + g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_ACCES); + g_assert_false (ret); + g_clear_error (&error); + + g_remove (file_name); + g_unlink (dir_name); + + g_free (file_name); + g_free (dir_name); + } +} + static void test_read_link (void) { @@ -1523,7 +1774,7 @@ main (int argc, char *argv[]) { g_setenv ("LC_ALL", "C", TRUE); - g_test_init (&argc, &argv, NULL); + g_test_init (&argc, &argv, G_TEST_OPTION_ISOLATE_DIRS, NULL); g_test_bug_base ("https://gitlab.gnome.org/GNOME/glib/merge_requests/"); @@ -1545,6 +1796,9 @@ main (int argc, g_test_add_func ("/fileutils/mkstemp", test_mkstemp); g_test_add_func ("/fileutils/mkdtemp", test_mkdtemp); g_test_add_func ("/fileutils/set-contents", test_set_contents); + g_test_add_func ("/fileutils/set-contents-full", test_set_contents_full); + g_test_add_func ("/fileutils/set-contents-full/read-only-file", test_set_contents_full_read_only_file); + g_test_add_func ("/fileutils/set-contents-full/read-only-directory", test_set_contents_full_read_only_directory); g_test_add_func ("/fileutils/read-link", test_read_link); g_test_add_func ("/fileutils/stdio-wrappers", test_stdio_wrappers); g_test_add_func ("/fileutils/fopen-modes", test_fopen_modes); diff --git a/meson.build b/meson.build index 839401c7d..627dcce57 100644 --- a/meson.build +++ b/meson.build @@ -875,6 +875,17 @@ if cc.compiles('''#include glib_conf.set('HAVE_OPEN_O_DIRECTORY', 1) endif +# fcntl takes F_FULLFSYNC as an option +# See https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fsync.2.html +if cc.compiles('''#include + #include + #include + void some_func (void) { + fcntl(0, F_FULLFSYNC, 0); + }''', name : 'fcntl() option F_FULLFSYNC') + glib_conf.set('HAVE_FCNTL_F_FULLFSYNC', 1) +endif + # Check whether there is a vsnprintf() function with C99 semantics installed. # (similar tests to AC_FUNC_VSNPRINTF_C99) # Check whether there is a snprintf() function with C99 semantics installed.