diff --git a/ChangeLog b/ChangeLog index 696818941a..cb8d0cda98 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2007-12-16 Tim-Philipp Müller + + * 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 * docs/gst/.cvsignore: diff --git a/gst/gstregistrybinary.c b/gst/gstregistrybinary.c index 76396a7b79..fc7ebd1ca3 100644 --- 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 1c0f5924ae..27d2a13113 100644 --- 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, "\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; + } }