diff options
author | Ronnie Sahlberg <sahlberg@google.com> | 2014-04-17 11:31:06 -0700 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2014-06-12 13:39:31 -0700 |
commit | e63e32695068b6875bb924c331e76e577702dee4 (patch) | |
tree | 9002b1f298e81f3e0b1be95587ed65d99e36eb8d | |
parent | 41080329ce3510722fd02f2b596e447465d5c27b (diff) | |
download | git-e63e32695068b6875bb924c331e76e577702dee4.tar.gz |
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 collissions that the previous locking would
protect against and cause the fetch to fail for to be even more rare.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
-rw-r--r-- | walker.c | 59 |
1 files changed, 35 insertions, 24 deletions
@@ -251,39 +251,36 @@ void walker_targets_free(int targets, char **target, const char **write_ref) int walker_fetch(struct walker *walker, int targets, char **target, const char **write_ref, const char *write_ref_log_details) { - struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *)); + struct strbuf ref_name = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + struct ref_transaction *transaction = NULL; unsigned char *sha1 = xmalloc(targets * 20); - char *msg; - int ret; + char *msg = NULL; int i; save_commit_buffer = 0; - for (i = 0; i < targets; i++) { - if (!write_ref || !write_ref[i]) - continue; - - lock[i] = lock_ref_sha1(write_ref[i], NULL); - if (!lock[i]) { - error("Can't lock ref %s", write_ref[i]); - goto unlock_and_fail; + if (write_ref) { + transaction = ref_transaction_begin(&err); + if (!transaction) { + error("%s", err.buf); + goto rollback_and_fail; } } - if (!walker->get_recover) for_each_ref(mark_complete, NULL); for (i = 0; i < targets; i++) { if (interpret_target(walker, target[i], &sha1[20 * i])) { error("Could not interpret response from server '%s' as something to pull", target[i]); - goto unlock_and_fail; + goto rollback_and_fail; } if (process(walker, lookup_unknown_object(&sha1[20 * i]))) - goto unlock_and_fail; + goto rollback_and_fail; } if (loop(walker)) - goto unlock_and_fail; + goto rollback_and_fail; if (write_ref_log_details) { msg = xmalloc(strlen(write_ref_log_details) + 12); @@ -294,19 +291,33 @@ int walker_fetch(struct walker *walker, int targets, char **target, for (i = 0; i < targets; i++) { if (!write_ref || !write_ref[i]) continue; - ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)"); - lock[i] = NULL; - if (ret) - goto unlock_and_fail; + strbuf_reset(&ref_name); + strbuf_addf(&ref_name, "refs/%s", write_ref[i]); + if (ref_transaction_update(transaction, ref_name.buf, + &sha1[20 * i], NULL, 0, 0, + &err)) { + error("%s", err.buf); + goto rollback_and_fail; + } + } + if (write_ref) { + if (ref_transaction_commit(transaction, + msg ? msg : "fetch (unknown)", + &err)) { + error("%s", err.buf); + goto rollback_and_fail; + } + ref_transaction_free(transaction); } - free(msg); + free(msg); return 0; -unlock_and_fail: - for (i = 0; i < targets; i++) - if (lock[i]) - unlock_ref(lock[i]); +rollback_and_fail: + ref_transaction_free(transaction); + free(msg); + strbuf_release(&err); + strbuf_release(&ref_name); return -1; } |