Commit 923317db3f5900230063a7715cec29aefdb8a713
2018-09-21T12:57:02
index: modernize error handling of `index_insert` The current error hanling of the function `index_insert` is currently very fragile. Instead of erroring out in case an error has happened, it will instead verify that no error has happened for each statement. This makes adding new code to that function an adventurous task. Improve the situation by converting the function to use our typical `goto out` pattern.
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
diff --git a/src/index.c b/src/index.c
index 465efaa..0e0dfe8 100644
--- a/src/index.c
+++ b/src/index.c
@@ -1337,57 +1337,58 @@ static int index_insert(
bool trust_mode,
bool trust_id)
{
- int error = 0;
- size_t path_length, position;
git_index_entry *existing, *best, *entry;
+ size_t path_length, position;
+ int error;
assert(index && entry_ptr);
entry = *entry_ptr;
- /* make sure that the path length flag is correct */
+ /* Make sure that the path length flag is correct */
path_length = ((struct entry_internal *)entry)->pathlen;
index_entry_adjust_namemask(entry, path_length);
- /* this entry is now up-to-date and should not be checked for raciness */
+ /* This entry is now up-to-date and should not be checked for raciness */
entry->flags_extended |= GIT_IDXENTRY_UPTODATE;
git_vector_sort(&index->entries);
- /* look if an entry with this path already exists, either staged, or (if
+ /*
+ * Look if an entry with this path already exists, either staged, or (if
* this entry is a regular staged item) as the "ours" side of a conflict.
*/
index_existing_and_best(&existing, &position, &best, index, entry);
- /* update the file mode */
+ /* Update the file mode */
entry->mode = trust_mode ?
git_index__create_mode(entry->mode) :
index_merge_mode(index, best, entry->mode);
- /* canonicalize the directory name */
- if (!trust_path)
- error = canonicalize_directory_path(index, entry, best);
+ /* Canonicalize the directory name */
+ if (!trust_path && (error = canonicalize_directory_path(index, entry, best)) < 0)
+ goto out;
- /* ensure that the given id exists (unless it's a submodule) */
- if (!error && !trust_id && INDEX_OWNER(index) &&
- (entry->mode & GIT_FILEMODE_COMMIT) != GIT_FILEMODE_COMMIT) {
+ /* Ensure that the given id exists (unless it's a submodule) */
+ if (!trust_id && INDEX_OWNER(index) &&
+ (entry->mode & GIT_FILEMODE_COMMIT) != GIT_FILEMODE_COMMIT) {
if (!git_object__is_valid(INDEX_OWNER(index), &entry->id,
- git_object__type_from_filemode(entry->mode)))
+ git_object__type_from_filemode(entry->mode))) {
error = -1;
+ goto out;
+ }
}
- /* look for tree / blob name collisions, removing conflicts if requested */
- if (!error)
- error = check_file_directory_collision(index, entry, position, replace);
-
- if (error < 0)
- /* skip changes */;
+ /* Look for tree / blob name collisions, removing conflicts if requested */
+ if ((error = check_file_directory_collision(index, entry, position, replace)) < 0)
+ goto out;
- /* if we are replacing an existing item, overwrite the existing entry
+ /*
+ * If we are replacing an existing item, overwrite the existing entry
* and return it in place of the passed in one.
*/
- else if (existing) {
+ if (existing) {
if (replace) {
index_entry_cpy(existing, entry);
@@ -1396,25 +1397,25 @@ static int index_insert(
}
index_entry_free(entry);
- *entry_ptr = entry = existing;
- }
- else {
- /* if replace is not requested or no existing entry exists, insert
+ *entry_ptr = existing;
+ } else {
+ /*
+ * If replace is not requested or no existing entry exists, insert
* at the sorted position. (Since we re-sort after each insert to
* check for dups, this is actually cheaper in the long run.)
*/
- error = git_vector_insert_sorted(&index->entries, entry, index_no_dups);
+ if ((error = git_vector_insert_sorted(&index->entries, entry, index_no_dups)) < 0)
+ goto out;
- if (error == 0) {
- INSERT_IN_MAP(index, entry, &error);
- }
+ INSERT_IN_MAP(index, entry, &error);
}
+ index->dirty = 1;
+
+out:
if (error < 0) {
index_entry_free(*entry_ptr);
*entry_ptr = NULL;
- } else {
- index->dirty = 1;
}
return error;