GNOME Bugzilla – Bug 643811
Date format for vimeo plugin
Last modified: 2011-04-01 15:18:10 UTC
Created attachment 182390 [details] [review] patch to add iso8601 date Other plugins seem to use iso8601[1]. Convert vimeo's date format to iso8601 format, hopefully we can have a consistent date format. [1] http://en.wikipedia.org/wiki/ISO_8601
Review of attachment 182390 [details] [review]: Quick review ::: src/vimeo/grl-vimeo.c @@ +205,3 @@ + date = g_strsplit (str, " ", -1); + return g_strdup_printf ("%sT%sZ", date[0], date[1]); +} You are leaking date here, you need to call g_strfreev() on it. @@ +246,3 @@ if (str) { + grl_media_set_date (media, str_to_iso8601(str)); potentially leaking the new string allocated with g_strdup_printf() don't have the semantics of grl_media_set_date() wrt the passed string but seeing that it used to just give the string from the hash table I guess the string returned by str_to_iso8601()
Review of attachment 182390 [details] [review]: ::: src/vimeo/grl-vimeo.c @@ +205,3 @@ + date = g_strsplit (str, " ", -1); + return g_strdup_printf ("%sT%sZ", date[0], date[1]); +} Yes, I've realized about that. The pushed patch has this leak fixed. Thanks! @@ +246,3 @@ if (str) { + grl_media_set_date (media, str_to_iso8601(str)); Yes, didn't realize. I pushed a fix for this (commit f7efacc1f04a20abf86243df786c71abfbc01d4b).
This was fixed as commit f7efacc1f04a20abf86243df786c71abfbc01d4b Author: Juan A. Suarez Romero <jasuarez@igalia.com> Date: Thu Mar 3 20:51:51 2011 +0100 vimeo: Plug a leak Do not leak iso8601 date. Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com> src/vimeo/grl-vimeo.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) commit 0a096e65cae6a5eb2e5ca22197c5d3f8e11d580f Author: Michael Wood <michael.g.wood@linux.intel.com> Date: Thu Mar 3 19:05:10 2011 +0000 vimeo: Add iso8601 formatting for date metadata Fixes https://bugzilla.gnome.org/show_bug.cgi?id=643811 src/vimeo/grl-vimeo.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-)