Unity

Merge lp:~brandontschaefer/unity/lp806248-in32-buffer-overflow into lp:unity

  1. lp806248-in32-buffer-overflow
  2. Merge into trunk
Proposed by Brandon Schaefer
Status: Merged
Approved by: Brandon Schaefer
Approved revision: no longer in the source branch.
Merged at revision: 2944
Proposed branch: lp:~brandontschaefer/unity/lp806248-in32-buffer-overflow
Merge into: lp:unity
Diff against target: 322 lines (+72/-28)
8 files modified
dash/previews/PreviewContainer.cpp (+1/-1)
launcher/Launcher.cpp (+14/-14)
launcher/SwitcherView.cpp (+3/-3)
plugins/unityshell/src/ElapsedTimeMonitor.cpp (+1/-1)
tests/CMakeLists.txt (+1/-0)
tests/test_glib_source.cpp (+3/-3)
tests/test_time_util.cpp (+38/-0)
unity-shared/TimeUtil.h (+11/-6)
To merge this branch: bzr merge lp:~brandontschaefer/unity/lp806248-in32-buffer-overflow
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Stephen M. Webb (community) Approve
Review via email: mp+137271@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.
Revision history for this message
Stephen M. Webb (bregma) wrote :

I don't think int64 is a good descriptive typedef for a time delta value. Could you maybe come up with a better name?

Revision history for this message
Stephen M. Webb (bregma) wrote :

OK

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/188/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/188/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/188/console

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'dash/previews/PreviewContainer.cpp'
2--- dash/previews/PreviewContainer.cpp 2012-10-29 09:34:54 +0000
3+++ dash/previews/PreviewContainer.cpp 2012-11-30 21:18:22 +0000
4@@ -571,7 +571,7 @@
5
6 float PreviewContainer::GetSwipeAnimationProgress(struct timespec const& current) const
7 {
8- int time_delta = TimeUtil::TimeDelta(&current, &last_progress_time_);
9+ DeltaTime time_delta = TimeUtil::TimeDelta(&current, &last_progress_time_);
10 float progress = content_layout_->GetAnimationProgress() + (navigation_progress_speed_ * time_delta);
11
12 return progress;
13
14=== modified file 'launcher/Launcher.cpp'
15--- launcher/Launcher.cpp 2012-11-26 19:24:33 +0000
16+++ launcher/Launcher.cpp 2012-11-30 21:18:22 +0000
17@@ -493,13 +493,13 @@
18 if (icon->IsVisible())
19 {
20 struct timespec icon_visible_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::VISIBLE);
21- int enter_ms = unity::TimeUtil::TimeDelta(&current, &icon_visible_time);
22+ DeltaTime enter_ms = unity::TimeUtil::TimeDelta(&current, &icon_visible_time);
23 return CLAMP((float) enter_ms / (float) ANIM_DURATION_SHORT, 0.0f, 1.0f);
24 }
25 else
26 {
27 struct timespec icon_hide_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::VISIBLE);
28- int hide_ms = unity::TimeUtil::TimeDelta(&current, &icon_hide_time);
29+ DeltaTime hide_ms = unity::TimeUtil::TimeDelta(&current, &icon_hide_time);
30 return 1.0f - CLAMP((float) hide_ms / (float) ANIM_DURATION_SHORT, 0.0f, 1.0f);
31 }
32 }
33@@ -531,7 +531,7 @@
34 float Launcher::IconPresentProgress(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
35 {
36 struct timespec icon_present_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::PRESENTED);
37- int ms = unity::TimeUtil::TimeDelta(&current, &icon_present_time);
38+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &icon_present_time);
39 float result = CLAMP((float) ms / (float) ANIM_DURATION, 0.0f, 1.0f);
40
41 if (icon->GetQuirk(AbstractLauncherIcon::Quirk::PRESENTED))
42@@ -543,7 +543,7 @@
43 float Launcher::IconUnfoldProgress(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
44 {
45 struct timespec icon_unfold_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::UNFOLDED);
46- int ms = unity::TimeUtil::TimeDelta(&current, &icon_unfold_time);
47+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &icon_unfold_time);
48 float result = CLAMP((float) ms / (float) ANIM_DURATION, 0.0f, 1.0f);
49
50 if (icon->GetQuirk(AbstractLauncherIcon::Quirk::UNFOLDED))
51@@ -555,7 +555,7 @@
52 float Launcher::IconUrgentProgress(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
53 {
54 struct timespec urgent_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::URGENT);
55- int urgent_ms = unity::TimeUtil::TimeDelta(&current, &urgent_time);
56+ DeltaTime urgent_ms = unity::TimeUtil::TimeDelta(&current, &urgent_time);
57 float result;
58
59 if (options()->urgent_animation() == URGENT_ANIMATION_WIGGLE)
60@@ -572,7 +572,7 @@
61 float Launcher::IconDropDimValue(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
62 {
63 struct timespec dim_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::DROP_DIM);
64- int dim_ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
65+ DeltaTime dim_ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
66 float result = CLAMP((float) dim_ms / (float) ANIM_DURATION, 0.0f, 1.0f);
67
68 if (icon->GetQuirk(AbstractLauncherIcon::Quirk::DROP_DIM))
69@@ -584,7 +584,7 @@
70 float Launcher::IconDesatValue(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
71 {
72 struct timespec dim_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::DESAT);
73- int ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
74+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &dim_time);
75 float result = CLAMP((float) ms / (float) ANIM_DURATION_SHORT_SHORT, 0.0f, 1.0f);
76
77 if (icon->GetQuirk(AbstractLauncherIcon::Quirk::DESAT))
78@@ -596,14 +596,14 @@
79 float Launcher::IconShimmerProgress(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
80 {
81 struct timespec shimmer_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::SHIMMER);
82- int shimmer_ms = unity::TimeUtil::TimeDelta(&current, &shimmer_time);
83+ DeltaTime shimmer_ms = unity::TimeUtil::TimeDelta(&current, &shimmer_time);
84 return CLAMP((float) shimmer_ms / (float) ANIM_DURATION_LONG, 0.0f, 1.0f);
85 }
86
87 float Launcher::IconCenterTransitionProgress(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
88 {
89 struct timespec save_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::CENTER_SAVED);
90- int save_ms = unity::TimeUtil::TimeDelta(&current, &save_time);
91+ DeltaTime save_ms = unity::TimeUtil::TimeDelta(&current, &save_time);
92 return CLAMP((float) save_ms / (float) ANIM_DURATION, 0.0f, 1.0f);
93 }
94
95@@ -619,7 +619,7 @@
96 float Launcher::IconPulseOnceValue(AbstractLauncherIcon::Ptr const& icon, struct timespec const &current) const
97 {
98 struct timespec pulse_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::PULSE_ONCE);
99- int pulse_ms = unity::TimeUtil::TimeDelta(&current, &pulse_time);
100+ DeltaTime pulse_ms = unity::TimeUtil::TimeDelta(&current, &pulse_time);
101 double pulse_progress = (double) CLAMP((float) pulse_ms / (ANIM_DURATION_LONG * PULSE_BLINK_LAMBDA * 2), 0.0f, 1.0f);
102
103 if (pulse_progress == 1.0f)
104@@ -646,7 +646,7 @@
105 return 1.0f;
106
107 struct timespec starting_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::STARTING);
108- int starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
109+ DeltaTime starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
110 double starting_progress = (double) CLAMP((float) starting_ms / (float)(ANIM_DURATION_LONG * STARTING_BLINK_LAMBDA), 0.0f, 1.0f);
111 double val = IsBackLightModeToggles() ? 3.0f : 4.0f;
112 return 1.0f-(0.5f + (float)(std::cos(M_PI * val * starting_progress)) * 0.5f);
113@@ -661,7 +661,7 @@
114 return 1.0f;
115
116 struct timespec starting_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::STARTING);
117- int starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
118+ DeltaTime starting_ms = unity::TimeUtil::TimeDelta(&current, &starting_time);
119 double starting_progress = (double) CLAMP((float) starting_ms / (float)(ANIM_DURATION_LONG * MAX_STARTING_BLINKS * STARTING_BLINK_LAMBDA * 2), 0.0f, 1.0f);
120
121 if (starting_progress == 1.0f && !icon->GetQuirk(AbstractLauncherIcon::Quirk::RUNNING))
122@@ -678,7 +678,7 @@
123 float result = 0.0f;
124
125 struct timespec running_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::RUNNING);
126- int running_ms = unity::TimeUtil::TimeDelta(&current, &running_time);
127+ DeltaTime running_ms = unity::TimeUtil::TimeDelta(&current, &running_time);
128 float running_progress = CLAMP((float) running_ms / (float) ANIM_DURATION_SHORT, 0.0f, 1.0f);
129
130 if (!icon->GetQuirk(AbstractLauncherIcon::Quirk::RUNNING))
131@@ -743,7 +743,7 @@
132 float Launcher::IconProgressBias(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current) const
133 {
134 struct timespec icon_progress_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::PROGRESS);
135- int ms = unity::TimeUtil::TimeDelta(&current, &icon_progress_time);
136+ DeltaTime ms = unity::TimeUtil::TimeDelta(&current, &icon_progress_time);
137 float result = CLAMP((float) ms / (float) ANIM_DURATION, 0.0f, 1.0f);
138
139 if (icon->GetQuirk(AbstractLauncherIcon::Quirk::PROGRESS))
140
141=== modified file 'launcher/SwitcherView.cpp'
142--- launcher/SwitcherView.cpp 2012-11-14 02:21:36 +0000
143+++ launcher/SwitcherView.cpp 2012-11-30 21:18:22 +0000
144@@ -223,7 +223,7 @@
145 {
146 std::vector<Window> const& xids = model_->DetailXids();
147
148- int ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
149+ DeltaTime ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
150 float progress = std::min<float>(1.0f, ms_since_change / static_cast<float>(animation_length()));
151
152 for (Window window : xids)
153@@ -485,7 +485,7 @@
154 ++i;
155 }
156
157- int ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
158+ DeltaTime ms_since_change = TimeUtil::TimeDelta(&current, &save_time_);
159 if (saved_args_.size () == results.size () && ms_since_change < animation_length)
160 {
161 float progress = (float) ms_since_change / (float) animation_length();
162@@ -585,7 +585,7 @@
163 text_view_->Draw(GfxContext, force_draw);
164 }
165
166- int ms_since_change = TimeUtil::TimeDelta(&current_, &save_time_);
167+ DeltaTime ms_since_change = TimeUtil::TimeDelta(&current_, &save_time_);
168
169 if (ms_since_change < animation_length && !redraw_idle_)
170 {
171
172=== modified file 'plugins/unityshell/src/ElapsedTimeMonitor.cpp'
173--- plugins/unityshell/src/ElapsedTimeMonitor.cpp 2012-03-14 06:24:18 +0000
174+++ plugins/unityshell/src/ElapsedTimeMonitor.cpp 2012-11-30 21:18:22 +0000
175@@ -39,7 +39,7 @@
176 {
177 struct timespec current;
178 clock_gettime(CLOCK_MONOTONIC, &current);
179- int diff = TimeUtil::TimeDelta(&current, &_start);
180+ DeltaTime diff = TimeUtil::TimeDelta(&current, &_start);
181
182 variant::BuilderWrapper(builder)
183 .add("elapsed-time", diff);
184
185=== modified file 'tests/CMakeLists.txt'
186--- tests/CMakeLists.txt 2012-11-28 03:44:41 +0000
187+++ tests/CMakeLists.txt 2012-11-30 21:18:22 +0000
188@@ -140,6 +140,7 @@
189 test_layout_system.cpp
190 test_model_iterator.cpp
191 test_previews.cpp
192+ test_time_util.cpp
193 test_ubus.cpp
194 test_unityshell_private.cpp
195 ${CMAKE_CURRENT_BINARY_DIR}/test_glib_signals_utils_marshal.cpp
196
197=== modified file 'tests/test_glib_source.cpp'
198--- tests/test_glib_source.cpp 2012-11-28 12:24:28 +0000
199+++ tests/test_glib_source.cpp 2012-11-30 21:18:22 +0000
200@@ -328,7 +328,7 @@
201 EXPECT_FALSE(timeout.IsRunning());
202 EXPECT_TRUE(callback_called);
203 EXPECT_EQ(callback_call_count, 1);
204- int time_delta = unity::TimeUtil::TimeDelta(&post, &pre);
205+ DeltaTime time_delta = unity::TimeUtil::TimeDelta(&post, &pre);
206 EXPECT_GE(time_delta, 500);
207 EXPECT_LT(time_delta, 2000);
208 }
209@@ -351,7 +351,7 @@
210 EXPECT_TRUE(callback_called);
211 EXPECT_GE(callback_call_count, 3);
212 EXPECT_LE(callback_call_count, 4);
213- int time_delta = unity::TimeUtil::TimeDelta(&post, &pre);
214+ DeltaTime time_delta = unity::TimeUtil::TimeDelta(&post, &pre);
215 EXPECT_GE(time_delta, 3500);
216 EXPECT_LT(time_delta, 5000);
217 }
218@@ -415,7 +415,7 @@
219
220 EXPECT_TRUE(callback_called);
221 EXPECT_GT(callback_call_count, 1);
222- int time_delta = unity::TimeUtil::TimeDelta(&post, &pre);
223+ DeltaTime time_delta = unity::TimeUtil::TimeDelta(&post, &pre);
224 EXPECT_GE(time_delta, 100);
225 EXPECT_LT(time_delta, 200);
226 }
227
228=== added file 'tests/test_time_util.cpp'
229--- tests/test_time_util.cpp 1970-01-01 00:00:00 +0000
230+++ tests/test_time_util.cpp 2012-11-30 21:18:22 +0000
231@@ -0,0 +1,38 @@
232+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
233+/*
234+* Copyright (C) 2012 Canonical Ltd
235+*
236+* This program is free software: you can redistribute it and/or modify
237+* it under the terms of the GNU General Public License version 3 as
238+* published by the Free Software Foundation.
239+*
240+* This program is distributed in the hope that it will be useful,
241+* but WITHOUT ANY WARRANTY; without even the implied warranty of
242+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
243+* GNU General Public License for more details.
244+*
245+* You should have received a copy of the GNU General Public License
246+* along with this program. If not, see <http://www.gnu.org/licenses/>.
247+*
248+* Authored by: Brandon Schaefer <brandon.schaefer@canonical.com>
249+*
250+*/
251+
252+
253+#include <limits>
254+#include <gtest/gtest.h>
255+#include <unity-shared/TimeUtil.h>
256+
257+using namespace testing;
258+using namespace std;
259+
260+TEST(TestTimeUtil, Testin32BufferOverflow)
261+{
262+ struct timespec start, end;
263+ unity::TimeUtil::SetTimeStruct(&start, &end);
264+
265+ end.tv_sec = 20736000 - start.tv_sec;
266+
267+ EXPECT_GT(unity::TimeUtil::TimeDelta(&end, &start), 0);
268+}
269+
270
271=== modified file 'unity-shared/TimeUtil.h'
272--- unity-shared/TimeUtil.h 2012-05-06 23:48:38 +0000
273+++ unity-shared/TimeUtil.h 2012-11-30 21:18:22 +0000
274@@ -19,29 +19,34 @@
275 */
276
277 #include <time.h>
278+#include <cstdint>
279+
280+typedef int64_t DeltaTime;
281
282 namespace unity {
283
284 class TimeUtil
285 {
286 public:
287- static int TimeDelta (struct timespec const* x, struct timespec const* y)
288+ static DeltaTime TimeDelta (struct timespec const* x, struct timespec const* y)
289 {
290- return ((x->tv_sec - y->tv_sec) * 1000) + ((x->tv_nsec - y->tv_nsec) / 1000000);
291+ DeltaTime d_sec = ((x->tv_sec - y->tv_sec));
292+ DeltaTime d_nsec = ((x->tv_nsec - y->tv_nsec));
293+ return (d_sec * 1000) + (d_nsec / 1000000);
294 }
295
296- static void SetTimeStruct(struct timespec* timer, struct timespec* sister = 0, int sister_relation = 0)
297+ static void SetTimeStruct(struct timespec* timer, struct timespec* sister = 0, DeltaTime sister_relation = 0)
298 {
299 struct timespec current;
300 clock_gettime(CLOCK_MONOTONIC, &current);
301
302 if (sister)
303 {
304- int diff = TimeDelta(&current, sister);
305+ DeltaTime diff = TimeDelta(&current, sister);
306
307 if (diff < sister_relation)
308 {
309- int remove = sister_relation - diff;
310+ DeltaTime remove = sister_relation - diff;
311 SetTimeBack(&current, remove);
312 }
313 }
314@@ -50,7 +55,7 @@
315 timer->tv_nsec = current.tv_nsec;
316 }
317
318- static void SetTimeBack(struct timespec* timeref, int remove)
319+ static void SetTimeBack(struct timespec* timeref, DeltaTime remove)
320 {
321 timeref->tv_sec -= remove / 1000;
322 remove = remove % 1000;