After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 782075 - gtimer: Handle gmtime() failure in g_time_val_to_iso8601()
gtimer: Handle gmtime() failure in g_time_val_to_iso8601()
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-05-02 15:03 UTC by Philip Withnall
Modified: 2017-05-05 10:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtimer: Handle gmtime() failure in g_time_val_to_iso8601() (1.79 KB, patch)
2017-05-02 15:03 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2017-05-02 15:03:33 UTC
Minor fix, but it is technically a bit of an API break, so merits some discussion. See the commit message for rationale.
Comment 1 Philip Withnall 2017-05-02 15:03:42 UTC
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>
Comment 2 Emmanuele Bassi (:ebassi) 2017-05-04 16:03:53 UTC
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?
Comment 3 Philip Withnall 2017-05-05 10:03:43 UTC
(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.
Comment 4 Emmanuele Bassi (:ebassi) 2017-05-05 10:06:25 UTC
(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.
Comment 5 Philip Withnall 2017-05-05 10:42:11 UTC
Let’s go go go then.

Attachment 350887 [details] pushed as f9a6a9b - gtimer: Handle gmtime() failure in g_time_val_to_iso8601()