We hand over the "referent" string (buffer) to several different
functions, one of which would sometimes keep the raw pointer,
referent.buf. (When split_symref_update calls string_list_insert.) The
previous patch removed that behavior, so we can now safely release the
string buffer before returning.
Signed-off-by: Martin Ågren <martin.ag...@gmail.com>
---
I appreciate Git's stance on memory leaks ("if we're about to exit or
die, why bother?") but this one feels different since
lock_ref_for_update is called in a loop rather deep down the call stack.
Feel free to tell me I'm wrong. Patch 1/2 does introduce some
malloc-churning. An alternative would be to call strbuf_release only
when we know it's safe, but that feels really ugly.
refs/files-backend.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 22daca2ba..6af07c404 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2252,7 +2252,7 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
struct strbuf referent = STRBUF_INIT;
int mustexist = (update->flags & REF_HAVE_OLD) &&
!is_null_oid(&update->old_oid);
- int ret;
+ int ret = 0;
struct ref_lock *lock;
files_assert_main_repository(refs, "lock_ref_for_update");
@@ -2264,7 +2264,7 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
ret = split_head_update(update, transaction, head_ref,
affected_refnames, err);
if (ret)
- return ret;
+ goto out;
}
ret = lock_raw_ref(refs, update->refname, mustexist,
@@ -2278,7 +2278,7 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
strbuf_addf(err, "cannot lock ref '%s': %s",
original_update_refname(update), reason);
free(reason);
- return ret;
+ goto out;
}
update->backend_data = lock;
@@ -2297,10 +2297,12 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
strbuf_addf(err, "cannot lock ref '%s':
"
"error reading reference",
original_update_refname(update));
- return -1;
+ ret = -1;
+ goto out;
}
} else if (check_old_oid(update, &lock->old_oid, err)) {
- return TRANSACTION_GENERIC_ERROR;
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto out;
}
} else {
/*
@@ -2314,13 +2316,15 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
referent.buf, transaction,
affected_refnames, err);
if (ret)
- return ret;
+ goto out;
}
} else {
struct ref_update *parent_update;
- if (check_old_oid(update, &lock->old_oid, err))
- return TRANSACTION_GENERIC_ERROR;
+ if (check_old_oid(update, &lock->old_oid, err)) {
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto out;
+ }
/*
* If this update is happening indirectly because of a
@@ -2357,7 +2361,8 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
"cannot update ref '%s': %s",
update->refname, write_err);
free(write_err);
- return TRANSACTION_GENERIC_ERROR;
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto out;
} else {
update->flags |= REF_NEEDS_COMMIT;
}
@@ -2371,10 +2376,14 @@ static int lock_ref_for_update(struct files_ref_store
*refs,
if (close_ref(lock)) {
strbuf_addf(err, "couldn't close '%s.lock'",
update->refname);
- return TRANSACTION_GENERIC_ERROR;
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto out;
}
}
- return 0;
+
+out:
+ strbuf_release(&referent);
+ return ret;
}
/*
--
2.14.1.151.g45c1275a3.dirty
