From 81cc6d92babd7ff5e5afe02e9875c56c0d13878a Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 8 Mar 2006 12:57:37 +0000 Subject: [PATCH] gst/: Rewrite registry-saving to avoid race conditions and check for failed writes. Original commit message from CVS: * gst/gstregistry.h: * gst/gstregistryxml.c: (gst_registry_save), (gst_registry_save_escaped), (gst_registry_xml_save_caps), (gst_registry_xml_save_pad_template), (gst_registry_xml_save_feature), (gst_registry_xml_save_plugin), (gst_registry_xml_write_cache): Rewrite registry-saving to avoid race conditions and check for failed writes. --- ChangeLog | 11 +++ common | 2 +- gst/gstregistry.h | 4 +- gst/gstregistryxml.c | 221 ++++++++++++++++++++++++++++++------------- 4 files changed, 170 insertions(+), 68 deletions(-) diff --git a/ChangeLog b/ChangeLog index d342185026..c4fbf65231 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2006-03-08 Michael Smith + + * gst/gstregistry.h: + * gst/gstregistryxml.c: (gst_registry_save), + (gst_registry_save_escaped), (gst_registry_xml_save_caps), + (gst_registry_xml_save_pad_template), + (gst_registry_xml_save_feature), (gst_registry_xml_save_plugin), + (gst_registry_xml_write_cache): + Rewrite registry-saving to avoid race conditions and check for + failed writes. + 2006-03-08 Wim Taymans * libs/gst/base/gstbasetransform.c: diff --git a/common b/common index c09cd18d32..d576cc6779 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit c09cd18d328f740ac532377fa5605b0f712cc6fd +Subproject commit d576cc6779aa9555121d4c78ab69cc620fae3e2b diff --git a/gst/gstregistry.h b/gst/gstregistry.h index c86ba59c8c..b24271e48f 100644 --- a/gst/gstregistry.h +++ b/gst/gstregistry.h @@ -54,8 +54,8 @@ struct _GstRegistry { GList *paths; - /* FIXME move elsewhere */ - FILE *cache_file; + /* FIXME move these elsewhere */ + int cache_file; /*< private >*/ gpointer _gst_reserved[GST_PADDING]; diff --git a/gst/gstregistryxml.c b/gst/gstregistryxml.c index 9e5fe25224..3689b0d86f 100644 --- a/gst/gstregistryxml.c +++ b/gst/gstregistryxml.c @@ -56,17 +56,32 @@ #define CLASS(registry) GST_XML_REGISTRY_CLASS (G_OBJECT_GET_CLASS (registry)) static gboolean -gst_registry_xml_save (GstRegistry * registry, gchar * format, ...) +gst_registry_save (GstRegistry * registry, gchar * format, ...) { va_list var_args; + size_t written, len; + gboolean ret; + char *str; va_start (var_args, format); - - vfprintf (registry->cache_file, format, var_args); - + str = g_strdup_vprintf (format, var_args); va_end (var_args); - return TRUE; + len = strlen (str); + + written = write (registry->cache_file, str, len); + + if (len == written) + ret = TRUE; + else { + ret = FALSE; + GST_ERROR ("Failed to write registry to temporary file: %s", + g_strerror (errno)); + } + + g_free (str); + + return ret; } static void @@ -527,15 +542,21 @@ gst_registry_xml_read_cache (GstRegistry * registry, const char *location) /* * Save */ -#define PUT_ESCAPED(prefix,tag,value) \ -G_STMT_START{ \ - const gchar *toconv = value; \ - if (toconv) { \ - gchar *v = g_markup_escape_text (toconv, strlen (toconv)); \ - gst_registry_xml_save (registry, prefix "<%s>%s\n", tag, v, tag); \ - g_free (v); \ - } \ -}G_STMT_END +static gboolean +gst_registry_save_escaped (GstRegistry * registry, char *prefix, char *tag, + char *value) +{ + gboolean ret = TRUE; + + if (value) { + gchar *v = g_markup_escape_text (value, strlen (value)); + + ret = gst_registry_save (registry, "%s<%s>%s\n", prefix, tag, v, tag); + g_free (v); + } + + return ret; +} static gboolean @@ -545,14 +566,15 @@ gst_registry_xml_save_caps (GstRegistry * registry, const GstCaps * caps) * faster when loading them later on */ char *s; GstCaps *copy = gst_caps_copy (caps); + gboolean ret; gst_caps_do_simplify (copy); s = gst_caps_to_string (copy); gst_caps_unref (copy); - PUT_ESCAPED (" ", "caps", s); + ret = gst_registry_save_escaped (registry, " ", "caps", s); g_free (s); - return TRUE; + return ret; } static gboolean @@ -561,9 +583,14 @@ gst_registry_xml_save_pad_template (GstRegistry * registry, { gchar *presence; - PUT_ESCAPED (" ", "nametemplate", template->name_template); - gst_registry_xml_save (registry, " %s\n", - (template->direction == GST_PAD_SINK ? "sink" : "src")); + if (!gst_registry_save_escaped (registry, " ", "nametemplate", + template->name_template)) + return FALSE; + + if (!gst_registry_save (registry, + " %s\n", + (template->direction == GST_PAD_SINK ? "sink" : "src"))) + return FALSE; switch (template->presence) { case GST_PAD_ALWAYS: @@ -579,11 +606,13 @@ gst_registry_xml_save_pad_template (GstRegistry * registry, presence = "unknown"; break; } - gst_registry_xml_save (registry, " %s\n", presence); + if (!gst_registry_save (registry, " %s\n", presence)) + return FALSE; if (template->static_caps.string) { - gst_registry_xml_save (registry, " %s\n", - template->static_caps.string); + if (!gst_registry_save (registry, " %s\n", + template->static_caps.string)) + return FALSE; } return TRUE; } @@ -592,50 +621,68 @@ static gboolean gst_registry_xml_save_feature (GstRegistry * registry, GstPluginFeature * feature) { - PUT_ESCAPED (" ", "name", feature->name); + if (!gst_registry_save_escaped (registry, " ", "name", feature->name)) + return FALSE; if (feature->rank > 0) { gint rank = feature->rank; - gst_registry_xml_save (registry, " %d\n", rank); + if (!gst_registry_save (registry, " %d\n", rank)) + return FALSE; } if (GST_IS_ELEMENT_FACTORY (feature)) { GstElementFactory *factory = GST_ELEMENT_FACTORY (feature); GList *walk; - PUT_ESCAPED (" ", "longname", factory->details.longname); - PUT_ESCAPED (" ", "class", factory->details.klass); - PUT_ESCAPED (" ", "description", factory->details.description); - PUT_ESCAPED (" ", "author", factory->details.author); + if (!gst_registry_save_escaped (registry, " ", "longname", + factory->details.longname)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "class", + factory->details.klass)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "description", + factory->details.description)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "author", + factory->details.author)) + return FALSE; walk = factory->staticpadtemplates; while (walk) { GstStaticPadTemplate *template = walk->data; - gst_registry_xml_save (registry, " \n"); - gst_registry_xml_save_pad_template (registry, template); - gst_registry_xml_save (registry, " \n"); + if (!gst_registry_save (registry, " \n")) + return FALSE; + if (!gst_registry_xml_save_pad_template (registry, template)) + return FALSE; + if (!gst_registry_save (registry, " \n")) + return FALSE; walk = g_list_next (walk); } walk = factory->interfaces; while (walk) { - PUT_ESCAPED (" ", "interface", (gchar *) walk->data); + if (!gst_registry_save_escaped (registry, " ", "interface", + (gchar *) walk->data)) + return FALSE; walk = g_list_next (walk); } if (GST_URI_TYPE_IS_VALID (factory->uri_type)) { gchar **protocol; - PUT_ESCAPED (" ", "uri_type", - factory->uri_type == GST_URI_SINK ? "sink" : "source"); + if (!gst_registry_save_escaped (registry, " ", "uri_type", + factory->uri_type == GST_URI_SINK ? "sink" : "source")) + return FALSE; g_assert (factory->uri_protocols); protocol = factory->uri_protocols; while (*protocol) { - PUT_ESCAPED (" ", "uri_protocol", *protocol); + if (!gst_registry_save_escaped (registry, " ", "uri_protocol", + *protocol)) + return FALSE; protocol++; } } @@ -644,16 +691,21 @@ gst_registry_xml_save_feature (GstRegistry * registry, gint i = 0; if (factory->caps) { - gst_registry_xml_save_caps (registry, factory->caps); + if (!gst_registry_xml_save_caps (registry, factory->caps)) + return FALSE; } if (factory->extensions) { while (factory->extensions[i]) { - PUT_ESCAPED (" ", "extension", factory->extensions[i]); + if (!gst_registry_save_escaped (registry, " ", "extension", + factory->extensions[i])) + return FALSE; i++; } } } else if (GST_IS_INDEX_FACTORY (feature)) { - PUT_ESCAPED (" ", "longdesc", GST_INDEX_FACTORY (feature)->longdesc); + if (!gst_registry_save_escaped (registry, " ", "longdesc", + GST_INDEX_FACTORY (feature)->longdesc)) + return FALSE; } return TRUE; } @@ -665,33 +717,58 @@ gst_registry_xml_save_plugin (GstRegistry * registry, GstPlugin * plugin) GList *walk; char s[100]; - PUT_ESCAPED (" ", "name", plugin->desc.name); - PUT_ESCAPED (" ", "description", plugin->desc.description); - PUT_ESCAPED (" ", "filename", plugin->filename); + if (!gst_registry_save_escaped (registry, " ", "name", plugin->desc.name)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "description", + plugin->desc.description)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "filename", plugin->filename)) + return FALSE; + sprintf (s, "%d", (int) plugin->file_size); - PUT_ESCAPED (" ", "size", s); + if (!gst_registry_save_escaped (registry, " ", "size", s)) + return FALSE; + sprintf (s, "%d", (int) plugin->file_mtime); - PUT_ESCAPED (" ", "m32p", s); - PUT_ESCAPED (" ", "version", plugin->desc.version); - PUT_ESCAPED (" ", "license", plugin->desc.license); - PUT_ESCAPED (" ", "source", plugin->desc.source); - PUT_ESCAPED (" ", "package", plugin->desc.package); - PUT_ESCAPED (" ", "origin", plugin->desc.origin); + if (!gst_registry_save_escaped (registry, " ", "m32p", s)) + return FALSE; + + if (!gst_registry_save_escaped (registry, " ", "version", + plugin->desc.version)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "license", + plugin->desc.license)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "source", plugin->desc.source)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "package", + plugin->desc.package)) + return FALSE; + if (!gst_registry_save_escaped (registry, " ", "origin", plugin->desc.origin)) + return FALSE; list = gst_registry_get_feature_list_by_plugin (registry, plugin->desc.name); for (walk = list; walk; walk = g_list_next (walk)) { GstPluginFeature *feature = GST_PLUGIN_FEATURE (walk->data); - gst_registry_xml_save (registry, " \n", - g_type_name (G_OBJECT_TYPE (feature))); - gst_registry_xml_save_feature (registry, feature); - gst_registry_xml_save (registry, " \n"); + if (!gst_registry_save (registry, + " \n", + g_type_name (G_OBJECT_TYPE (feature)))) + goto fail; + if (!gst_registry_xml_save_feature (registry, feature)) + goto fail; + if (!gst_registry_save (registry, " \n")) + goto fail; } gst_plugin_feature_list_free (list); - return TRUE; + +fail: + gst_plugin_feature_list_free (list); + return FALSE; + } /** @@ -712,9 +789,9 @@ gst_registry_xml_write_cache (GstRegistry * registry, const char *location) g_return_val_if_fail (GST_IS_REGISTRY (registry), FALSE); - tmp_location = g_strconcat (location, ".tmp", NULL); - registry->cache_file = fopen (tmp_location, "w"); - if (registry->cache_file == NULL) { + tmp_location = g_strconcat (location, ".tmpXXXXXX", NULL); + registry->cache_file = g_mkstemp (tmp_location); + if (registry->cache_file == -1) { char *dir; /* oops, I bet the directory doesn't exist */ @@ -722,14 +799,17 @@ gst_registry_xml_write_cache (GstRegistry * registry, const char *location) g_mkdir_with_parents (dir, 0777); g_free (dir); - registry->cache_file = fopen (tmp_location, "w"); + registry->cache_file = g_mkstemp (tmp_location); } - if (registry->cache_file == NULL) { + if (registry->cache_file == -1) { + g_free (tmp_location); return FALSE; } - gst_registry_xml_save (registry, "\n"); - gst_registry_xml_save (registry, "\n"); + if (!gst_registry_save (registry, "\n")) + goto fail; + if (!gst_registry_save (registry, "\n")) + goto fail; for (walk = g_list_last (registry->plugins); walk; @@ -752,13 +832,17 @@ gst_registry_xml_write_cache (GstRegistry * registry, const char *location) } } - gst_registry_xml_save (registry, "\n"); - gst_registry_xml_save_plugin (registry, plugin); - gst_registry_xml_save (registry, "\n"); + if (!gst_registry_save (registry, "\n")) + goto fail; + if (!gst_registry_xml_save_plugin (registry, plugin)) + goto fail; + if (!gst_registry_save (registry, "\n")) + goto fail; } - gst_registry_xml_save (registry, "\n"); + if (!gst_registry_save (registry, "\n")) + goto fail; - fclose (registry->cache_file); + close (registry->cache_file); if (g_file_test (tmp_location, G_FILE_TEST_EXISTS)) { #ifdef WIN32 @@ -766,7 +850,14 @@ gst_registry_xml_write_cache (GstRegistry * registry, const char *location) #endif rename (tmp_location, location); } + g_free (tmp_location); return TRUE; + +fail: + close (registry->cache_file); + g_free (tmp_location); + + return FALSE; }