On 2/4/21 1:49 AM, Zhihong Yu wrote: > Hi, > For 0007-Remove-the-special-batch-mode-use-a-larger--20210203.patch : > > + /* same as preceding value, so store it */ > + if (compare_values(&range->values[start + i - 1], > + &range->values[start + i], > + (void *) &cxt) == 0) > + continue; > + > + range->values[start + n] = range->values[start + i]; > > It seems the comment doesn't match the code: the value is stored when > subsequent value is different from the previous. >
Yeah, you're right the comment is wrong - the code is doing exactly the opposite. I'll need to go through this more carefully.
> For has_matching_range(): > + int midpoint = (start + end) / 2; > > I think the standard notion for midpoint is start + (end-start)/2. > > + /* this means we ran out of ranges in the last step */ > + if (start > end) > + return false; > > It seems the above should be ahead of computation of midpoint. >
Not sure why would that be an issue, as we're not using the value and the values are just plain integers (so no overflows ...).