Unity

Merge lp:~brandontschaefer/unity/lp806248-fix-int32-buffer-overflow-4.0 into lp:unity/4.0

  1. lp806248-fix-int32-buffer-overflow-4.0
  2. Merge into 4.0
Proposed by Brandon Schaefer
Status: Rejected
Rejected by: Andrea Azzarone
Proposed branch: lp:~brandontschaefer/unity/lp806248-fix-int32-buffer-overflow-4.0
Merge into: lp:unity/4.0
Diff against target: 288 lines (+69/-26)
6 files modified
plugins/unityshell/src/ElapsedTimeMonitor.cpp (+1/-1)
plugins/unityshell/src/Launcher.cpp (+18/-18)
plugins/unityshell/src/SwitcherView.cpp (+3/-3)
plugins/unityshell/src/TimeUtil.h (+9/-4)
tests/CMakeLists.txt (+1/-0)
tests/test_time_util.cpp (+37/-0)
To merge this branch: bzr merge lp:~brandontschaefer/unity/lp806248-fix-int32-buffer-overflow-4.0
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Approve
Review via email: mp+137684@code.launchpad.net

Commit message

Change TimeUtil to use int64, so the buffer wont overflow within our lifetimes.

Description of the change

=== Problem ===
After 24 days the int32 used in TimeUtil would overflow causing problems.

=== Fix ===
Change TimeUtil to use int64, so it wont overflow within our lifetimes.

=== Test ===
Unit test showing an int32 buffer overflow, and asserting it doesn't anymore.

To post a comment you must log in.
lp:~brandontschaefer/unity/lp806248-fix-int32-buffer-overflow-4.0 updated
1740. By Brandon Schaefer

* Fix header

Revision history for this message
Andrea Azzarone (azzar1) :
review: Approve

Unmerged revisions

1740. By Brandon Schaefer

* Fix header

1739. By Brandon Schaefer

* Fix int32 buffer overflow in TimeUtil, now uses int64

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/ElapsedTimeMonitor.cpp'
2--- plugins/unityshell/src/ElapsedTimeMonitor.cpp 2011-08-30 19:46:25 +0000
3+++ plugins/unityshell/src/ElapsedTimeMonitor.cpp 2012-12-05 17:50:29 +0000
4@@ -37,7 +37,7 @@
5 {
6 struct timespec current;
7 clock_gettime(CLOCK_MONOTONIC, &current);
8- int diff = TimeUtil::TimeDelta(&current, &_start);
9+ DeltaTime diff = TimeUtil::TimeDelta(&current, &_start);
10
11 g_variant_builder_add(builder, "{sv}", "elapsed-time", g_variant_new_uint32(diff));
12 }
13
14=== modified file 'plugins/unityshell/src/Launcher.cpp'
15--- plugins/unityshell/src/Launcher.cpp 2011-09-29 03:45:59 +0000
16+++ plugins/unityshell/src/Launcher.cpp 2012-12-05 17:50:29 +0000
17@@ -89,7 +89,7 @@
18
19 NUX_IMPLEMENT_OBJECT_TYPE(Launcher);
20
21-void SetTimeBack(struct timespec* timeref, int remove)
22+void SetTimeBack(struct timespec* timeref, DeltaTime remove)
23 {
24 timeref->tv_sec -= remove / 1000;
25 remove = remove % 1000;
26@@ -735,18 +735,18 @@
27 return false;
28 }
29
30-void Launcher::SetTimeStruct(struct timespec* timer, struct timespec* sister, int sister_relation)
31+void Launcher::SetTimeStruct(struct timespec* timer, struct timespec* sister, DeltaTime sister_relation)
32 {
33 struct timespec current;
34 clock_gettime(CLOCK_MONOTONIC, &current);
35
36 if (sister)
37 {
38- int diff = unity::TimeUtil::TimeDelta(&current, sister);
39+ DeltaTime diff = unity::TimeUtil::TimeDelta(&current, sister);
40
41 if (diff < sister_relation)
42 {
43- int remove = sister_relation - diff;
44+ DeltaTime remove = sister_relation - diff;
45 SetTimeBack(&current, remove);
46 }
47 }
48@@ -777,13 +777,13 @@
49 if (icon->GetQuirk(LauncherIcon::QUIRK_VISIBLE))
50 {
51 struct timespec icon_visible_time = icon->GetQuirkTime(LauncherIcon::QUIRK_VISIBLE);
52- int enter_ms = unity::TimeUtil::TimeDelta(&current, &icon_visible_time);
53+ DeltaTime enter_ms = unity::TimeUtil::TimeDelta(&current, &icon_visible_time);
54 return CLAMP((float) enter_ms / (float) ANIM_DURATION_SHORT, 0.0f, 1.0f);
55 }
56 else
57 {
58 struct timespec icon_hide_time = icon->GetQuirkTime(LauncherIcon::QUIRK_VISIBLE);
59- int hide_ms = unity::TimeUtil::TimeDelta(&current, &icon_hide_time);
60+ DeltaTime hide_ms = unity::TimeUtil::TimeDelta(&current, &icon_hide_time);
61 return 1.0f - CLAMP((float) hide_ms / (float) ANIM_DURATION_SHORT, 0.0f, 1.0f);
62 }
63 }
64@@ -817,7 +817,7 @@
65 float Launcher::IconPresentProgress(LauncherIcon* icon, struct timespec const& current)
66 {
67 struct timespec icon_present_time = icon->GetQuirkTime(LauncherIcon::QUIRK_PRESENTED);
68- int ms = unity::TimeUtil::TimeDelta(&current, &icon_present_time);
69+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &icon_present_time);
70 float result = CLAMP((float) ms / (float) ANIM_DURATION, 0.0f, 1.0f);
71
72 if (icon->GetQuirk(LauncherIcon::QUIRK_PRESENTED))
73@@ -829,7 +829,7 @@
74 float Launcher::IconUrgentProgress(LauncherIcon* icon, struct timespec const& current)
75 {
76 struct timespec urgent_time = icon->GetQuirkTime(LauncherIcon::QUIRK_URGENT);
77- int urgent_ms = unity::TimeUtil::TimeDelta(&current, &urgent_time);
78+ DeltaTime urgent_ms = unity::TimeUtil::TimeDelta(&current, &urgent_time);
79 float result;
80
81 if (_urgent_animation == URGENT_ANIMATION_WIGGLE)
82@@ -846,7 +846,7 @@
83 float Launcher::IconDropDimValue(LauncherIcon* icon, struct timespec const& current)
84 {
85 struct timespec dim_time = icon->GetQuirkTime(LauncherIcon::QUIRK_DROP_DIM);
86- int dim_ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
87+ DeltaTime dim_ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
88 float result = CLAMP((float) dim_ms / (float) ANIM_DURATION, 0.0f, 1.0f);
89
90 if (icon->GetQuirk(LauncherIcon::QUIRK_DROP_DIM))
91@@ -858,7 +858,7 @@
92 float Launcher::IconDesatValue(LauncherIcon* icon, struct timespec const& current)
93 {
94 struct timespec dim_time = icon->GetQuirkTime(LauncherIcon::QUIRK_DESAT);
95- int ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
96+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
97 float result = CLAMP((float) ms / (float) ANIM_DURATION_SHORT_SHORT, 0.0f, 1.0f);
98
99 if (icon->GetQuirk(LauncherIcon::QUIRK_DESAT))
100@@ -870,14 +870,14 @@
101 float Launcher::IconShimmerProgress(LauncherIcon* icon, struct timespec const& current)
102 {
103 struct timespec shimmer_time = icon->GetQuirkTime(LauncherIcon::QUIRK_SHIMMER);
104- int shimmer_ms = unity::TimeUtil::TimeDelta(&current, &shimmer_time);
105+ DeltaTime shimmer_ms = unity::TimeUtil::TimeDelta(&current, &shimmer_time);
106 return CLAMP((float) shimmer_ms / (float) ANIM_DURATION_LONG, 0.0f, 1.0f);
107 }
108
109 float Launcher::IconCenterTransitionProgress(LauncherIcon* icon, struct timespec const& current)
110 {
111 struct timespec save_time = icon->GetQuirkTime(LauncherIcon::QUIRK_CENTER_SAVED);
112- int save_ms = unity::TimeUtil::TimeDelta(&current, &save_time);
113+ DeltaTime save_ms = unity::TimeUtil::TimeDelta(&current, &save_time);
114 return CLAMP((float) save_ms / (float) ANIM_DURATION, 0.0f, 1.0f);
115 }
116
117@@ -893,7 +893,7 @@
118 float Launcher::IconPulseOnceValue(LauncherIcon *icon, struct timespec const &current)
119 {
120 struct timespec pulse_time = icon->GetQuirkTime(LauncherIcon::QUIRK_PULSE_ONCE);
121- int pulse_ms = unity::TimeUtil::TimeDelta(&current, &pulse_time);
122+ DeltaTime pulse_ms = unity::TimeUtil::TimeDelta(&current, &pulse_time);
123 double pulse_progress = (double) CLAMP((float) pulse_ms / (ANIM_DURATION_LONG * PULSE_BLINK_LAMBDA * 2), 0.0f, 1.0f);
124
125 if (pulse_progress == 1.0f)
126@@ -914,7 +914,7 @@
127 float Launcher::IconStartingBlinkValue(LauncherIcon* icon, struct timespec const& current)
128 {
129 struct timespec starting_time = icon->GetQuirkTime(LauncherIcon::QUIRK_STARTING);
130- int starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
131+ DeltaTime starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
132 double starting_progress = (double) CLAMP((float) starting_ms / (float)(ANIM_DURATION_LONG * STARTING_BLINK_LAMBDA), 0.0f, 1.0f);
133 double val = IsBackLightModeToggles() ? 3.0f : 4.0f;
134 return 0.5f + (float)(std::cos(M_PI * val * starting_progress)) * 0.5f;
135@@ -923,7 +923,7 @@
136 float Launcher::IconStartingPulseValue(LauncherIcon* icon, struct timespec const& current)
137 {
138 struct timespec starting_time = icon->GetQuirkTime(LauncherIcon::QUIRK_STARTING);
139- int starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
140+ DeltaTime starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
141 double starting_progress = (double) CLAMP((float) starting_ms / (float)(ANIM_DURATION_LONG * MAX_STARTING_BLINKS * STARTING_BLINK_LAMBDA * 2), 0.0f, 1.0f);
142
143 if (starting_progress == 1.0f && !icon->GetQuirk(LauncherIcon::QUIRK_RUNNING))
144@@ -940,7 +940,7 @@
145 float result = 0.0f;
146
147 struct timespec running_time = icon->GetQuirkTime(LauncherIcon::QUIRK_RUNNING);
148- int running_ms = unity::TimeUtil::TimeDelta(&current, &running_time);
149+ DeltaTime running_ms = unity::TimeUtil::TimeDelta(&current, &running_time);
150 float running_progress = CLAMP((float) running_ms / (float) ANIM_DURATION_SHORT, 0.0f, 1.0f);
151
152 if (!icon->GetQuirk(LauncherIcon::QUIRK_RUNNING))
153@@ -1005,7 +1005,7 @@
154 float Launcher::IconProgressBias(LauncherIcon* icon, struct timespec const& current)
155 {
156 struct timespec icon_progress_time = icon->GetQuirkTime(LauncherIcon::QUIRK_PROGRESS);
157- int ms = unity::TimeUtil::TimeDelta(&current, &icon_progress_time);
158+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &icon_progress_time);
159 float result = CLAMP((float) ms / (float) ANIM_DURATION, 0.0f, 1.0f);
160
161 if (icon->GetQuirk(LauncherIcon::QUIRK_PROGRESS))
162@@ -1423,7 +1423,7 @@
163 UBUS_PLACE_ENTRY_ACTIVATE_REQUEST,
164 g_variant_new("(sus)", "home.lens", 0, ""));
165
166- remaining_time_before_hide = BEFORE_HIDE_LAUNCHER_ON_SUPER_DURATION - CLAMP((int)(unity::TimeUtil::TimeDelta(&current, &_times[TIME_SUPER_PRESSED])), 0, BEFORE_HIDE_LAUNCHER_ON_SUPER_DURATION);
167+ remaining_time_before_hide = BEFORE_HIDE_LAUNCHER_ON_SUPER_DURATION - CLAMP((DeltaTime)(unity::TimeUtil::TimeDelta(&current, &_times[TIME_SUPER_PRESSED])), 0, BEFORE_HIDE_LAUNCHER_ON_SUPER_DURATION);
168
169 if (_super_hide_launcher_handle > 0)
170 g_source_remove(_super_hide_launcher_handle);
171
172=== modified file 'plugins/unityshell/src/SwitcherView.cpp'
173--- plugins/unityshell/src/SwitcherView.cpp 2011-09-27 22:53:57 +0000
174+++ plugins/unityshell/src/SwitcherView.cpp 2012-12-05 17:50:29 +0000
175@@ -249,7 +249,7 @@
176 {
177 std::vector<Window> xids = model_->DetailXids ();
178
179- int ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
180+ DeltaTime ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
181 float progress = MIN (1.0f, (float) ms_since_change / (float) animation_length());
182
183 for (Window window : xids)
184@@ -506,7 +506,7 @@
185 ++i;
186 }
187
188- int ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
189+ DeltaTime ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
190 if (saved_args_.size () == results.size () && ms_since_change < animation_length)
191 {
192 float progress = (float) ms_since_change / (float) animation_length();
193@@ -780,7 +780,7 @@
194 text_view_->SetBaseY(background_geo.y + background_geo.height - 45);
195 text_view_->Draw(GfxContext, force_draw);
196
197- int ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
198+ DeltaTime ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
199
200 if (ms_since_change < animation_length && redraw_handle_ == 0)
201 redraw_handle_ = g_timeout_add(0, &SwitcherView::OnDrawTimeout, this);
202
203=== modified file 'plugins/unityshell/src/TimeUtil.h'
204--- plugins/unityshell/src/TimeUtil.h 2011-08-30 19:00:32 +0000
205+++ plugins/unityshell/src/TimeUtil.h 2012-12-05 17:50:29 +0000
206@@ -19,19 +19,24 @@
207 */
208
209 #include <time.h>
210+#include <cstdint>
211+
212+typedef int64_t DeltaTime
213
214 namespace unity {
215
216 class TimeUtil
217 {
218 public:
219-static int TimeDelta (struct timespec const* x, struct timespec const* y);
220+static DeltaTime TimeDelta (struct timespec const* x, struct timespec const* y);
221 };
222
223-inline
224-int TimeUtil::TimeDelta (struct timespec const* x, struct timespec const* y)
225+inline
226+DeltaTime TimeUtil::TimeDelta (struct timespec const* x, struct timespec const* y)
227 {
228- return ((x->tv_sec - y->tv_sec) * 1000) + ((x->tv_nsec - y->tv_nsec) / 1000000);
229+ DeltaTime d_sec = (x->tv_sec - y->tv_sec);
230+ DeltaTime d_nsec = (x->tv_nsec - y->tv_nsec);
231+ return (d_sec * 1000) + (d_nsec / 1000000);
232 }
233
234 }
235
236=== modified file 'tests/CMakeLists.txt'
237--- tests/CMakeLists.txt 2012-01-06 13:02:00 +0000
238+++ tests/CMakeLists.txt 2012-12-05 17:50:29 +0000
239@@ -541,6 +541,7 @@
240 test_main.cpp
241 test_model.cpp
242 test_texture_cache.cpp
243+ test_time_util.cpp
244 test_utils.h
245 test_ratings_filter.cpp
246 test_results.cpp
247
248=== added file 'tests/test_time_util.cpp'
249--- tests/test_time_util.cpp 1970-01-01 00:00:00 +0000
250+++ tests/test_time_util.cpp 2012-12-05 17:50:29 +0000
251@@ -0,0 +1,37 @@
252+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
253+/*
254+* Copyright (C) 2012 Canonical Ltd
255+*
256+* This program is free software: you can redistribute it and/or modify
257+* it under the terms of the GNU General Public License version 3 as
258+* published by the Free Software Foundation.
259+*
260+* This program is distributed in the hope that it will be useful,
261+* but WITHOUT ANY WARRANTY; without even the implied warranty of
262+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
263+* GNU General Public License for more details.
264+*
265+* You should have received a copy of the GNU General Public License
266+* along with this program. If not, see <http://www.gnu.org/licenses/>.
267+*
268+* Authored by: Brandon Schaefer <brandon.schaefer@canonical.com>
269+*
270+*/
271+
272+
273+#include <cinttypes>
274+#include <gtest/gtest.h>
275+#include "TimeUtil.h"
276+
277+using namespace testing;
278+
279+TEST(TestTimeUtil, Testin32BufferOverflow)
280+{
281+ struct timespec start, end;
282+ unity::TimeUtil::SetTimeStruct(&start);
283+ unity::TimeUtil::SetTimeStruct(&end);
284+
285+ end.tv_sec = start.tv_sec + INT32_MAX;
286+
287+ EXPECT_GT(unity::TimeUtil::TimeDelta(&end, &start), 0);
288+}

Subscribers

People subscribed via source and target branches

to all changes: