gst/gstregistrybinary.c: Use g_remove() and g_rename(). Check result of g_rename...

Original commit message from CVS:
* gst/gstregistrybinary.c: (gst_registry_binary_write_cache):
Use g_remove() and g_rename(). Check result of g_rename(), and
don't leak the open file descriptor if we error out when writing.
* gst/gstregistryxml.c: (load_plugin), (gst_registry_xml_write_cache):
Must check the return value of close() after writing out the new
registry file.  Sometimes write problems such as out-of-diskspace
are only reported when the file is closed and not already during
the write.  This may have caused partial/broken registry files in
some rare circumstances. Should fix #503675.

diff --git a/ChangeLog b/ChangeLog
index 6968189..cb8d0cd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2007-12-16  Tim-Philipp Müller  <tim at centricular dot net>
+
+       * gst/gstregistrybinary.c: (gst_registry_binary_write_cache):
+         Use g_remove() and g_rename(). Check result of g_rename(), and
+         don't leak the open file descriptor if we error out when writing.
+
+       * gst/gstregistryxml.c: (load_plugin), (gst_registry_xml_write_cache):
+         Must check the return value of close() after writing out the new
+         registry file.  Sometimes write problems such as out-of-diskspace
+         are only reported when the file is closed and not already during
+         the write.  This may have caused partial/broken registry files in
+         some rare circumstances. Should fix #503675.
+
 2007-12-16  Edward Hervey  <edward.hervey@collabora.co.uk>
 
        * docs/gst/.cvsignore:

diff --git a/gst/gstregistrybinary.c b/gst/gstregistrybinary.c
index 76396a7..fc7ebd1 100644 (file)
--- a/gst/gstregistrybinary.c
+++ b/gst/gstregistrybinary.c
@@ -544,16 +544,17 @@ gst_registry_binary_write_cache (GstRegistry * registry, const char *location)
   }
   g_list_free (to_write);
 
-  if (close (registry->cache_file) < 0) {
-    GST_DEBUG ("Can't close registry file : %s", g_strerror (errno));
-    goto fail;
-  }
+  if (close (registry->cache_file) < 0)
+    goto close_failed;
 
   if (g_file_test (tmp_location, G_FILE_TEST_EXISTS)) {
 #ifdef WIN32
-    remove (location);
+    g_remove (location);
 #endif
-    rename (tmp_location, location);
+    if (g_rename (tmp_location, location) < 0)
+      goto rename_failed;
+  } else {
+    /* FIXME: shouldn't we return FALSE here? */
   }
 
   g_free (tmp_location);
@@ -562,8 +563,26 @@ gst_registry_binary_write_cache (GstRegistry * registry, const char *location)
 
   /* Errors */
 fail:
-  g_free (tmp_location);
-  return FALSE;
+  {
+    (void) close (registry->cache_file);
+    /* fall through */
+  }
+fail_after_close:
+  {
+    g_remove (tmp_location);
+    g_free (tmp_location);
+    return FALSE;
+  }
+close_failed:
+  {
+    GST_ERROR ("close() failed: %s", g_strerror (errno));
+    goto fail_after_close;
+  }
+rename_failed:
+  {
+    GST_ERROR ("g_rename() failed: %s", g_strerror (errno));
+    goto fail_after_close;
+  }
 }
 
 

diff --git a/gst/gstregistryxml.c b/gst/gstregistryxml.c
index 1c0f592..27d2a13 100644 (file)
--- a/gst/gstregistryxml.c
+++ b/gst/gstregistryxml.c
@@ -916,22 +916,44 @@ gst_registry_xml_write_cache (GstRegistry * registry, const char *location)
   if (!gst_registry_save (registry, "</GST-PluginRegistry>\n"))
     goto fail;
 
-  close (registry->cache_file);
+  /* check return value of close(), write errors may only get reported here */
+  if (close (registry->cache_file) < 0)
+    goto close_failed;
 
   if (g_file_test (tmp_location, G_FILE_TEST_EXISTS)) {
 #ifdef WIN32
     g_remove (location);
 #endif
-    g_rename (tmp_location, location);
+    if (g_rename (tmp_location, location) < 0)
+      goto rename_failed;
+  } else {
+    /* FIXME: shouldn't we return FALSE here? */
   }
 
   g_free (tmp_location);
-
+  GST_INFO ("Wrote XML registry cache");
   return TRUE;
 
+/* ERRORS */
 fail:
-  close (registry->cache_file);
-  g_free (tmp_location);
-
-  return FALSE;
+  {
+    (void) close (registry->cache_file);
+    /* fall through */
+  }
+fail_after_close:
+  {
+    g_remove (tmp_location);
+    g_free (tmp_location);
+    return FALSE;
+  }
+close_failed:
+  {
+    GST_ERROR ("close() failed: %s", g_strerror (errno));
+    goto fail_after_close;
+  }
+rename_failed:
+  {
+    GST_ERROR ("g_rename() failed: %s", g_strerror (errno));
+    goto fail_after_close;
+  }
 }