Thread (40 messages) 40 messages, 4 authors, 2016-08-15

Re: [PATCH 5/8] xdl_change_compact(): fix compaction heuristic to adjust io

flat view

From: Jeff King <hidden>
Date: 2016-08-11 04:16:30

On Wed, Aug 10, 2016 at 06:58:06PM +0200, Michael Haggerty wrote:
quoted
And it looks like rchgo[io] always ends the loop on a 0. So it seems
like we would just hit that condition again.
Correct...in this loop. But there is another place where `io` is
incremented unconditionally. In the version before my changes, it is via
the preincrement operator in this while statement conditional:

https://github.com/mhagger/git/blob/a28705da929ad746abcb34270947f738549d3246/xdiff/xdiffi.c#L502

After my changes, the unconditional increment is more obvious because I
took it out of the while condition:

https://github.com/mhagger/git/blob/39a135da93834fd72ee923d95d0cebfe525dfe7a/xdiff/xdiffi.c#L541

(BTW, I think this is a good example of how patch 2/8 makes the code
easier to reason about.)
Ah, yeah, I totally missed that case (and I agree the code after your
2/8 makes it much more obvious).
I didn't do the hard work to determine whether `io` could *really* walk
off the end of the array, but I don't see an obvious reason why it
*couldn't*.
Yeah, I'm in agreement.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help