walker.c: use ref transaction for ref updates

Switch to using ref transactions in walker_fetch(). As part of the refactoring
to use ref transactions we also fix a potential memory leak where in the
original code if write_ref_sha1() would fail we would end up returning from
the function without free()ing the msg string.

Note that this function is only called when fetching from a remote HTTP
repository onto the local (most of the time single-user) repository which
likely means that the type of collisions that the previous locking would
protect against and cause the fetch to fail for are even more rare.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Ronnie Sahlberg
2014-04-17 11:31:06 -07:00
committed by Junio C Hamano
parent 3f09ba7543
commit b6b10bb44c

View File

@ -251,40 +251,40 @@ void walker_targets_free(int targets, char **target, const char **write_ref)
int walker_fetch(struct walker *walker, int targets, char **target, int walker_fetch(struct walker *walker, int targets, char **target,
const char **write_ref, const char *write_ref_log_details) const char **write_ref, const char *write_ref_log_details)
{ {
struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *)); struct strbuf refname = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
struct ref_transaction *transaction = NULL;
unsigned char *sha1 = xmalloc(targets * 20); unsigned char *sha1 = xmalloc(targets * 20);
char *msg; char *msg = NULL;
int ret; int i, ret = -1;
int i;
save_commit_buffer = 0; save_commit_buffer = 0;
for (i = 0; i < targets; i++) { if (write_ref) {
if (!write_ref || !write_ref[i]) transaction = ref_transaction_begin(&err);
continue; if (!transaction) {
error("%s", err.buf);
lock[i] = lock_ref_sha1(write_ref[i], NULL); goto done;
if (!lock[i]) {
error("Can't lock ref %s", write_ref[i]);
goto unlock_and_fail;
} }
} }
if (!walker->get_recover) if (!walker->get_recover)
for_each_ref(mark_complete, NULL); for_each_ref(mark_complete, NULL);
for (i = 0; i < targets; i++) { for (i = 0; i < targets; i++) {
if (interpret_target(walker, target[i], &sha1[20 * i])) { if (interpret_target(walker, target[i], &sha1[20 * i])) {
error("Could not interpret response from server '%s' as something to pull", target[i]); error("Could not interpret response from server '%s' as something to pull", target[i]);
goto unlock_and_fail; goto done;
} }
if (process(walker, lookup_unknown_object(&sha1[20 * i]))) if (process(walker, lookup_unknown_object(&sha1[20 * i])))
goto unlock_and_fail; goto done;
} }
if (loop(walker)) if (loop(walker))
goto unlock_and_fail; goto done;
if (!write_ref) {
ret = 0;
goto done;
}
if (write_ref_log_details) { if (write_ref_log_details) {
msg = xmalloc(strlen(write_ref_log_details) + 12); msg = xmalloc(strlen(write_ref_log_details) + 12);
sprintf(msg, "fetch from %s", write_ref_log_details); sprintf(msg, "fetch from %s", write_ref_log_details);
@ -292,23 +292,33 @@ int walker_fetch(struct walker *walker, int targets, char **target,
msg = NULL; msg = NULL;
} }
for (i = 0; i < targets; i++) { for (i = 0; i < targets; i++) {
if (!write_ref || !write_ref[i]) if (!write_ref[i])
continue; continue;
ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)"); strbuf_reset(&refname);
lock[i] = NULL; strbuf_addf(&refname, "refs/%s", write_ref[i]);
if (ret) if (ref_transaction_update(transaction, refname.buf,
goto unlock_and_fail; &sha1[20 * i], NULL, 0, 0,
&err)) {
error("%s", err.buf);
goto done;
} }
}
if (ref_transaction_commit(transaction,
msg ? msg : "fetch (unknown)",
&err)) {
error("%s", err.buf);
goto done;
}
ret = 0;
done:
ref_transaction_free(transaction);
free(msg); free(msg);
free(sha1);
return 0; strbuf_release(&err);
strbuf_release(&refname);
unlock_and_fail: return ret;
for (i = 0; i < targets; i++)
if (lock[i])
unlock_ref(lock[i]);
return -1;
} }
void walker_free(struct walker *walker) void walker_free(struct walker *walker)