Re: g_file_write()

[Soeren Sandmann <sandmann daimi au dk>]

Havoc Pennington <hp redhat com> writes:

> Should it do the atomic write thing? (write to a temporary file
> alongside the new file, then rename() to the final filename)
> 
> The minus is that you need double the size of the file to write it, but
> the plus is that you aren't screwed if you run out of disk space mid-
> overwrite.

I think it makes sense to do the atomic write. Here is a new patch
that does that. It also uses the g_* versions where appropriate and
hopefully doesn't leak.

Patch is attached and also available at 

http://www.daimi.au.dk/~sandmann/filewrite.patch

Thanks,
Søren

? filewrite.patch
Index: gfileutils.c
===================================================================
RCS file: /cvs/gnome/glib/glib/gfileutils.c,v
retrieving revision 1.59
diff -u -r1.59 gfileutils.c
--- gfileutils.c	24 Feb 2005 23:46:36 -0000	1.59
+++ gfileutils.c	6 Mar 2005 23:14:24 -0000
@@ -811,6 +811,180 @@
 
 #endif
 
+static gchar *
+write_to_temp_file (const gchar *contents,
+		    gint length,
+		    const gchar *template,
+		    GError **err)
+{
+  gchar *tmp_name;
+  gchar *display_name;
+  gchar *retval;
+  FILE *file;
+  gint fd;
+  int save_errno;
+
+  retval = NULL;
+  
+  tmp_name = g_strdup_printf (".%s.XXXXXX", template);
+
+  errno = 0;
+  fd = g_mkstemp (tmp_name);
+  save_errno = errno;
+  display_name = g_filename_display_name (tmp_name);
+      
+  if (fd == -1)
+    {
+      g_set_error (err,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (save_errno),
+		   _("Failed to create file '%s': %s"),
+		   display_name, g_strerror (save_errno));
+      
+      goto out;
+    }
+
+  errno = 0;
+  file = g_fopen (tmp_name, "wb");
+  if (!file)
+    {
+      g_set_error (err,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (errno),
+		   _("Failed to open file '%s' for writing: g_fopen() failed: %s"),
+		   display_name,
+		   g_strerror (errno));
+
+      goto out;
+    }
+
+  /* We have a FILE pointer now, so close the filedescriptor */
+
+  if (length > 0)
+    {
+      size_t n_written;
+      
+      errno = 0;
+
+      n_written = fwrite (contents, 1, length, file);
+      
+      if (n_written < length)
+	{
+ 	  g_set_error (err,
+		       G_FILE_ERROR,
+		       g_file_error_from_errno (errno),
+		       _("Failed to write file '%s': fwrite() failed: %s"),
+		       display_name,
+		       g_strerror (errno));
+
+	  goto out;
+	}
+    }
+  
+  errno = 0;
+  if (fclose (file) == EOF)
+    {
+      g_set_error (err,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (errno),
+		   _("Failed to close file '%s': fclose() failed: %s"),
+		   display_name, 
+		   g_strerror (errno));
+
+      goto out;
+    }
+  
+  retval = g_strdup (tmp_name);
+
+ out:
+  if (fd != -1)
+    close (fd);
+  g_free (tmp_name);
+  g_free (display_name);
+  
+  return retval;
+}
+
+/**
+ * g_file_write:
+ * @filename: name of a file to write @contents to, in the GLib file name encoding
+ * @contents: string to write to the file
+ * @length: length of @contents, or -1 if @contents is nul-terminated
+ * @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.
+ *
+ * If the call was sucessful, it returns %TRUE. If the call was not successful,
+ * it returns  %FALSE and sets @error. The error domain is #G_FILE_ERROR. Possible
+ * error codes are those in the #GFileError enumeration.
+ *
+ * Return value: %TRUE on success, %FALSE if an error occurred
+ **/
+gboolean
+g_file_write (const gchar *filename,
+	      const gchar *contents,
+	      gsize	   length,
+	      GError	 **error)
+{
+  char *tmp_filename = NULL;
+  char *display_filename = NULL;
+  char *display_tmpname = NULL;
+  gboolean retval;
+  
+  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);
+ 
+  if (length == -1)
+    length = strlen (contents);
+
+  retval = FALSE;
+  
+  tmp_filename = write_to_temp_file (contents, length, filename, error);
+  
+  if (!tmp_filename)
+    goto out;
+
+  display_tmpname = g_filename_display_name (tmp_filename);
+  display_filename = g_filename_display_name (filename);
+  
+  if (g_file_test (filename, G_FILE_TEST_EXISTS) && g_unlink (filename) == -1)
+    {
+      g_set_error (error,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (errno),
+		   _("Existing file '%s' could not be removed: g_unlink() failed: %s"),
+		   display_filename,
+		   g_strerror (errno));
+      
+      g_unlink (tmp_filename);
+      goto out;
+    }
+
+  if (g_rename (tmp_filename, filename) == -1)
+    {
+      g_set_error (error,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (errno),
+		   _("Failed to rename file '%s' to '%s': g_rename() failed: %s"),
+		   display_tmpname,
+		   display_filename,
+		   g_strerror (errno));
+
+      g_unlink (tmp_filename);
+      goto out;
+    }
+
+  retval = TRUE;
+  
+ out:
+  g_free (display_tmpname);
+  g_free (display_filename);
+  g_free (tmp_filename);
+  return retval;
+}
+
 /*
  * mkstemp() implementation is from the GNU C library.
  * Copyright (C) 1991,92,93,94,95,96,97,98,99 Free Software Foundation, Inc.
Index: gfileutils.h
===================================================================
RCS file: /cvs/gnome/glib/glib/gfileutils.h,v
retrieving revision 1.14
diff -u -r1.14 gfileutils.h
--- gfileutils.h	27 Oct 2004 16:46:29 -0000	1.14
+++ gfileutils.h	6 Mar 2005 23:14:24 -0000
@@ -86,6 +86,10 @@
                               gchar       **contents,
                               gsize        *length,    
                               GError      **error);
+gboolean g_file_write        (const gchar  *filename,
+			      const gchar  *contents,
+			      gsize 	    length,
+			      GError	  **error);
 gchar   *g_file_read_link    (const gchar  *filename,
 			      GError      **error);
 
Index: gmain.c
===================================================================
RCS file: /cvs/gnome/glib/glib/gmain.c,v
retrieving revision 1.125
diff -u -r1.125 gmain.c
--- gmain.c	8 Nov 2004 18:49:35 -0000	1.125
+++ gmain.c	6 Mar 2005 23:14:25 -0000
@@ -34,7 +34,9 @@
 #include "config.h"
 
 /* uncomment the next line to get poll() debugging info */
-/* #define G_MAIN_POLL_DEBUG */
+#if 0
+#define G_MAIN_POLL_DEBUG
+#endif
 
 #include "galias.h"
 #include "glib.h"
@@ -2478,6 +2480,10 @@
 
   UNLOCK_CONTEXT (context);
 
+#if 0
+  g_print ("n_ready: %d\n", n_ready);
+#endif
+  
   return n_ready > 0;
 }
 
@@ -2555,7 +2561,7 @@
   
   UNLOCK_CONTEXT (context);
 
-  g_main_context_prepare (context, &max_priority); 
+  gboolean ready_before_poll = g_main_context_prepare (context, &max_priority);
   
   while ((nfds = g_main_context_query (context, max_priority, &timeout, fds, 
 				       allocated_nfds)) > allocated_nfds)
@@ -2570,10 +2576,11 @@
   if (!block)
     timeout = 0;
   
-  g_main_context_poll (context, timeout, max_priority, fds, nfds);
+  if (!ready_before_poll)
+    g_main_context_poll (context, timeout, max_priority, fds, nfds);
   
   some_ready = g_main_context_check (context, max_priority, fds, nfds);
-  
+
   if (dispatch)
     g_main_context_dispatch (context);
   
@@ -2849,6 +2856,22 @@
   return loop->context;
 }
 
+static double
+timeval_to_ms (const GTimeVal *timeval)
+{
+  return (timeval->tv_sec * G_USEC_PER_SEC + timeval->tv_usec) / 1000.0;
+}
+
+static double
+time_diff (const GTimeVal *first,
+	   const GTimeVal *second)
+{
+  double first_ms = timeval_to_ms (first);
+  double second_ms = timeval_to_ms (second);
+
+  return first_ms - second_ms;
+}
+
 /* HOLDS: context's lock */
 static void
 g_main_context_poll (GMainContext *context,
@@ -2867,6 +2890,7 @@
 
   if (n_fds || timeout != 0)
     {
+	GTimeVal before, after; int ret;
 #ifdef	G_MAIN_POLL_DEBUG
       g_print ("g_main_poll(%d) timeout: %d\n", n_fds, timeout);
       poll_timer = g_timer_new ();
@@ -2877,7 +2901,10 @@
       poll_func = context->poll_func;
       
       UNLOCK_CONTEXT (context);
-      if ((*poll_func) (fds, n_fds, timeout) < 0 && errno != EINTR)
+      
+      g_get_current_time (&before);
+      
+      if ((ret = (*poll_func) (fds, n_fds, timeout)) < 0 && errno != EINTR)
 	{
 #ifndef G_OS_WIN32
 	  g_warning ("poll(2) failed due to: %s.",
@@ -2886,6 +2913,13 @@
 	  /* If g_poll () returns -1, it has already called g_warning() */
 #endif
 	}
+
+      g_get_current_time (&after);
+
+#if 0
+      g_print ("time spent in poll(..., %d) -> %d: %d ms\n", timeout, 
+	       ret, (int)time_diff (&after, &before));
+#endif
       
 #ifdef	G_MAIN_POLL_DEBUG
       LOCK_CONTEXT (context);