From 4ff3734ff5748cbc53f7b89c7148a4e3daba3230 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Mon, 26 Nov 2018 16:04:10 +0000 Subject: [PATCH] g_timeout_*_seconds: don't overflow for large intervals Previously, the `guint interval` parameter, measured in seconds, was multiplied by 1000 and stored in another `guint` field. For intervals greater than (G_MAXUINT / 1000) seconds, this would overflow; the timeout would fire much sooner than intended. Since GTimeoutSource already keeps track of whether it was created at millisecond or second resolution, always store the passed interval directly. We later convert the interval to microseconds, stored in a gint64, so can move the `* 1000` to there. The eagle-eyed reader might notice that there is no obvious guarantee that the source's expiration time in microseconds won't overflow the gint64, but I don't think this is a new problem. Previously, the monotonic time would have to reach (2 ** 63 - 2 ** 32) microseconds for this overflow to occur; now it would have to reach approximately (2 ** 63 - 2 ** 42) microseconds. Both of these are 292.47 millennia to 5 significant figures. Fixes #1600. --- glib/gmain.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/glib/gmain.c b/glib/gmain.c index 5e849b9ac..347882551 100644 --- a/glib/gmain.c +++ b/glib/gmain.c @@ -315,6 +315,7 @@ struct _GMainLoop struct _GTimeoutSource { GSource source; + /* Measured in seconds if 'seconds' is TRUE, or milliseconds otherwise. */ guint interval; gboolean seconds; }; @@ -4615,8 +4616,6 @@ g_timeout_set_expiration (GTimeoutSource *timeout_source, { gint64 expiration; - expiration = current_time + (guint64) timeout_source->interval * 1000; - if (timeout_source->seconds) { gint64 remainder; @@ -4638,6 +4637,8 @@ g_timeout_set_expiration (GTimeoutSource *timeout_source, timer_perturb = 0; } + expiration = current_time + (guint64) timeout_source->interval * 1000 * 1000; + /* We want the microseconds part of the timeout to land on the * 'timer_perturb' mark, but we need to make sure we don't try to * set the timeout in the past. We do this by ensuring that we @@ -4653,6 +4654,10 @@ g_timeout_set_expiration (GTimeoutSource *timeout_source, expiration -= remainder; expiration += timer_perturb; } + else + { + expiration = current_time + (guint64) timeout_source->interval * 1000; + } g_source_set_ready_time ((GSource *) timeout_source, expiration); } @@ -4735,7 +4740,7 @@ g_timeout_source_new_seconds (guint interval) GSource *source = g_source_new (&g_timeout_funcs, sizeof (GTimeoutSource)); GTimeoutSource *timeout_source = (GTimeoutSource *)source; - timeout_source->interval = 1000 * interval; + timeout_source->interval = interval; timeout_source->seconds = TRUE; g_timeout_set_expiration (timeout_source, g_get_monotonic_time ());