Junio C Hamano <[email protected]> writes:
After saying "Will merge to 'next'" in the recent "What's cooking"
report, I noticed that a few loose ends were never tied on this
topic.
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index dd0dba5b1d..252a21cc19 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -500,6 +500,10 @@ information.
>> --pickaxe-regex::
>> Treat the <string> given to `-S` as an extended POSIX regular
>> expression to match.
>> +--blobfind=<blob-id>::
>> + Restrict the output such that one side of the diff
>> + matches the given blob-id.
>> +
>> endif::git-format-patch[]
>
> Can we have a blank line between these enumerations to make the
> source easier to read? Thanks.
>
> ...
> So, we keep an unmerged pair, a pair that mentions a sought-blob on
> one side or the other side? I am not sure if we want to keep the
> unmerged pair for the purpose of this one.
>
>> + diff_free_filepair(p);
>> + q->queue[i] = NULL;
>> + c--;
>
> Also, if you are doing the in-place shrinking and have already
> introduced another counter 'j' that is initialized to 0, I think it
> makes more sense to do the shrinking in-place. 'i' will stay to be
> the source-scan pointer that runs 0 thru q->nr, while 'j' can be
> used in this loop (where you have 'continue') to move the current
> one that is determined to survive from q->queue[i] to q->queue[j++].
>
> Then you do not need 'c'; when the loop ends, 'j' would be the
> number of surviving entries and q->nr can be adjusted to it. Unlike
> the usual pattern taken by the other diffcore transformations where
> a new queue is populated and the old one discarded, this would leave
> the q->queue[] over-allocated, but I do not think it is too bad.
Here is to illustrate the last point. I still think we should keep
the unmerged entries for the purpose of blobfind but it should be
trivial to fix that.
diffcore-blobfind.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/diffcore-blobfind.c b/diffcore-blobfind.c
index 5d222fc336..bf63ba61dc 100644
--- a/diffcore-blobfind.c
+++ b/diffcore-blobfind.c
@@ -8,40 +8,31 @@
static void diffcore_filter_blobs(struct diff_queue_struct *q,
struct diff_options *options)
{
- int i, j = 0, c = q->nr;
+ int src, dst;
if (!options->blobfind)
BUG("blobfind oidset not initialized???");
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
+ for (src = dst = 0; src < q->nr; src++) {
+ struct diff_filepair *p = q->queue[src];
if (DIFF_PAIR_UNMERGED(p) ||
(DIFF_FILE_VALID(p->one) &&
oidset_contains(options->blobfind, &p->one->oid)) ||
(DIFF_FILE_VALID(p->two) &&
- oidset_contains(options->blobfind, &p->two->oid)))
- continue;
-
- diff_free_filepair(p);
- q->queue[i] = NULL;
- c--;
- }
-
- /* Keep it sorted. */
- i = 0; j = 0;
- while (i < c) {
- while (!q->queue[j])
- j++;
- q->queue[i] = q->queue[j];
- i++; j++;
+ oidset_contains(options->blobfind, &p->two->oid))) {
+ q->queue[dst] = p;
+ dst++;
+ } else {
+ diff_free_filepair(p);
+ }
}
- q->nr = c;
-
- if (!c) {
+ if (!dst) {
free(q->queue);
DIFF_QUEUE_CLEAR(q);
+ } else {
+ q->nr = dst;
}
}
