Jeff King <peff@xxxxxxxx> writes:
> Subject: [PATCH] diff: discard blob data from stat-unmatched pairs
>
> When performing a tree-level diff against the working tree, we may find
> that our index stat information is dirty, so we queue a filepair to be
> examined later. If the actual content hasn't changed, we call this a
> stat-unmatch; the stat information was out of date, but there's no
> actual diff. Normally diffcore_std() would detect and remove these
> identical filepairs via diffcore_skip_stat_unmatch(). However, when
> "--quiet" is used, we want to stop the diff as soon as we see any
> changes, so we check for stat-unmatches immediately in diff_change().
>
> That check may require us to actually load the file contents into the
> pair of diff_filespecs. If we find that the pair isn't a stat-unmatch,
> then no big deal; we'd likely load the contents later anyway to generate
> a patch, do rename detection, etc, so we want to hold on to it. But if
> it a stat-unmatch, then we have no more use for that data; the whole
> point is that we're going discard the pair. However, we never free the
> allocated diff_filespec data.
Nicely spotted. So we can discard when quiet is in effect after
this check, which makes sense.
After reading the initial analysis, I wondered if the fix we did in
f34b205f (diff: do not quit early on stat-dirty files, 2014-01-25)
was suboptimal and we should have instead done the "if QUICK, check
if the pair is merely stat-unmatch" in the loop(s) that call
diff_change(), hoping that it may have given us a better control
over the lifetime of the filespecs in each diff_filepair, but I do
not think it would made much difference.
> if (options->flags.quick && options->skip_stat_unmatch &&
> - !diff_filespec_check_stat_unmatch(options->repo, p))
> + !diff_filespec_check_stat_unmatch(options->repo, p)) {
> + diff_free_filespec_data(p->one);
> + diff_free_filespec_data(p->two);
> return;
> + }
Thanks. Will queue (with that s/it/& is/ typofix mentioned
elsewhere).
