util: Simplify purple_util_write_data_to_file_absolute()

Tue, 24 Apr 2018 23:32:23 -0500

author
Mike Ruprecht <cmaiku@gmail.com>
date
Tue, 24 Apr 2018 23:32:23 -0500
changeset 38996
01ac2f470b5f
parent 38995
80586ad5c5a4
child 38997
dd61084f0101

util: Simplify purple_util_write_data_to_file_absolute()

This patch replaces the contents of
purple_util_write_data_to_file_absolute() with g_file_set_contents().
I compared the two functions and it seems the only things the latter
doesn't do that the former does is:
1: use fflush()
2: explicitly check that the file size matches what was requested

g_file_set_contents() used to use fflush(), but was changed to use
write() instead of fwrite() so it's possible that eliminates the need?
I expect it should also be fine that it doesn't explicitly check the
file size separately either these days.

libpurple/util.c file | annotate | diff | comparison | revisions
--- a/libpurple/util.c	Tue May 01 16:32:13 2018 -0400
+++ b/libpurple/util.c	Tue Apr 24 23:32:23 2018 -0500
@@ -3196,158 +3196,17 @@
 	return ret;
 }
 
-/*
- * This function is long and beautiful, like my--um, yeah.  Anyway,
- * it includes lots of error checking so as we don't overwrite
- * people's settings if there is a problem writing the new values.
- */
 gboolean
 purple_util_write_data_to_file_absolute(const char *filename_full, const char *data, gssize size)
 {
-	gchar *filename_temp;
-	FILE *file;
-	gsize real_size, byteswritten;
-	GStatBuf st;
-#ifndef HAVE_FILENO
-	int fd;
-#endif
-
-	purple_debug_misc("util", "Writing file %s",
-					filename_full);
-
-	g_return_val_if_fail((size >= -1), FALSE);
-
-	filename_temp = g_strdup_printf("%s.save", filename_full);
-
-	/* Remove an old temporary file, if one exists */
-	if (g_file_test(filename_temp, G_FILE_TEST_EXISTS))
-	{
-		if (g_unlink(filename_temp) == -1)
-		{
-			purple_debug_error("util", "Error removing old file "
-					   "%s: %s\n",
-					   filename_temp, g_strerror(errno));
-		}
-	}
-
-	/* Open file */
-	file = g_fopen(filename_temp, "wb");
-	if (file == NULL)
-	{
-		purple_debug_error("util", "Error opening file %s for "
-				   "writing: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		return FALSE;
-	}
-
-	/* Write to file */
-	real_size = (size == -1) ? strlen(data) : (size_t) size;
-	byteswritten = fwrite(data, 1, real_size, file);
-
-#ifdef HAVE_FILENO
-#ifndef _WIN32
-	/* Set file permissions */
-	if (fchmod(fileno(file), S_IRUSR | S_IWUSR) == -1) {
-		purple_debug_error("util", "Error setting permissions of "
-			"file %s: %s\n", filename_temp, g_strerror(errno));
-	}
-#endif
-
-	/* Apparently XFS (and possibly other filesystems) do not
-	 * guarantee that file data is flushed before file metadata,
-	 * so this procedure is insufficient without some flushage. */
-	if (fflush(file) < 0) {
-		purple_debug_error("util", "Error flushing %s: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		fclose(file);
-		return FALSE;
-	}
-	if (fsync(fileno(file)) < 0) {
-		purple_debug_error("util", "Error syncing file contents for %s: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		fclose(file);
+	GError *err = NULL;
+
+	if (!g_file_set_contents(filename_full, data, size, &err)) {
+		purple_debug_error("util", "Error writing file: %s: %s\n",
+				   filename_full, err->message);
+		g_clear_error(&err);
 		return FALSE;
 	}
-#endif
-
-	/* Close file */
-	if (fclose(file) != 0)
-	{
-		purple_debug_error("util", "Error closing file %s: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		return FALSE;
-	}
-
-#ifndef HAVE_FILENO
-	/* This is the same effect (we hope) as the HAVE_FILENO block
-	 * above, but for systems without fileno(). */
-	if ((fd = open(filename_temp, O_RDWR)) < 0) {
-		purple_debug_error("util", "Error opening file %s for flush: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		return FALSE;
-	}
-
-#ifndef _WIN32
-	/* copy-pasta! */
-	if (fchmod(fd, S_IRUSR | S_IWUSR) == -1) {
-		purple_debug_error("util", "Error setting permissions of "
-			"file %s: %s\n", filename_temp, g_strerror(errno));
-	}
-#endif
-
-	if (fsync(fd) < 0) {
-		purple_debug_error("util", "Error syncing %s: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		close(fd);
-		return FALSE;
-	}
-	if (close(fd) < 0) {
-		purple_debug_error("util", "Error closing %s after sync: %s\n",
-				   filename_temp, g_strerror(errno));
-		g_free(filename_temp);
-		return FALSE;
-	}
-#endif
-
-	/* Ensure the file is the correct size */
-	if (byteswritten != real_size)
-	{
-		purple_debug_error("util", "Error writing to file %s: Wrote %"
-				   G_GSIZE_FORMAT " bytes "
-				   "but should have written %" G_GSIZE_FORMAT
-				   "; is your disk full?\n",
-				   filename_temp, byteswritten, real_size);
-		g_free(filename_temp);
-		return FALSE;
-	}
-#ifndef __COVERITY__
-	/* Use stat to be absolutely sure.
-	 * It causes TOCTOU coverity warning (against g_rename below),
-	 * but it's not a threat for us.
-	 */
-	if ((g_stat(filename_temp, &st) == -1) || ((gsize)st.st_size != real_size)) {
-		purple_debug_error("util", "Error writing data to file %s: "
-			"couldn't g_stat file", filename_temp);
-		g_free(filename_temp);
-		return FALSE;
-	}
-#endif /* __COVERITY__ */
-
-	/* Rename to the REAL name */
-	if (g_rename(filename_temp, filename_full) == -1)
-	{
-		purple_debug_error("util", "Error renaming %s to %s: %s\n",
-				   filename_temp, filename_full,
-				   g_strerror(errno));
-	}
-
-	g_free(filename_temp);
 
 	return TRUE;
 }

mercurial