index: introduce git_index_iterator #4884
Conversation
|
Note that I think this is more accurately re-introduce. I think that we used to have an iterator for the index (likely with callbacks, because that was the iterator pattern of the day way back then) but we got rid of it in favor of expecting consumers to use |
| seen++; | ||
| } | ||
|
|
||
| cl_assert_equal_i(expected, seen); |
There was a problem hiding this comment.
This test fails on all platforms:
1) Failure:
index::tests::can_modify_while_iterating [D:\a\1\s\tests\index\tests.c:1073]
expected != seen
109 != 110
This is explained by the off-by-one above
| git_index *index); | ||
|
|
||
| /** | ||
| * Return the next index entry in-order from the iterator. |
There was a problem hiding this comment.
Should we maybe state what order the caller may expect?
tests/index/tests.c
Outdated
| cl_assert_equal_i(entry->file_size, test_entries[i].file_size); | ||
| found++; | ||
| } | ||
| } |
There was a problem hiding this comment.
If the iterator would return the same entry ARRAY_SIZE(test_entries) times, then this test would still succeed. If the test entries array was sorted as expected, then you could instead just loop through test_entries and verify that the current entry from test entries matches the one returned by the iterator. Like that, we know that all entries have been found and that the order is the expected one while avoiding this nested loop.
src/index.c
Outdated
| { | ||
| assert(out && it); | ||
|
|
||
| if (it->cur > git_vector_length(&it->snap)) |
There was a problem hiding this comment.
There's an off-by-one here. You will try to access the vector with it->cur == git_vector_length(&it->snap), leading to an OOB
Provide a public git_index_iterator API that is backed by an index snapshot. This allows consumers to provide a stable iteration even while manipulating the index during iteration.
ethomson
force-pushed
the
ethomson/index_iterator
branch
from
17c6e99
to
c358bbc
Compare
|
Oy, thanks for pointing out my arithmetic error. 🙄 Fixed up with your suggestions. |
|
Looks good to me, thanks! |
Provide a public git_index_iterator API that is backed by an index snapshot. This allows consumers to provide a stable iteration even while manipulating the index during iteration.