Mouse-buttons swapped after winetest

[Paul Vriens]

On Fri, 2005-06-24 at 17:44, Saulius Krasuckas wrote:
> * On Tue, 21 Jun 2005, Paul Vriens wrote:
> > * On Tue, 2005-06-21 at 21:45, Paul Vriens wrote:
> > > 
> > > with the latest winetest
> > > http://www.astro.gla.ac.uk/users/paulm/WRT/CrossBuilt/winetest-200506211000-paul-mingw.exe
> > > 
> > > my mouse-buttons (left and right) are swapped after the test.
>   ...
> > sysparams.c:1005:testing SPI_{GET,SET}MOUSEBUTTONSWAP
> > sysparams.c:171:SPI_{GET,SET}MOUSEBUTTONSWAP not supported on this platform. Skipping test
> 
> IMHO the bug is caused by a patch of Vitaly Lipatov [1], which by itself 
> showed up a bug, caused by a combined patch from you, Paul, and Justin 
> Chevrier [2].
> 
> Following [1]:
> 
> | @@ -1010,7 +1010,7 @@ static void test_SPI_SETMOUSEBUTTONSWAP(
> |          SetLastError(0xdeadbeef);
> |          rc=SystemParametersInfoA( SPI_SETMOUSEBUTTONSWAP, vals[i], 0,
> |                                    SPIF_UPDATEINIFILE | SPIF_SENDCHANGE );
> | -        if (!test_error_msg(rc,"SPI_{GET,SET}MOUSEBUTTONSWAP"))
> | +        if (!test_error_msg(rc!=vals[i],"SPI_{GET,SET}MOUSEBUTTONSWAP"))
> |              return;
>   ...
> | @@ -1023,7 +1023,7 @@ static void test_SPI_SETMOUSEBUTTONSWAP(
> |  
> |      rc=SystemParametersInfoA( SPI_SETMOUSEBUTTONSWAP, old_b, 0,
> |                                SPIF_UPDATEINIFILE );
> | -    ok(rc!=0,"***warning*** failed to restore the original value: rc=%d err=%ld\n",rc,GetLastError());
> | +    ok(!rc,"***warning*** failed to restore the original value: rc=%d err=%ld\n",rc,GetLastError());
>  
> Such checks doesn't work on winME.  I have reverted them and tests is 
> doing fine.  Further, following [2]:
I agree these changes don't make sense. Although Vitaly says in his
patch:
/* MSDN says it returns previous state of swapbutton always */
it doesn't seem to be the case for at least win98/winME/NT/W2K and XP
(which is almost all versions !)
Reverting the above fixes, it for me as well.

> 
> | @@ -955,9 +1007,12 @@ static void test_SPI_SETMOUSEBUTTONSWAP(
> |  
> |      for (i=0;i<sizeof(vals)/sizeof(*vals);i++)
> |      {
> | +        SetLastError(0xdeadbeef);
> |          rc=SystemParametersInfoA( SPI_SETMOUSEBUTTONSWAP, vals[i], 0,
> |                                    SPIF_UPDATEINIFILE | SPIF_SENDCHANGE );
> | -        ok(rc!=0,"%d: rc=%d err=%ld\n",i,rc,GetLastError());
> | +        if (!test_error_msg(rc,"SPI_{GET,SET}MOUSEBUTTONSWAP"))
> | +            return;
> 
> This wrong, IMHO, as the "return" statement eliminates a possibility to 
> restore SM_SWAPBUTTON metrics after the for-loop ends.  I would use 
> "break" or "continue" here.
I agree that this can be done nicer, but I'd like to have some comments
from Vitaly about his patch before changing anything
> 
> Then would be nice to print "err" in following format "err=%0x%08lx" or 
> such after a restoration.  Any more ideas?
Why do you want to change that? I thought %08lx is pretty common in most
of the Wine code. (I remember seeing your email on wine-devel about that
though).

Cheers,

Paul.