Re: [PATCH v2 05/16] blame: accept multiple -L ranges

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Aug 6, 2013 at 5:33 PM, Junio C Hamano <gitster@pobox.com> 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).