Re: [PATCH 4/7] Fix qdisc_watchdog_schedule_range_ns range check

[Thomas Gleixner]

On Thu, Oct 01 2020 at 22:51, Erez Geva wrote:

Fixes should be at the beginning of a patch series and not be hidden
somewhere in the middle.

>    - As all parameters are unsigned.

This is not a sentence and this list style does not make that changelog
more readable.

>    - If 'expires' is bigger than 'last_expires' then the left expression
>      overflows.

This would be the most important information and should be clearly
spelled out as problem description at the very beginning of the change
log.

>    - It is better to use addition and check both ends of the range.

Is better? Either your change is correcting the problem or not. Just
better but incorrect does not cut it.

But let's look at the problem itself. The check is about:

    B <= A <= B + C

A, B, C are all unsigned. So if B > A then the result is false.

Now lets look at the implementation:

    if (A - B <= C)
    	return;

which works correctly due the wonders of unsigned math.

For B <= A the check is obviously correct.

If B > A then the result of the unsigned subtraction A - B is a very
large positive number which is pretty much guaranteed to be larger than
C, i.e. the result is false.

So while not immediately obvious, it's still correct.

Thanks,

        tglx