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 762438 - [PATCH] format_utc_offset returning bogus results
[PATCH] format_utc_offset returning bogus results
Status: RESOLVED FIXED
Product: gnome-calendar
Classification: Applications
Component: Backend
3.19.x
Other Linux
: Normal normal
: 3.26
Assigned To: GNOME Calendar maintainers
GNOME Calendar maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-22 11:50 UTC by Marinus Schraal
Modified: 2017-04-17 18:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix format_utc_offset (1.68 KB, patch)
2016-02-22 11:50 UTC, Marinus Schraal
none Details | Review
fix format_utc_offset (1.84 KB, patch)
2016-02-22 19:06 UTC, Marinus Schraal
none Details | Review
fix format_utc_offset (1.84 KB, patch)
2016-02-23 23:48 UTC, Marinus Schraal
committed Details | Review

Description Marinus Schraal 2016-02-22 11:50:31 UTC
Created attachment 321824 [details] [review]
fix format_utc_offset

While looking into some offset issues I noticed format_utc_offset is called in two different ways. One is by icaltimezone_get_utc_offset which returns seconds (and in my case is always 0 afaics) and the other is g_date_time_get_utc_offset which returns a GTimeSpan (gint64).

The latter returned bogus results with offset being defined as gint. Adapted the function to handle both possibilities.
Comment 1 Georges Basile Stavracas Neto 2016-02-22 16:39:18 UTC
Review of attachment 321824 [details] [review]:

It makes sense, only small nitpicks.

::: src/gcal-utils.c
@@ +804,3 @@
   }
 
+  if (offset >= 1000000) {

Please, add a comment before explaining why this check is needed here.

@@ +806,3 @@
+  if (offset >= 1000000) {
+    offset = offset / 1000000;
+  }

Don't add curly braces here.
Comment 2 Marinus Schraal 2016-02-22 19:06:39 UTC
Created attachment 321885 [details] [review]
fix format_utc_offset
Comment 3 Georges Basile Stavracas Neto 2016-02-23 23:16:40 UTC
Review of attachment 321885 [details] [review]:

::: src/gcal-utils.c
@@ +806,3 @@
+  /* offset can be seconds or microseconds */
+  if (offset >= 1000000)
+    offset = offset / 1000000;

Now that I'm looking... does this code works with negative timezones?
Comment 4 Marinus Schraal 2016-02-23 23:48:09 UTC
Created attachment 322189 [details] [review]
fix format_utc_offset

Definitely, just before that the code makes sure the offset is positive: https://git.gnome.org/browse/gnome-calendar/tree/src/gcal-utils.c#n801 .

Patch updated only commit msg formatting.
Comment 5 Georges Basile Stavracas Neto 2016-02-24 01:09:39 UTC
Cool. I wrote that code, and forgot about it :)

Anyway, thanks for working on that.