Merge lp:~gero-bare/midori/midori-fix-undef-behavior into lp:midori

Proposed by Gero.Bare
Status: Superseded
Proposed branch: lp:~gero-bare/midori/midori-fix-undef-behavior
Merge into: lp:midori
Diff against target: 34 lines (+14/-3)
1 file modified
extensions/mouse-gestures.c (+14/-3)
To merge this branch: bzr merge lp:~gero-bare/midori/midori-fix-undef-behavior
Reviewer Review Type Date Requested Status
Cris Dywan Needs Fixing
Review via email: mp+240782@code.launchpad.net

This proposal has been superseded by a proposal from 2014-11-22.

Description of the change

Fix two undefined behaviors.

First fix subtraction of two unsigned int. If the second operand is greater than the first, the result wraps around starting at MAX_UINT with is probably way more than intended.

Cast Double to Float. M_PI is a double which means promotes all the operation to double, but because the function fabsf takes a float, the result is truncated to float risking to give an erroneous answer.

To post a comment you must log in.
6827. By Gero.Bare

Add a comentary of warning.

Revision history for this message
Cris Dywan (kalikiana) wrote :

11 + //Remember x1,x2,y1,y2 are guint unsigned integers
12 + //substract a greater number from a lower is undefined
13 + //this guard that.

Good explanation. However please use correct English grammar.

// Remember that x1, x2, y1 and y2 are guint unsigned integers.
// Subtracting a greater number from a lower one is undefined.
// This guards against that.

14 + if ( x1 > x2)

18 + if ( y1 > y2)

Please use no space after the opening brace ie. "if (y1 > y2)".

30 + if (fabsf (angle - dir_angle) < DEVIANCE || fabsf (angle - dir_angle + 2 *(float)(M_PI)) < DEVIANCE)

Please leave a space after the asterisk ie. "+ 2 * (float)(M_PI)".

Sorry to be nit picking. With these stylistic changes it'll be good to go!

review: Needs Fixing
6828. By Gero.Bare

Fix style issues.

Revision history for this message
Gero.Bare (gero-bare) wrote :

Christian I hope We are good now. No problem, you are doing your work.

Revision history for this message
Cris Dywan (kalikiana) wrote :

Please update the comment as well. Again:

// Remember that x1, x2, y1 and y2 are guint unsigned integers.
// Subtracting a greater number from a lower one is undefined.
// This guards against that.

6829. By Gero.Bare

Updated comment.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'extensions/mouse-gestures.c'
--- extensions/mouse-gestures.c 2013-10-30 00:05:40 +0000
+++ extensions/mouse-gestures.c 2014-11-09 15:53:49 +0000
@@ -128,8 +128,19 @@
128static guint128static guint
129dist_sqr (guint x1, guint y1, guint x2, guint y2)129dist_sqr (guint x1, guint y1, guint x2, guint y2)
130{130{
131 guint xdiff = abs(x1 - x2);131 guint xdiff = 0, ydiff = 0;
132 guint ydiff = abs(y1 - y2);132 // Remember that x1, x2, y1 and y2 are guint unsigned integers.
133 // Subtracting a greater number from a lower one is undefined.
134 // This guards against that.
135
136 if (x1 > x2)
137 xdiff = x1 - x2;
138 else
139 xdiff = x2 - x1;
140 if (y1 > y2)
141 ydiff = y1 - y2;
142 else
143 ydiff = y2 - y1;
133 return xdiff * xdiff + ydiff * ydiff;144 return xdiff * xdiff + ydiff * ydiff;
134}145}
135146
@@ -159,7 +170,7 @@
159 return distance < MINLENGTH / 2;170 return distance < MINLENGTH / 2;
160171
161 float dir_angle = get_angle_for_direction (direction);172 float dir_angle = get_angle_for_direction (direction);
162 if (fabsf(angle - dir_angle) < DEVIANCE || fabsf(angle - dir_angle + 2 * M_PI) < DEVIANCE)173 if (fabsf (angle - dir_angle) < DEVIANCE || fabsf (angle - dir_angle + 2 * (float)(M_PI)) < DEVIANCE)
163 return TRUE;174 return TRUE;
164175
165 if(distance < MINLENGTH / 2)176 if(distance < MINLENGTH / 2)

Subscribers

People subscribed via source and target branches

to all changes: