Commit 3dbee456564a9baf24631bfe219f81434d8fdfa6

Russell Belfer 2014-02-07T14:10:35

Some index internals refactoring Again, laying groundwork for some index iterator changes, this contains a bunch of code refactorings for index internals that should make it easier down the line to add locking around index modifications. Also this removes the redundant prefix_position function and fixes some potential memory leaks.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
diff --git a/include/git2/index.h b/include/git2/index.h
index dd6a28e..4d33f13 100644
--- a/include/git2/index.h
+++ b/include/git2/index.h
@@ -77,7 +77,12 @@ typedef struct git_index_entry {
 #define GIT_IDXENTRY_VALID     (0x8000)
 #define GIT_IDXENTRY_STAGESHIFT 12
 
-#define GIT_IDXENTRY_STAGE(E) (((E)->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT)
+#define GIT_IDXENTRY_STAGE(E) \
+	(((E)->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT)
+
+#define GIT_IDXENTRY_STAGE_SET(E,S) do { \
+	(E)->flags = ((E)->flags & ~GIT_IDXENTRY_STAGEMASK) | \
+		(((S) & 0x03) << GIT_IDXENTRY_STAGESHIFT); } while (0)
 
 /**
  * Bitmasks for on-disk fields of `git_index_entry`'s `flags_extended`
@@ -327,12 +332,14 @@ GIT_EXTERN(size_t) git_index_entrycount(const git_index *index);
 
 /**
  * Clear the contents (all the entries) of an index object.
- * This clears the index object in memory; changes must be manually
- * written to disk for them to take effect.
+ *
+ * This clears the index object in memory; changes must be explicitly
+ * written to disk for them to take effect persistently.
  *
  * @param index an existing index object
+ * @return 0 on success, error code < 0 on failure
  */
-GIT_EXTERN(void) git_index_clear(git_index *index);
+GIT_EXTERN(int) git_index_clear(git_index *index);
 
 /**
  * Get a pointer to one of the entries in the index
diff --git a/src/checkout.c b/src/checkout.c
index f9bc5e9..a251c08 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -277,19 +277,23 @@ static int checkout_action_wd_only(
 
 	/* check if item is tracked in the index but not in the checkout diff */
 	if (data->index != NULL) {
+		size_t pos;
+
+		error = git_index__find(
+			&pos, data->index, wd->path, 0, GIT_INDEX_STAGE_ANY);
+
 		if (wd->mode != GIT_FILEMODE_TREE) {
-			if (!(error = git_index_find(NULL, data->index, wd->path))) {
+			if (!error) { /* found by git_index__find call */
 				notify = GIT_CHECKOUT_NOTIFY_DIRTY;
 				remove = ((data->strategy & GIT_CHECKOUT_FORCE) != 0);
 			} else if (error != GIT_ENOTFOUND)
 				return error;
 			else
-				giterr_clear();
+				error = 0; /* git_index__find does not set error msg */
 		} else {
 			/* for tree entries, we have to see if there are any index
 			 * entries that are contained inside that tree
 			 */
-			size_t pos = git_index__prefix_position(data->index, wd->path);
 			const git_index_entry *e = git_index_get_byindex(data->index, pos);
 
 			if (e != NULL && data->diff->pfxcomp(e->path, wd->path) == 0) {
diff --git a/src/index.c b/src/index.c
index 08c4b79..8bc5cb1 100644
--- a/src/index.c
+++ b/src/index.c
@@ -345,7 +345,7 @@ void git_index__set_ignore_case(git_index *index, bool ignore_case)
 int git_index_open(git_index **index_out, const char *index_path)
 {
 	git_index *index;
-	int error;
+	int error = -1;
 
 	assert(index_out);
 
@@ -354,7 +354,8 @@ int git_index_open(git_index **index_out, const char *index_path)
 
 	if (index_path != NULL) {
 		index->index_file_path = git__strdup(index_path);
-		GITERR_CHECK_ALLOC(index->index_file_path);
+		if (!index->index_file_path)
+			goto fail;
 
 		/* Check if index file is stored on disk already */
 		if (git_path_exists(index->index_file_path) == true)
@@ -364,22 +365,24 @@ int git_index_open(git_index **index_out, const char *index_path)
 	if (git_vector_init(&index->entries, 32, index_cmp) < 0 ||
 		git_vector_init(&index->names, 32, conflict_name_cmp) < 0 ||
 		git_vector_init(&index->reuc, 32, reuc_cmp) < 0)
-		return -1;
+		goto fail;
 
 	index->entries_cmp_path = index_cmp_path;
 	index->entries_search = index_srch;
 	index->entries_search_path = index_srch_path;
 	index->reuc_search = reuc_srch;
 
-	if ((index_path != NULL) && ((error = git_index_read(index, true)) < 0)) {
-		git_index_free(index);
-		return error;
-	}
+	if (index_path != NULL && (error = git_index_read(index, true)) < 0)
+		goto fail;
 
 	*index_out = index;
 	GIT_REFCOUNT_INC(index);
 
 	return 0;
+
+fail:
+	git_index_free(index);
+	return error;
 }
 
 int git_index_new(git_index **out)
@@ -418,18 +421,20 @@ static void index_entries_free(git_vector *entries)
 	git_vector_clear(entries);
 }
 
-void git_index_clear(git_index *index)
+int git_index_clear(git_index *index)
 {
 	assert(index);
 
+	git_tree_cache_free(index->tree);
+	index->tree = NULL;
+
 	index_entries_free(&index->entries);
 	git_index_reuc_clear(index);
 	git_index_name_clear(index);
 
 	git_futils_filestamp_set(&index->stamp, NULL);
 
-	git_tree_cache_free(index->tree);
-	index->tree = NULL;
+	return 0;
 }
 
 static int create_index_error(int error, const char *msg)
@@ -495,7 +500,7 @@ int git_index_read(git_index *index, int force)
 
 	if (!index->on_disk) {
 		if (force)
-			git_index_clear(index);
+			return git_index_clear(index);
 		return 0;
 	}
 
@@ -507,8 +512,10 @@ int git_index_read(git_index *index, int force)
 	if (error < 0)
 		return error;
 
-	git_index_clear(index);
-	error = parse_index(index, buffer.ptr, buffer.size);
+	error = git_index_clear(index);
+
+	if (!error)
+		error = parse_index(index, buffer.ptr, buffer.size);
 
 	if (!error)
 		git_futils_filestamp_set(&index->stamp, &stamp);
@@ -679,12 +686,31 @@ static int index_entry_init(
 
 	entry->id = oid;
 	entry->path = git__strdup(rel_path);
-	GITERR_CHECK_ALLOC(entry->path);
+	if (!entry->path) {
+		git__free(entry);
+		return -1;
+	}
 
 	*entry_out = entry;
 	return 0;
 }
 
+static int index_remove_entry(git_index *index, size_t pos)
+{
+	int error = 0;
+	git_index_entry *entry = git_vector_get(&index->entries, pos);
+
+	if (entry != NULL)
+		git_tree_cache_invalidate_path(index->tree, entry->path);
+
+	error = git_vector_remove(&index->entries, pos);
+
+	if (!error)
+		index_entry_free(entry);
+
+	return error;
+}
+
 static int index_entry_reuc_init(git_index_reuc_entry **reuc_out,
 	const char *path,
 	int ancestor_mode, const git_oid *ancestor_oid,
@@ -701,8 +727,10 @@ static int index_entry_reuc_init(git_index_reuc_entry **reuc_out,
 	GITERR_CHECK_ALLOC(reuc);
 
 	reuc->path = git__strdup(path);
-	if (reuc->path == NULL)
+	if (reuc->path == NULL) {
+		git__free(reuc);
 		return -1;
+	}
 
 	if ((reuc->mode[0] = ancestor_mode) > 0)
 		git_oid_cpy(&reuc->oid[0], ancestor_oid);
@@ -717,22 +745,29 @@ static int index_entry_reuc_init(git_index_reuc_entry **reuc_out,
 	return 0;
 }
 
-static git_index_entry *index_entry_dup(const git_index_entry *source_entry)
+static int index_entry_dup(git_index_entry **out, const git_index_entry *src)
 {
 	git_index_entry *entry;
 
+	if (!src) {
+		*out = NULL;
+		return 0;
+	}
+
 	entry = git__malloc(sizeof(git_index_entry));
-	if (!entry)
-		return NULL;
+	GITERR_CHECK_ALLOC(entry);
 
-	memcpy(entry, source_entry, sizeof(git_index_entry));
+	memcpy(entry, src, sizeof(git_index_entry));
 
 	/* duplicate the path string so we own it */
 	entry->path = git__strdup(entry->path);
-	if (!entry->path)
-		return NULL;
+	if (!entry->path) {
+		git__free(entry);
+		return -1;
+	}
 
-	return entry;
+	*out = entry;
+	return 0;
 }
 
 static int has_file_name(git_index *index,
@@ -757,7 +792,9 @@ static int has_file_name(git_index *index,
 		retval = -1;
 		if (!ok_to_replace)
 			break;
-		git_vector_remove(&index->entries, --pos);
+
+		if (index_remove_entry(index, --pos) < 0)
+			break;
 	}
 	return retval;
 }
@@ -790,7 +827,8 @@ static int has_dir_name(git_index *index,
 			if (!ok_to_replace)
 				break;
 
-			git_vector_remove(&index->entries, position);
+			if (index_remove_entry(index, position) < 0)
+				break;
 			continue;
 		}
 
@@ -830,12 +868,22 @@ static int check_file_directory_collision(git_index *index,
 	return 0;
 }
 
-static int index_insert(git_index *index, git_index_entry *entry, int replace)
+/* index_insert takes ownership of the new entry - if it can't insert
+ * it, then it will return an error **and also free the entry**.  When
+ * it replaces an existing entry, it will update the entry_ptr with the
+ * actual entry in the index (and free the passed in one).
+ */
+static int index_insert(
+	git_index *index, git_index_entry **entry_ptr, int replace)
 {
+	int error = 0;
 	size_t path_length, position;
-	git_index_entry **existing = NULL;
+	git_index_entry *existing = NULL, *entry;
 
-	assert(index && entry && entry->path != NULL);
+	assert(index && entry_ptr);
+
+	entry = *entry_ptr;
+	assert(entry && entry->path);
 
 	/* make sure that the path length flag is correct */
 	path_length = strlen(entry->path);
@@ -850,27 +898,41 @@ static int index_insert(git_index *index, git_index_entry *entry, int replace)
 	/* look if an entry with this path already exists */
 	if (!git_index__find(
 			&position, index, entry->path, 0, GIT_IDXENTRY_STAGE(entry))) {
-		existing = (git_index_entry **)&index->entries.contents[position];
+		existing = index->entries.contents[position];
 		/* update filemode to existing values if stat is not trusted */
-		entry->mode = index_merge_mode(index, *existing, entry->mode);
+		entry->mode = index_merge_mode(index, existing, entry->mode);
 	}
 
-	if (check_file_directory_collision(index, entry, position, replace) < 0)
-		return -1;
+	error = check_file_directory_collision(index, entry, position, replace);
+	if (error < 0)
+		goto done;
+
+	/* if we are replacing an existing item, overwrite the existing entry
+	 * and return it in place of the passed in one.
+	 */
+	if (existing && replace) {
+		git__free(entry->path);
+		entry->path = existing->path;
+
+		memcpy(existing, entry, sizeof(*entry));
+		*entry_ptr = existing;
+
+		git__free(entry);
+		return 0;
+	}
 
 	/* if replacing is not requested or no existing entry exists, just
 	 * insert entry at the end; the index is no longer sorted
 	 */
-	if (!replace || !existing)
-		return git_vector_insert(&index->entries, entry);
+	error = git_vector_insert(&index->entries, entry);
 
-	/* exists, replace it (preserving name from existing entry) */
-	git__free(entry->path);
-	entry->path = (*existing)->path;
-	git__free(*existing);
-	*existing = entry;
+done:
+	if (error < 0) {
+		index_entry_free(*entry_ptr);
+		*entry_ptr = NULL;
+	}
 
-	return 0;
+	return error;
 }
 
 static int index_conflict_to_reuc(git_index *index, const char *path)
@@ -907,19 +969,15 @@ int git_index_add_bypath(git_index *index, const char *path)
 	assert(index && path);
 
 	if ((ret = index_entry_init(&entry, index, path)) < 0 ||
-		(ret = index_insert(index, entry, 1)) < 0)
-		goto on_error;
+		(ret = index_insert(index, &entry, 1)) < 0)
+		return ret;
 
 	/* Adding implies conflict was resolved, move conflict entries to REUC */
 	if ((ret = index_conflict_to_reuc(index, path)) < 0 && ret != GIT_ENOTFOUND)
-		goto on_error;
+		return ret;
 
 	git_tree_cache_invalidate_path(index->tree, entry->path);
 	return 0;
-
-on_error:
-	index_entry_free(entry);
-	return ret;
 }
 
 int git_index_remove_bypath(git_index *index, const char *path)
@@ -942,14 +1000,11 @@ int git_index_add(git_index *index, const git_index_entry *source_entry)
 	git_index_entry *entry = NULL;
 	int ret;
 
-	entry = index_entry_dup(source_entry);
-	if (entry == NULL)
-		return -1;
+	assert(index && source_entry);
 
-	if ((ret = index_insert(index, entry, 1)) < 0) {
-		index_entry_free(entry);
+	if ((ret = index_entry_dup(&entry, source_entry)) < 0 ||
+		(ret = index_insert(index, &entry, 1)) < 0)
 		return ret;
-	}
 
 	git_tree_cache_invalidate_path(index->tree, entry->path);
 	return 0;
@@ -958,8 +1013,6 @@ int git_index_add(git_index *index, const git_index_entry *source_entry)
 int git_index_remove(git_index *index, const char *path, int stage)
 {
 	size_t position;
-	int error;
-	git_index_entry *entry;
 
 	if (git_index__find(&position, index, path, 0, stage) < 0) {
 		giterr_set(GITERR_INDEX, "Index does not contain %s at stage %d",
@@ -967,16 +1020,7 @@ int git_index_remove(git_index *index, const char *path, int stage)
 		return GIT_ENOTFOUND;
 	}
 
-	entry = git_vector_get(&index->entries, position);
-	if (entry != NULL)
-		git_tree_cache_invalidate_path(index->tree, entry->path);
-
-	error = git_vector_remove(&index->entries, position);
-
-	if (!error)
-		index_entry_free(entry);
-
-	return error;
+	return index_remove_entry(index, position);
 }
 
 int git_index_remove_directory(git_index *index, const char *dir, int stage)
@@ -989,9 +1033,7 @@ int git_index_remove_directory(git_index *index, const char *dir, int stage)
 	if (git_buf_sets(&pfx, dir) < 0 || git_path_to_dir(&pfx) < 0)
 		return -1;
 
-	git_vector_sort(&index->entries);
-
-	pos = git_index__prefix_position(index, pfx.ptr);
+	git_index__find(&pos, index, pfx.ptr, pfx.size, GIT_INDEX_STAGE_ANY);
 
 	while (1) {
 		entry = git_vector_get(&index->entries, pos);
@@ -1003,11 +1045,8 @@ int git_index_remove_directory(git_index *index, const char *dir, int stage)
 			continue;
 		}
 
-		git_tree_cache_invalidate_path(index->tree, entry->path);
-
-		if ((error = git_vector_remove(&index->entries, pos)) < 0)
+		if ((error = index_remove_entry(index, pos)) < 0)
 			break;
-		index_entry_free(entry);
 
 		/* removed entry at 'pos' so we don't need to increment it */
 	}
@@ -1063,22 +1102,6 @@ int git_index_find(size_t *at_pos, git_index *index, const char *path)
 	return 0;
 }
 
-size_t git_index__prefix_position(git_index *index, const char *path)
-{
-	struct entry_srch_key srch_key;
-	size_t pos;
-
-	srch_key.path = path;
-	srch_key.path_len = strlen(path);
-	srch_key.stage = 0;
-
-	git_vector_sort(&index->entries);
-	git_vector_bsearch2(
-		&pos, &index->entries, index->entries_search, &srch_key);
-
-	return pos;
-}
-
 int git_index_conflict_add(git_index *index,
 	const git_index_entry *ancestor_entry,
 	const git_index_entry *our_entry,
@@ -1090,21 +1113,22 @@ int git_index_conflict_add(git_index *index,
 
 	assert (index);
 
-	if ((ancestor_entry != NULL && (entries[0] = index_entry_dup(ancestor_entry)) == NULL) ||
-		(our_entry != NULL && (entries[1] = index_entry_dup(our_entry)) == NULL) ||
-		(their_entry != NULL && (entries[2] = index_entry_dup(their_entry)) == NULL))
-		return -1;
+	if ((ret = index_entry_dup(&entries[0], ancestor_entry)) < 0 ||
+		(ret = index_entry_dup(&entries[1], our_entry)) < 0 ||
+		(ret = index_entry_dup(&entries[2], their_entry)) < 0)
+		goto on_error;
 
 	for (i = 0; i < 3; i++) {
 		if (entries[i] == NULL)
 			continue;
 
 		/* Make sure stage is correct */
-		entries[i]->flags = (entries[i]->flags & ~GIT_IDXENTRY_STAGEMASK) |
-			((i+1) << GIT_IDXENTRY_STAGESHIFT);
+		GIT_IDXENTRY_STAGE_SET(entries[i], i + 1);
 
-		if ((ret = index_insert(index, entries[i], 1)) < 0)
+		if ((ret = index_insert(index, &entries[i], 1)) < 0)
 			goto on_error;
+
+		entries[i] = NULL; /* don't free if later entry fails */
 	}
 
 	return 0;
@@ -1196,7 +1220,7 @@ int git_index_conflict_get(
 
 int git_index_conflict_remove(git_index *index, const char *path)
 {
-	size_t pos, posmax;
+	size_t pos;
 	git_index_entry *conflict_entry;
 	int error = 0;
 
@@ -1205,10 +1229,7 @@ int git_index_conflict_remove(git_index *index, const char *path)
 	if (git_index_find(&pos, index, path) < 0)
 		return GIT_ENOTFOUND;
 
-	posmax = git_index_entrycount(index);
-
-	while (pos < posmax) {
-		conflict_entry = git_vector_get(&index->entries, pos);
+	while ((conflict_entry = git_vector_get(&index->entries, pos)) != NULL) {
 
 		if (index->entries_cmp_path(conflict_entry->path, path) != 0)
 			break;
@@ -1218,14 +1239,11 @@ int git_index_conflict_remove(git_index *index, const char *path)
 			continue;
 		}
 
-		if ((error = git_vector_remove(&index->entries, pos)) < 0)
-			return error;
-
-		index_entry_free(conflict_entry);
-		posmax--;
+		if ((error = index_remove_entry(index, pos)) < 0)
+			break;
 	}
 
-	return 0;
+	return error;
 }
 
 static int index_conflicts_match(const git_vector *v, size_t idx, void *p)
@@ -1341,32 +1359,36 @@ const git_index_name_entry *git_index_name_get_byindex(
 	return git_vector_get(&index->names, n);
 }
 
+static void index_name_entry_free(git_index_name_entry *ne)
+{
+	if (!ne)
+		return;
+	git__free(ne->ancestor);
+	git__free(ne->ours);
+	git__free(ne->theirs);
+	git__free(ne);
+}
+
 int git_index_name_add(git_index *index,
 	const char *ancestor, const char *ours, const char *theirs)
 {
 	git_index_name_entry *conflict_name;
 
-	assert ((ancestor && ours) || (ancestor && theirs) || (ours && theirs));
+	assert((ancestor && ours) || (ancestor && theirs) || (ours && theirs));
 
 	conflict_name = git__calloc(1, sizeof(git_index_name_entry));
 	GITERR_CHECK_ALLOC(conflict_name);
 
-	if (ancestor) {
-		conflict_name->ancestor = git__strdup(ancestor);
-		GITERR_CHECK_ALLOC(conflict_name->ancestor);
-	}
-
-	if (ours) {
-		conflict_name->ours = git__strdup(ours);
-		GITERR_CHECK_ALLOC(conflict_name->ours);
-	}
-
-	if (theirs) {
-		conflict_name->theirs = git__strdup(theirs);
-		GITERR_CHECK_ALLOC(conflict_name->theirs);
+	if ((ancestor && !(conflict_name->ancestor = git__strdup(ancestor))) ||
+		(ours     && !(conflict_name->ours     = git__strdup(ours))) ||
+		(theirs   && !(conflict_name->theirs   = git__strdup(theirs))) ||
+		git_vector_insert(&index->names, conflict_name) < 0)
+	{
+		index_name_entry_free(conflict_name);
+		return -1;
 	}
 
-	return git_vector_insert(&index->names, conflict_name);
+	return 0;
 }
 
 void git_index_name_clear(git_index *index)
@@ -1376,18 +1398,8 @@ void git_index_name_clear(git_index *index)
 
 	assert(index);
 
-	git_vector_foreach(&index->names, i, conflict_name) {
-		if (conflict_name->ancestor)
-			git__free(conflict_name->ancestor);
-
-		if (conflict_name->ours)
-			git__free(conflict_name->ours);
-
-		if (conflict_name->theirs)
-			git__free(conflict_name->theirs);
-
-		git__free(conflict_name);
-	}
+	git_vector_foreach(&index->names, i, conflict_name)
+		index_name_entry_free(conflict_name);
 
 	git_vector_clear(&index->names);
 }
@@ -1432,15 +1444,13 @@ int git_index_reuc_add(git_index *index, const char *path,
 
 	assert(index && path);
 
-	if ((error = index_entry_reuc_init(&reuc, path, ancestor_mode, ancestor_oid, our_mode, our_oid, their_mode, their_oid)) < 0 ||
+	if ((error = index_entry_reuc_init(&reuc, path, ancestor_mode,
+			ancestor_oid, our_mode, our_oid, their_mode, their_oid)) < 0 ||
 		(error = index_reuc_insert(index, reuc, 1)) < 0)
-	{
 		index_entry_reuc_free(reuc);
-		return error;
-	}
 
 	return error;
-} 
+}
 
 int git_index_reuc_find(size_t *at_pos, git_index *index, const char *path)
 {
@@ -1752,6 +1762,7 @@ static size_t read_extension(git_index *index, const char *buffer, size_t buffer
 
 static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
 {
+	int error = 0;
 	unsigned int i;
 	struct index_header header = { 0 };
 	git_oid checksum_calculated, checksum_expected;
@@ -1771,8 +1782,8 @@ static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
 	git_hash_buf(&checksum_calculated, buffer, buffer_size - INDEX_FOOTER_SIZE);
 
 	/* Parse header */
-	if (read_header(&header, buffer) < 0)
-		return -1;
+	if ((error = read_header(&header, buffer)) < 0)
+		return error;
 
 	seek_forward(INDEX_HEADER_SIZE);
 
@@ -1784,22 +1795,29 @@ static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
 		git_index_entry *entry;
 
 		entry = git__malloc(sizeof(git_index_entry));
-		GITERR_CHECK_ALLOC(entry);
+		if (!entry) {
+			error = -1;
+			goto done;
+		}
 
 		entry_size = read_entry(entry, buffer, buffer_size);
 
 		/* 0 bytes read means an object corruption */
-		if (entry_size == 0)
-			return index_error_invalid("invalid entry");
+		if (entry_size == 0) {
+			error = index_error_invalid("invalid entry");
+			goto done;
+		}
 
-		if (git_vector_insert(&index->entries, entry) < 0)
-			return -1;
+		if ((error = git_vector_insert(&index->entries, entry)) < 0)
+			goto done;
 
 		seek_forward(entry_size);
 	}
 
-	if (i != header.entry_count)
-		return index_error_invalid("header entries changed while parsing");
+	if (i != header.entry_count) {
+		error = index_error_invalid("header entries changed while parsing");
+		goto done;
+	}
 
 	/* There's still space for some extensions! */
 	while (buffer_size > INDEX_FOOTER_SIZE) {
@@ -1808,20 +1826,28 @@ static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
 		extension_size = read_extension(index, buffer, buffer_size);
 
 		/* see if we have read any bytes from the extension */
-		if (extension_size == 0)
-			return index_error_invalid("extension is truncated");
+		if (extension_size == 0) {
+			error = index_error_invalid("extension is truncated");
+			goto done;
+		}
 
 		seek_forward(extension_size);
 	}
 
-	if (buffer_size != INDEX_FOOTER_SIZE)
-		return index_error_invalid("buffer size does not match index footer size");
+	if (buffer_size != INDEX_FOOTER_SIZE) {
+		error = index_error_invalid(
+			"buffer size does not match index footer size");
+		goto done;
+	}
 
 	/* 160-bit SHA-1 over the content of the index file before this checksum. */
 	git_oid_fromraw(&checksum_expected, (const unsigned char *)buffer);
 
-	if (git_oid__cmp(&checksum_calculated, &checksum_expected) != 0)
-		return index_error_invalid("calculated checksum does not match expected");
+	if (git_oid__cmp(&checksum_calculated, &checksum_expected) != 0) {
+		error = index_error_invalid(
+			"calculated checksum does not match expected");
+		goto done;
+	}
 
 #undef seek_forward
 
@@ -1831,7 +1857,8 @@ static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
 	git_vector_set_sorted(&index->entries, !index->ignore_case);
 	git_vector_sort(&index->entries);
 
-	return 0;
+done:
+	return error;
 }
 
 static bool is_index_extended(git_index *index)
@@ -1916,19 +1943,20 @@ static int write_entries(git_index *index, git_filebuf *file)
 {
 	int error = 0;
 	size_t i;
-	git_vector case_sorted;
+	git_vector case_sorted, *entries;
 	git_index_entry *entry;
-	git_vector *out = &index->entries;
 
 	/* If index->entries is sorted case-insensitively, then we need
 	 * to re-sort it case-sensitively before writing */
 	if (index->ignore_case) {
 		git_vector_dup(&case_sorted, &index->entries, index_cmp);
 		git_vector_sort(&case_sorted);
-		out = &case_sorted;
+		entries = &case_sorted;
+	} else {
+		entries = &index->entries;
 	}
 
-	git_vector_foreach(out, i, entry)
+	git_vector_foreach(entries, i, entry)
 		if ((error = write_disk_entry(file, entry)) < 0)
 			break;
 
@@ -2179,8 +2207,11 @@ int git_index_read_tree(git_index *index, const git_tree *tree)
 
 	if (!error) {
 		git_vector_sort(&entries);
-		git_index_clear(index);
-		git_vector_swap(&entries, &index->entries);
+
+		if ((error = git_index_clear(index)) < 0)
+			/* well, this isn't good */;
+		else
+			git_vector_swap(&entries, &index->entries);
 	}
 
 	git_vector_free(&entries);
@@ -2272,17 +2303,14 @@ int git_index_add_all(
 			break;
 
 		/* make the new entry to insert */
-		if ((entry = index_entry_dup(wd)) == NULL) {
-			error = -1;
+		if ((error = index_entry_dup(&entry, wd)) < 0)
 			break;
-		}
+
 		entry->id = blobid;
 
 		/* add working directory item to index */
-		if ((error = index_insert(index, entry, 1)) < 0) {
-			index_entry_free(entry);
+		if ((error = index_insert(index, &entry, 1)) < 0)
 			break;
-		}
 
 		git_tree_cache_invalidate_path(index->tree, wd->path);
 
diff --git a/src/index.h b/src/index.h
index 17f04f0..cabdbca 100644
--- a/src/index.h
+++ b/src/index.h
@@ -50,11 +50,13 @@ struct git_index_conflict_iterator {
 extern void git_index_entry__init_from_stat(
 	git_index_entry *entry, struct stat *st, bool trust_mode);
 
-extern size_t git_index__prefix_position(git_index *index, const char *path);
-
 extern int git_index_entry__cmp(const void *a, const void *b);
 extern int git_index_entry__cmp_icase(const void *a, const void *b);
 
+/* Search index for `path`, returning GIT_ENOTFOUND if it does not exist.
+ * `at_pos` is set to the position where it is or would be inserted.
+ * Pass `path_len` as strlen of path or 0 to call strlen internally.
+ */
 extern int git_index__find(
 	size_t *at_pos, git_index *index, const char *path, size_t path_len, int stage);
 
diff --git a/src/iterator.c b/src/iterator.c
index a7a4491..4565525 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -815,8 +815,10 @@ static int index_iterator__reset(
 	if (iterator__reset_range(self, start, end) < 0)
 		return -1;
 
-	ii->current = ii->base.start ?
-		git_index__prefix_position(ii->index, ii->base.start) : 0;
+	ii->current = 0;
+
+	if (ii->base.start)
+		git_index__find(&ii->current, ii->index, ii->base.start, 0, 0);
 
 	if ((ie = index_iterator__skip_conflicts(ii)) == NULL)
 		return 0;