GNOME Bugzilla – Bug 782075
gtimer: Handle gmtime() failure in g_time_val_to_iso8601()
Last modified: 2017-05-05 10:42:15 UTC
Minor fix, but it is technically a bit of an API break, so merits some discussion. See the commit message for rationale.
Created attachment 350887 [details] [review] gtimer: Handle gmtime() failure in g_time_val_to_iso8601() g_time_val_to_iso8601() has a limit to the future dates it can convert, imposed by what gmtime() can fit in its year field. If gmtime() fails, gracefully return NULL from g_time_val_to_iso8601() rather than trying to dereference the NULL structure and crashing. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 350887 [details] [review]: I think it's perfectly fine to change the result to a nullable string, but the introspection ABI, as you mentioned, will change. Maybe we should return an empty string instead?
(In reply to Emmanuele Bassi (:ebassi) from comment #2) > Review of attachment 350887 [details] [review] [review]: > > I think it's perfectly fine to change the result to a nullable string, but > the introspection ABI, as you mentioned, will change. I think that’s probably fine; we’re still fairly routinely adding missing (nullable) annotations in other places. > Maybe we should return an empty string instead? Eww. I guess we could, if we’re averse to changing the introspection API.
(In reply to Philip Withnall from comment #3) > (In reply to Emmanuele Bassi (:ebassi) from comment #2) > > Review of attachment 350887 [details] [review] [review] [review]: > > > > I think it's perfectly fine to change the result to a nullable string, but > > the introspection ABI, as you mentioned, will change. > > I think that’s probably fine; we’re still fairly routinely adding missing > (nullable) annotations in other places. > > > Maybe we should return an empty string instead? > > Eww. I guess we could, if we’re averse to changing the introspection API. I'm not — we do, as you mentioned, routinely add nullable/optional/transfer annotations. I just wanted to add a potential workaround, in case this is brought up as a breakage. As far as I'm concerned, attachment 350887 [details] [review] is ACK-by: me.
Let’s go go go then. Attachment 350887 [details] pushed as f9a6a9b - gtimer: Handle gmtime() failure in g_time_val_to_iso8601()