Commit 217667028918d1993d22508881e5df8fa2755e6a
2016-06-27T15:20:20
blame: do not decrement commit refcount in make_origin When we create a blame origin, we try to look up the blob that is to be blamed at a certain revision. When this lookup fails, e.g. because the file did not exist at that certain revision, we fail to create the blame origin and return `NULL`. The blame origin that we have just allocated is thereby free'd with `origin_decref`. The `origin_decref` function does not only decrement reference counts for the blame origin, though, but also for its commit and blob. When this is done in the error case, we will cause an uneven reference count for these objects. This may result in hard-to-debug failures at seemingly unrelated code paths, where we try to access these objects when they in fact have already been free'd. Fix the issue by refactoring `make_origin` such that we only allocate the object after the only function that may fail so that we do not have to call `origin_decref` at all. Also fix the `pass_blame` function, which indirectly calls `make_origin`, to free the commit when `make_origin` failed.
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
diff --git a/src/blame_git.c b/src/blame_git.c
index 700207e..96785c7 100644
--- a/src/blame_git.c
+++ b/src/blame_git.c
@@ -37,25 +37,27 @@ static void origin_decref(git_blame__origin *o)
static int make_origin(git_blame__origin **out, git_commit *commit, const char *path)
{
git_blame__origin *o;
+ git_object *blob;
size_t path_len = strlen(path), alloc_len;
int error = 0;
+ if ((error = git_object_lookup_bypath(&blob, (git_object*)commit,
+ path, GIT_OBJ_BLOB)) < 0)
+ return error;
+
GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*o), path_len);
GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1);
o = git__calloc(1, alloc_len);
GITERR_CHECK_ALLOC(o);
o->commit = commit;
+ o->blob = (git_blob *) blob;
o->refcnt = 1;
strcpy(o->path, path);
- if (!(error = git_object_lookup_bypath((git_object**)&o->blob, (git_object*)commit,
- path, GIT_OBJ_BLOB))) {
- *out = o;
- } else {
- origin_decref(o);
- }
- return error;
+ *out = o;
+
+ return 0;
}
/* Locate an existing origin or create a new one. */
@@ -529,8 +531,16 @@ static int pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt)
goto finish;
porigin = find_origin(blame, p, origin);
- if (!porigin)
+ if (!porigin) {
+ /*
+ * We only have to decrement the parent's
+ * reference count when no porigin has
+ * been created, as otherwise the commit
+ * is assigned to the created object.
+ */
+ git_commit_free(p);
continue;
+ }
if (porigin->blob && origin->blob &&
!git_oid_cmp(git_blob_id(porigin->blob), git_blob_id(origin->blob))) {
pass_whole_blame(blame, origin, porigin);