Jaime Soriano Pastor jsorianopas...@gmail.com writes:
I think this line is dangerous, if add_cache_entry is not able to
remove higher-stages it will be looping forever, as happens in the
case of this thread.
I cannot see why it's even needed, and removing it doesn't break any test.
On Sun, Aug 24, 2014 at 7:57 PM, Jaime Soriano Pastor
jsorianopas...@gmail.com wrote:
Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com
---
read-cache.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index c1a9619..3d70386 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1971,7 +1971,6 @@ int read_index_unmerged(struct index_state *istate)
if (add_index_entry(istate, new_ce, 0))
return error(%s: cannot drop to stage #0,
new_ce-name);
- i = index_name_pos(istate, new_ce-name, len);
I think the original idea was that regardless of how many entries
with the same name were removed because of the replacement (or
addition) of new_ce, by making i point at the newly added
new_ce, we would make sure that the loop will continue from the
next entry. The if/return expected that add_index_entry() will get
rid of all the other entries with the same name as new_ce has or it
will return an error.
Without the bug in add_index_entry(), because new_ce always has
the same name as ce, the entry we found at i by the loop, we know
that index_name_pos() will give the same i we already have, so
removing this line should be a no-op.
Now, add_index_entry() in your case did not notice that it failed to
remove all other entries with the same name as new_ce, resulting
in your looping forever. Among the merged and unmerged entries
with the same name exists in the index file class of index file
corruption, we could treat the merged and unmerged entries with the
same name not just exists but next to each other, unmerged ones
coming immediately after merged one case specially (i.e. declaring
that it is more likely for a broken software to leave both next to
each other than otherwise) and try to accomodate it as your original
patch did. I am not absolutely sure if such a special case is worth
it, and with your updated [1/2] read_index_from(): check order of
entries when reading index we will not be doing so, which is good.
With that safety in place, the bug in add_index_entry() will
disappear; it is safe not to adjust i by calling index_name_pos()
and this patch, [2/2] read_index_unmerged(): remove unnecessary
loop index adjustment, will be a good thing to do.
Thanks.
}
return unmerged;
}
--
2.0.4.1.g0b8a4f9.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
