On 02/08/2012 04:41 PM, Chase Douglas wrote:
On 02/08/2012 11:30 PM, Peter Hutterer wrote:
On Wed, Feb 08, 2012 at 09:10:51PM +0100, Chase Douglas wrote:
On 02/08/2012 03:54 PM, Peter Hutterer wrote:
On Tue, Feb 07, 2012 at 01:07:08PM -0800, Chase Douglas wrote:
Signed-off-by: Chase Douglas<[email protected]>
---
  src/synaptics.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/synaptics.c b/src/synaptics.c
index a4c1e5a..106b5ee 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -1791,7 +1791,7 @@ HandleTapProcessing(SynapticsPrivate *priv, struct 
SynapticsHwState *hw,
  }

  #define HIST(a) (priv->move_hist[((priv->hist_index - (a) + 
SYNAPTICS_MOVE_HISTORY) % SYNAPTICS_MOVE_HISTORY)])
-#define HIST_DELTA(a, b, e) ((HIST((a)).e) - (HIST((b)).e))
+#define HIST_DELTA(a, b, e) ((int)((HIST((a)).e) - (HIST((b)).e)))

this only casts the result to a signed int, the actual computation is still
undefined for b>  a. With the revert, HIST_DELTA is only used in one place
so we could either do it there or just write a helper function like this

Why is it undefined? We can have a negative time difference, which is
what the calculation was assuming, IIUC.

yeah, but IIRC (unsigned int)a - (unsigned int)b is undefined for b>  a.

I thought in C that was still defined. I think it's used in many places,
including TCP timestamp wrap-around in the kernel. Maybe what I'm
remembering is slightly different? I must admit I don't know where or
how to find a definitive answer though.

I think whether it's defined or not, if b > a when using this macro to calculate times it's a bug - either the parameters have been passed backwards, or the history contains non-monotonic timestamps.

I suppose the cast could also be problematic if the result proved too large to fit into an int, but that would seem to indicate an invalid motion history as well.

There's only one usage of this macro left after reverting my motion estimator anyway. Perhaps it should just die (and I'll resurrect it as an error checking function later...)
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to