Re: [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