undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'ma/split-symref-update-fix'

Browse Source 
A leakfix.

* ma/split-symref-update-fix:
  refs/files-backend: add `refname`, not "HEAD", to list
  refs/files-backend: correct return value in lock_ref_for_update
  refs/files-backend: fix memory leak in lock_ref_for_update
  refs/files-backend: add longer-scoped copy of string to list
This commit is contained in:
Junio C Hamano 2017-09-19 10:47:53 +09:00
commit dafbe1993e
1 changed files with 44 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
refs/files-backend.c
View File

@ -2099,11 +2099,10 @@ static int split_head_update(struct ref_update *update,
/* /*
* First make sure that HEAD is not already in the * First make sure that HEAD is not already in the
* transaction. This insertion is O(N) in the transaction * transaction. This check is O(lg N) in the transaction
* size, but it happens at most once per transaction. * size, but it happens at most once per transaction.
*/ */
item = string_list_insert(affected_refnames, "HEAD"); if (string_list_has_string(affected_refnames, "HEAD")) {
if (item->util) {
/* An entry already existed */ /* An entry already existed */
strbuf_addf(err, strbuf_addf(err,
"multiple updates for 'HEAD' (including one " "multiple updates for 'HEAD' (including one "
@ -2118,6 +2117,14 @@ static int split_head_update(struct ref_update *update,
update->new_oid.hash, update->old_oid.hash, update->new_oid.hash, update->old_oid.hash,
update->msg); update->msg);
/*
* Add "HEAD". This insertion is O(N) in the transaction
* size, but it happens at most once per transaction.
* Add new_update->refname instead of a literal "HEAD".
*/
if (strcmp(new_update->refname, "HEAD"))
BUG("%s unexpectedly not 'HEAD'", new_update->refname);
item = string_list_insert(affected_refnames, new_update->refname);
item->util = new_update; item->util = new_update;
return 0; return 0;
@ -2144,13 +2151,12 @@ static int split_symref_update(struct files_ref_store *refs,
/* /*
* First make sure that referent is not already in the * First make sure that referent is not already in the
* transaction. This insertion is O(N) in the transaction * transaction. This check is O(lg N) in the transaction
* size, but it happens at most once per symref in a * size, but it happens at most once per symref in a
* transaction. * transaction.
*/ */
item = string_list_insert(affected_refnames, referent); if (string_list_has_string(affected_refnames, referent)) {
if (item->util) { /* An entry already exists */
/* An entry already existed */
strbuf_addf(err, strbuf_addf(err,
"multiple updates for '%s' (including one " "multiple updates for '%s' (including one "
"via symref '%s') are not allowed", "via symref '%s') are not allowed",
@ -2185,6 +2191,17 @@ static int split_symref_update(struct files_ref_store *refs,
update->flags |= REF_LOG_ONLY | REF_NODEREF; update->flags |= REF_LOG_ONLY | REF_NODEREF;
update->flags &= ~REF_HAVE_OLD; update->flags &= ~REF_HAVE_OLD;
/*
* Add the referent. This insertion is O(N) in the transaction
* size, but it happens at most once per symref in a
* transaction. Make sure to add new_update->refname, which will
* be valid as long as affected_refnames is in use, and NOT
* referent, which might soon be freed by our caller.
*/
item = string_list_insert(affected_refnames, new_update->refname);
if (item->util)
BUG("%s unexpectedly found in affected_refnames",
new_update->refname);
item->util = new_update; item->util = new_update;
return 0; return 0;
@ -2256,7 +2273,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
struct strbuf referent = STRBUF_INIT; struct strbuf referent = STRBUF_INIT;
int mustexist = (update->flags & REF_HAVE_OLD) && int mustexist = (update->flags & REF_HAVE_OLD) &&
!is_null_oid(&update->old_oid); !is_null_oid(&update->old_oid);
int ret; int ret = 0;
struct ref_lock *lock; struct ref_lock *lock;
files_assert_main_repository(refs, "lock_ref_for_update"); files_assert_main_repository(refs, "lock_ref_for_update");
@ -2268,7 +2285,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
ret = split_head_update(update, transaction, head_ref, ret = split_head_update(update, transaction, head_ref,
affected_refnames, err); affected_refnames, err);
if (ret) if (ret)
return ret; goto out;
} }
ret = lock_raw_ref(refs, update->refname, mustexist, ret = lock_raw_ref(refs, update->refname, mustexist,
@ -2282,7 +2299,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
strbuf_addf(err, "cannot lock ref '%s': %s", strbuf_addf(err, "cannot lock ref '%s': %s",
original_update_refname(update), reason); original_update_refname(update), reason);
free(reason); free(reason);
return ret; goto out;
} }
update->backend_data = lock; update->backend_data = lock;
@ -2301,10 +2318,12 @@ static int lock_ref_for_update(struct files_ref_store *refs,
strbuf_addf(err, "cannot lock ref '%s': " strbuf_addf(err, "cannot lock ref '%s': "
"error reading reference", "error reading reference",
original_update_refname(update)); original_update_refname(update));
return -1; ret = TRANSACTION_GENERIC_ERROR;
goto out;
} }
} else if (check_old_oid(update, &lock->old_oid, err)) { } else if (check_old_oid(update, &lock->old_oid, err)) {
return TRANSACTION_GENERIC_ERROR; ret = TRANSACTION_GENERIC_ERROR;
goto out;
} }
} else { } else {
/* /*
@ -2318,13 +2337,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,
referent.buf, transaction, referent.buf, transaction,
affected_refnames, err); affected_refnames, err);
if (ret) if (ret)
return ret; goto out;
} }
} else { } else {
struct ref_update *parent_update; struct ref_update *parent_update;
if (check_old_oid(update, &lock->old_oid, err)) if (check_old_oid(update, &lock->old_oid, err)) {
return TRANSACTION_GENERIC_ERROR; ret = TRANSACTION_GENERIC_ERROR;
goto out;
}
/* /*
* If this update is happening indirectly because of a * If this update is happening indirectly because of a
@ -2361,7 +2382,8 @@ static int lock_ref_for_update(struct files_ref_store *refs,
"cannot update ref '%s': %s", "cannot update ref '%s': %s",
update->refname, write_err); update->refname, write_err);
free(write_err); free(write_err);
return TRANSACTION_GENERIC_ERROR; ret = TRANSACTION_GENERIC_ERROR;
goto out;
} else { } else {
update->flags |= REF_NEEDS_COMMIT; update->flags |= REF_NEEDS_COMMIT;
} }
@ -2375,10 +2397,14 @@ static int lock_ref_for_update(struct files_ref_store *refs,
if (close_ref(lock)) { if (close_ref(lock)) {
strbuf_addf(err, "couldn't close '%s.lock'", strbuf_addf(err, "couldn't close '%s.lock'",
update->refname); update->refname);
return TRANSACTION_GENERIC_ERROR; ret = TRANSACTION_GENERIC_ERROR;
goto out;
} }
} }
return 0;
out:
strbuf_release(&referent);
return ret;
} }
/* /*