This change of mine is obvious since InExt and OutExt are DWORD, thus unsigned, and thus always greater or equal zero so my patch should be fine in any case.
However, looking at the else-part of the if-statement, I have some doubts this is working as designed, and in general the abs() invocations are bogus.
Any expert who could help with a more extensive patch?
Gerald
ChangeLog: Simplify condition in ScaleForContext().
Index: dlls/wintab32/context.c =================================================================== RCS file: /home/wine/wine/dlls/wintab32/context.c,v retrieving revision 1.22 diff -u -3 -p -r1.22 context.c --- dlls/wintab32/context.c 21 Sep 2007 12:24:49 -0000 1.22 +++ dlls/wintab32/context.c 30 Nov 2007 23:53:47 -0000 @@ -176,7 +176,7 @@ int TABLET_PostTabletMessage(LPOPENCONTE static inline DWORD ScaleForContext(DWORD In, DWORD InOrg, DWORD InExt, DWORD OutOrg, DWORD OutExt) { - if (((InExt > 0 )&&(OutExt > 0)) || ((InExt<0) && (OutExt < 0))) + if ((InExt > 0) && (OutExt > 0)) return ((In - InOrg) * abs(OutExt) / abs(InExt)) + OutOrg; else return ((abs(InExt) - (In - InOrg))*abs(OutExt) / abs(InExt)) + OutOrg;
On Sat, 1 Dec 2007, Gerald Pfeifer wrote:
This change of mine is obvious since InExt and OutExt are DWORD, thus unsigned, and thus always greater or equal zero so my patch should be fine in any case.
However, looking at the else-part of the if-statement, I have some doubts this is working as designed, and in general the abs() invocations are bogus.
Any expert who could help with a more extensive patch?
Both the two <0 checks and the abs() invocations are needless since DWORDS are positive by definition.
(I did not get _any_ response for my first mail a month ago and I still believe the else-part looks bogus, but can we get at least this first cleanup in?)
Gerald
ChangeLog: Simplify condition in ScaleForContext().
Index: dlls/wintab32/context.c =================================================================== RCS file: /home/wine/wine/dlls/wintab32/context.c,v retrieving revision 1.26 diff -u -3 -p -r1.26 context.c --- dlls/wintab32/context.c 31 Dec 2007 18:34:43 -0000 1.26 +++ dlls/wintab32/context.c 1 Jan 2008 16:59:56 -0000 @@ -160,10 +160,10 @@ int TABLET_PostTabletMessage(LPOPENCONTE static inline DWORD ScaleForContext(DWORD In, DWORD InOrg, DWORD InExt, DWORD OutOrg, DWORD OutExt) { - if (((InExt > 0 )&&(OutExt > 0)) || ((InExt<0) && (OutExt < 0))) - return ((In - InOrg) * abs(OutExt) / abs(InExt)) + OutOrg; + if ((InExt > 0) && (OutExt > 0)) + return ( (In - InOrg) * OutExt / InExt ) + OutOrg; else - return ((abs(InExt) - (In - InOrg))*abs(OutExt) / abs(InExt)) + OutOrg; + return ( (InExt - (In - InOrg)) * OutExt / InExt ) + OutOrg; }
LPOPENCONTEXT AddPacketToContextQueue(LPWTPACKET packet, HWND hwnd)
Gerald Pfeifer wrote on Tuesday, January 01, 2008 6:03 PM:
This code obviously was written with signed operands in mind. I wonder if they could get signed somewhere and this function simply uses the wrong datatype.
(I did not get _any_ response for my first mail a month ago and I still believe the else-part looks bogus, but can we get at least this first cleanup in?)
In the else statement InExt could be 0 (either that or OutExt would be 0) which would cause a division by 0 exception. Definitely doesn't sound right to me. But I have no idea what that code is supposed to do.
@@ -160,10 +160,10 @@ int TABLET_PostTabletMessage(LPOPENCONTE static inline DWORD ScaleForContext(DWORD In, DWORD InOrg, DWORD InExt, DWORD OutOrg, DWORD OutExt) {
- if (((InExt > 0 )&&(OutExt > 0)) || ((InExt<0) && (OutExt < 0)))
return ((In - InOrg) * abs(OutExt) / abs(InExt)) + OutOrg;
- if ((InExt > 0) && (OutExt > 0))
return ( (In - InOrg) * OutExt / InExt ) + OutOrg;
return ((abs(InExt) - (In - InOrg))*abs(OutExt) /abs(InExt)) + OutOrg;
return ( (InExt - (In - InOrg)) * OutExt / InExt ) + OutOrg;}
LPOPENCONTEXT AddPacketToContextQueue(LPWTPACKET packet, HWND hwnd)
Rolf Kalbermatter
-
Gerald Pfeifer -
Rolf Kalbermatter