On Tue, Aug 6, 2013 at 5:33 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> + for (range_i = ranges.nr; range_i > 0; --range_i) {
>> + const struct range *r = &ranges.ranges[range_i - 1];
>> + long bottom = r->start;
>> + long top = r->end;
>> + struct blame_entry *next = ent;
>> + ent = xcalloc(1, sizeof(*ent));
>> + ent->lno = bottom;
>> + ent->num_lines = top - bottom;
>> + ent->suspect = o;
>> + ent->s_lno = bottom;
>> + ent->next = next;
>> + if (next)
>> + next->prev = ent;
>> + origin_incref(o);
>> + }
>> + origin_decref(o);
>
> Hmph, I do not see where the need for this decref is coming from.
> Did we incref once too many somewhere?
Each constructed blame_entry must own a reference to the suspect.
o->refcnt should equal the number of blame_entries. At construction, a
'struct origin' has refcnt 1. In the original code, which supported
only a single initial range (blame_entry), we had:
o = get-initial-suspect(); # refcnt already 1
ent->suspect = o; # refcnt still 1
sb.ent = ent;
assign_blame(&sb);
So, o->refcnt equals the number of blame_entries (1) when
assign_blame() is called.
The new for-loop calls origin_incref() on each iteration since each
blame_entry needs to own a reference to the suspect. Assume that we
have two disjoint -L ranges:
o = get-initial-suspect(); # refcnt already 1
foreach range:
ent = new blame_entry;
ent->suspect = o;
origin_incref(o); # refcnt++
end
# for 2 ranges, refcnt incremented twice, so value is 3
origin_decref(o); # refcnt = 2
sb.ent = ent;
assign_blame(&sb);
Thus, as with the original code, o->refcnt equals the number of
blame_entries (2) when assign_blame() is called.
The same holds for the boundary case when the file is empty and there
is no range. o->refcnt starts at 1, the loop is never entered so no
blame_entries are created, and o->refcnt gets decremented to 0, which
again matches the number of blame_entries (0).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html