gst/gstregistrybinary.c: Use g_remove() and g_rename(). Check result of g_rename(), and don't leak the open file desc...

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.

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:

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) {
+  if (close (registry->cache_file) < 0)
-    GST_DEBUG ("Can't close registry file : %s", g_strerror (errno));
+    goto close_failed;
-    goto fail;
-  }
 
   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;
+  }
 }
 
 

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);
+    (void) close (registry->cache_file);
-
+    /* fall through */
-  return FALSE;
+  }
+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;
+  }
 }