On Tue, Aug 15, 2017 at 04:23:50AM +0000, Kevin Willford wrote: > > This read_cache_from() should be a noop, right, because it immediately > > sees istate->initialized is set? So it shouldn't matter that it is not > > in the conditional with discard_cache(). Though if its only purpose is > > to re-read the just-discarded contents, perhaps it makes sense to put it > > there for readability. > > I thought about that and didn't know if there were cases when this would be called > and the cache has not been loaded. It didn't look like it since it is only called from > cmd_commit and prepare_index is called before it. Also if in the future this call would > be made when it had not read the index yet so thought it was safest just to leave > this as always being called since it is basically a noop if the istate->initialized is set. Yeah, I agree it might be safer as you have it. And hopefully the comment above the discard would point anybody in the right direction. > Also based on this commit > https://github.com/git/git/commit/2888605c649ccd423232161186d72c0e6c458a48 > it looked like the discard_cache was added independent of the read_cache_from call, > which made me think that the two were not tied together. Yeah, I tried to dig in the history and figure it out, but it was not nearly as clear as I would have liked. :) -Peff