proxmox-mirrors/libgit2
1
0
Fork 0
mirror of https://git.proxmox.com/git/libgit2 synced 2025-05-09 22:39:38 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Add tests and fix use of freed memory

Browse Source 
This adds some tests for updating the index and having it remove
items to make sure that the iteration over the index still works
even as earlier items are removed.

In testing with valgrind, this found a path that would use the
path string from the index entry after it had been freed.  The
bug fix is simply to copy the path of the index entry before
doing any actual index manipulation.
This commit is contained in:
Russell Belfer 2013-06-19 15:54:19 -07:00
parent f30fff45a7
commit 7863523a1b
2 changed files with 29 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/index.c
View File

@ -2176,6 +2176,7 @@ static int index_apply_to_all(
size_t i; size_t i;
git_pathspec_context ps; git_pathspec_context ps;
const char *match; const char *match;
git_buf path = GIT_BUF_INIT;
assert(index); assert(index);
@ -2205,23 +2206,27 @@ static int index_apply_to_all(
} }
} }
/* index manipulation may alter entry, so don't depend on it */
if ((error = git_buf_sets(&path, entry->path)) < 0)
break;
switch (action) { switch (action) {
case INDEX_ACTION_NONE: case INDEX_ACTION_NONE:
break; break;
case INDEX_ACTION_UPDATE: case INDEX_ACTION_UPDATE:
error = git_index_add_bypath(index, entry->path); error = git_index_add_bypath(index, path.ptr);
if (error == GIT_ENOTFOUND) { if (error == GIT_ENOTFOUND) {
giterr_clear(); giterr_clear();
error = git_index_remove_bypath(index, entry->path); error = git_index_remove_bypath(index, path.ptr);
if (!error) /* back up foreach if we removed this */ if (!error) /* back up foreach if we removed this */
i--; i--;
} }
break; break;
case INDEX_ACTION_REMOVE: case INDEX_ACTION_REMOVE:
if (!(error = git_index_remove_bypath(index, entry->path))) if (!(error = git_index_remove_bypath(index, path.ptr)))
i--; /* back up foreach if we removed this */ i--; /* back up foreach if we removed this */
break; break;
default: default:
@ -2231,6 +2236,7 @@ static int index_apply_to_all(
} }
} }
git_buf_free(&path);
git_pathspec_context_free(&ps); git_pathspec_context_free(&ps);
return error; return error;

20
tests-clar/index/addall.c
View File

@ -132,6 +132,7 @@ static void commit_index_to_head(
cl_git_pass(git_repository_index(&index, repo)); cl_git_pass(git_repository_index(&index, repo));
cl_git_pass(git_index_write_tree(&tree_id, index)); cl_git_pass(git_index_write_tree(&tree_id, index));
cl_git_pass(git_index_write(index)); /* not needed, but might as well */
git_index_free(index); git_index_free(index);
cl_git_pass(git_tree_lookup(&tree, repo, &tree_id)); cl_git_pass(git_tree_lookup(&tree, repo, &tree_id));
@ -250,5 +251,24 @@ void test_index_addall__repo_lifecycle(void)
cl_git_pass(git_index_update_all(index, NULL, NULL, NULL)); cl_git_pass(git_index_update_all(index, NULL, NULL, NULL));
check_status(g_repo, 0, 1, 0, 3, 0, 0, 0); check_status(g_repo, 0, 1, 0, 3, 0, 0, 0);
strs[0] = "*";
cl_git_pass(git_index_add_all(index, &paths, 0, NULL, NULL));
check_status(g_repo, 3, 1, 0, 0, 0, 0, 0);
/* must be able to remove at any position while still updating other files */
cl_must_pass(p_unlink("addall/.gitignore"));
cl_git_rewritefile("addall/file.zzz", "reconstructed file");
cl_git_rewritefile("addall/more.zzz", "altered file reality");
check_status(g_repo, 3, 1, 0, 1, 1, 1, 0);
cl_git_pass(git_index_update_all(index, NULL, NULL, NULL));
check_status(g_repo, 2, 1, 0, 1, 0, 0, 0);
/* this behavior actually matches 'git add -u' where "file.zzz" has
* been removed from the index, so when you go to update, even though
* it exists in the HEAD, it is not re-added to the index, leaving it
* as a DELETE when comparing HEAD to index and as an ADD comparing
* index to worktree
*/
git_index_free(index); git_index_free(index);
} }