[PATCH] comctl32/progress: Fix wrapping of values in PBM_STEPIT and add tests

[Fabian Maurer]

On Montag, 26. Februar 2018 23:15:04 CET Nikolay Sivov wrote:
> On 2/27/2018 12:14 AM, Fabian Maurer wrote:
> > @@ -656,7 +656,7 @@ static LRESULT WINAPI ProgressWindowProc(HWND hwnd,
> > UINT message,> 
> >          oldVal = infoPtr->CurVal;
> >          infoPtr->CurVal += infoPtr->Step;
> >          if(infoPtr->CurVal > infoPtr->MaxVal)
> > 
> > -	    infoPtr->CurVal = infoPtr->MinVal;
> > +	    infoPtr->CurVal = infoPtr->CurVal % infoPtr->MaxVal;
> 
> This looks wrong. I don't think MinVal should be ignored. Your tests
> don't show it because you have [0, 12] range.

Thanks, completely overlooked that. I rewrote the tests to account for that - and also to 
account for negative stepping values.

> 
> > +static void test_wrapping(void)
> 
> It should be test_PBM_STEPIT().
> 
> > +{
> > +    const int TEST_MAXIMUM = 12;
> > +    const int TEST_STEP_SMALL = 5;
> > +    const int TEST_STEP_BIG = 50;
> 
> Constants are not helping here I think, it's easier and is more readable
> to have them inlined.

Thanks for the info, changed it.

> Do we have a bug report for this issue by any chance?

We don't, but reactos has it (for wine code): https://jira.reactos.org/browse/CORE-13873

Regards,
Fabian Maurer


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20180227/c3b6dc6c/attachment.html>