{"thread":{"id":"37127","subject":"[PATCH v21 02/19] refs.c: ref_transaction_commit should not free the transaction","startedAt":"2014-07-14T20:58:42Z","lastAt":"2014-07-14T20:59:01Z","messageCount":20,"participants":["Ronnie Sahlberg"],"isPatch":true,"patchVersion":21,"patchTotal":19},"messages":[{"id":"246020","messageId":"1405371541-32121-1-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":null,"subject":"[PATCH v21 00/19] Use ref transactions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:42Z","receivedAt":"2014-07-14T20:58:42Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"List, Jun\n\nHere is a resend of the first part of the ref transactions patches.\nThis new series consists of the first 19 patches in the series and\nhave been reviewed by mhagger.\nThis series should be good to go now.\n\nI will resend the remaining part of the series separately once I have addressed\nMichaels concerns.\n\n\nVersion 21:\n -- split the patch series in two. These are the first 19 patches that\n    are now reviewed by mhagger.\n\nRonnie Sahlberg (19):\n  refs.c: remove ref_transaction_rollback\n  refs.c: ref_transaction_commit should not free the transaction\n  refs.c: constify the sha arguments for\n    ref_transaction_create|delete|update\n  refs.c: allow passing NULL to ref_transaction_free\n  refs.c: add a strbuf argument to ref_transaction_commit for error\n    logging\n  lockfile.c: add a new public function unable_to_lock_message\n  lockfile.c: make lock_file return a meaningful errno on failure\n  refs.c: add an err argument to repack_without_refs\n  refs.c: make sure log_ref_setup returns a meaningful errno\n  refs.c: verify_lock should set errno to something meaningful\n  refs.c: make remove_empty_directories always set errno to something\n    sane\n  refs.c: commit_packed_refs to return a meaningful errno on failure\n  refs.c: make resolve_ref_unsafe set errno to something meaningful on\n    error\n  refs.c: log_ref_write should try to return meaningful errno\n  refs.c: make ref_update_reject_duplicates take a strbuf argument for\n    errors\n  refs.c: make update_ref_write update a strbuf on failure\n  update-ref: use err argument to get error from ref_transaction_commit\n  refs.c: remove the onerr argument to ref_transaction_commit\n  refs.c: change ref_transaction_update() to do error checking and\n    return status\n\n builtin/remote.c     |   5 +-\n builtin/update-ref.c |  20 +++---\n cache.h              |   4 +-\n lockfile.c           |  39 ++++++-----\n refs.c               | 178 ++++++++++++++++++++++++++++++++++++---------------\n refs.h               |  53 +++++++++------\n 6 files changed, 199 insertions(+), 100 deletions(-)\n\n-- \n2.0.1.546.gf603308\n"},{"id":"246012","messageId":"1405371541-32121-2-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 01/19] refs.c: remove ref_transaction_rollback","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:43Z","receivedAt":"2014-07-14T20:58:43Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We do not yet need both a rollback and a free function for transactions.\nRemove ref_transaction_rollback and use ref_transaction_free instead.\n\nAt a later stage we may reintroduce a rollback function if we want to start\nadding reusable transactions and similar.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c |  7 +------\n refs.h | 16 +++++++---------\n 2 files changed, 8 insertions(+), 15 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex dc45774..6d841a0 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3334,7 +3334,7 @@ struct ref_transaction *ref_transaction_begin(void)\n \treturn xcalloc(1, sizeof(struct ref_transaction));\n }\n \n-static void ref_transaction_free(struct ref_transaction *transaction)\n+void ref_transaction_free(struct ref_transaction *transaction)\n {\n \tint i;\n \n@@ -3345,11 +3345,6 @@ static void ref_transaction_free(struct ref_transaction *transaction)\n \tfree(transaction);\n }\n \n-void ref_transaction_rollback(struct ref_transaction *transaction)\n-{\n-\tref_transaction_free(transaction);\n-}\n-\n static struct ref_update *add_update(struct ref_transaction *transaction,\n \t\t\t\t     const char *refname)\n {\ndiff --git a/refs.h b/refs.h\nindex 4e3050d..cfd1832 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -219,18 +219,12 @@ enum action_on_err {\n \n /*\n  * Begin a reference transaction.  The reference transaction must\n- * eventually be commited using ref_transaction_commit() or rolled\n- * back using ref_transaction_rollback().\n+ * eventually be commited using ref_transaction_commit() or freed by\n+ * calling ref_transaction_free().\n  */\n struct ref_transaction *ref_transaction_begin(void);\n \n /*\n- * Roll back a ref_transaction and free all associated data.\n- */\n-void ref_transaction_rollback(struct ref_transaction *transaction);\n-\n-\n-/*\n  * The following functions add a reference check or update to a\n  * ref_transaction.  In all of them, refname is the name of the\n  * reference to be affected.  The functions make internal copies of\n@@ -238,7 +232,6 @@ void ref_transaction_rollback(struct ref_transaction *transaction);\n  * can be REF_NODEREF; it is passed to update_ref_lock().\n  */\n \n-\n /*\n  * Add a reference update to transaction.  new_sha1 is the value that\n  * the reference should have after the update, or zeros if it should\n@@ -280,6 +273,11 @@ void ref_transaction_delete(struct ref_transaction *transaction,\n int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t   const char *msg, enum action_on_err onerr);\n \n+/*\n+ * Free an existing transaction and all associated data.\n+ */\n+void ref_transaction_free(struct ref_transaction *transaction);\n+\n /** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n \t\tconst unsigned char *sha1, const unsigned char *oldval,\n-- \n2.0.1.546.gf603308\n"},{"id":"246010","messageId":"1405371541-32121-3-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 02/19] refs.c: ref_transaction_commit should not free the transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:44Z","receivedAt":"2014-07-14T20:58:44Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c | 1 +\n refs.c               | 1 -\n refs.h               | 5 ++---\n 3 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 405267f..1fd7a89 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -369,6 +369,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tupdate_refs_stdin();\n \t\tret = ref_transaction_commit(transaction, msg,\n \t\t\t\t\t     UPDATE_REFS_DIE_ON_ERR);\n+\t\tref_transaction_free(transaction);\n \t\treturn ret;\n \t}\n \ndiff --git a/refs.c b/refs.c\nindex 6d841a0..d9cac6d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3509,7 +3509,6 @@ cleanup:\n \t\tif (updates[i]->lock)\n \t\t\tunlock_ref(updates[i]->lock);\n \tfree(delnames);\n-\tref_transaction_free(transaction);\n \treturn ret;\n }\n \ndiff --git a/refs.h b/refs.h\nindex cfd1832..7d946f6 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -219,8 +219,7 @@ enum action_on_err {\n \n /*\n  * Begin a reference transaction.  The reference transaction must\n- * eventually be commited using ref_transaction_commit() or freed by\n- * calling ref_transaction_free().\n+ * be freed by calling ref_transaction_free().\n  */\n struct ref_transaction *ref_transaction_begin(void);\n \n@@ -268,7 +267,7 @@ void ref_transaction_delete(struct ref_transaction *transaction,\n /*\n  * Commit all of the changes that have been queued in transaction, as\n  * atomically as possible.  Return a nonzero value if there is a\n- * problem.  The ref_transaction is freed by this function.\n+ * problem.\n  */\n int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t   const char *msg, enum action_on_err onerr);\n-- \n2.0.1.546.gf603308\n"},{"id":"246011","messageId":"1405371541-32121-4-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 03/19] refs.c: constify the sha arguments for ref_transaction_create|delete|update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:45Z","receivedAt":"2014-07-14T20:58:45Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"ref_transaction_create|delete|update has no need to modify the sha1\narguments passed to it so it should use const unsigned char* instead\nof unsigned char*.\n\nSome functions, such as fast_forward_to(), already have its old/new\nsha1 arguments as consts. This function will at some point need to\nuse ref_transaction_update() in which case this change is required.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 7 ++++---\n refs.h | 7 ++++---\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d9cac6d..21ed465 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3359,7 +3359,8 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n \n void ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t    const char *refname,\n-\t\t\t    unsigned char *new_sha1, unsigned char *old_sha1,\n+\t\t\t    const unsigned char *new_sha1,\n+\t\t\t    const unsigned char *old_sha1,\n \t\t\t    int flags, int have_old)\n {\n \tstruct ref_update *update = add_update(transaction, refname);\n@@ -3373,7 +3374,7 @@ void ref_transaction_update(struct ref_transaction *transaction,\n \n void ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t    const char *refname,\n-\t\t\t    unsigned char *new_sha1,\n+\t\t\t    const unsigned char *new_sha1,\n \t\t\t    int flags)\n {\n \tstruct ref_update *update = add_update(transaction, refname);\n@@ -3387,7 +3388,7 @@ void ref_transaction_create(struct ref_transaction *transaction,\n \n void ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t    const char *refname,\n-\t\t\t    unsigned char *old_sha1,\n+\t\t\t    const unsigned char *old_sha1,\n \t\t\t    int flags, int have_old)\n {\n \tstruct ref_update *update = add_update(transaction, refname);\ndiff --git a/refs.h b/refs.h\nindex 7d946f6..8c7f9c4 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -240,7 +240,8 @@ struct ref_transaction *ref_transaction_begin(void);\n  */\n void ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t    const char *refname,\n-\t\t\t    unsigned char *new_sha1, unsigned char *old_sha1,\n+\t\t\t    const unsigned char *new_sha1,\n+\t\t\t    const unsigned char *old_sha1,\n \t\t\t    int flags, int have_old);\n \n /*\n@@ -251,7 +252,7 @@ void ref_transaction_update(struct ref_transaction *transaction,\n  */\n void ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t    const char *refname,\n-\t\t\t    unsigned char *new_sha1,\n+\t\t\t    const unsigned char *new_sha1,\n \t\t\t    int flags);\n \n /*\n@@ -261,7 +262,7 @@ void ref_transaction_create(struct ref_transaction *transaction,\n  */\n void ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t    const char *refname,\n-\t\t\t    unsigned char *old_sha1,\n+\t\t\t    const unsigned char *old_sha1,\n \t\t\t    int flags, int have_old);\n \n /*\n-- \n2.0.1.546.gf603308\n"},{"id":"246019","messageId":"1405371541-32121-5-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 04/19] refs.c: allow passing NULL to ref_transaction_free","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:46Z","receivedAt":"2014-07-14T20:58:46Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Allow ref_transaction_free(NULL) as a no-op. This makes ref_transaction_free\neasier to use and more similar to plain 'free'.\n\nIn particular, it lets us rollback unconditionally as part of cleanup code\nafter setting 'transaction = NULL' if a transaction has been committed or\nrolled back already.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 21ed465..1d6dece 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3338,6 +3338,9 @@ void ref_transaction_free(struct ref_transaction *transaction)\n {\n \tint i;\n \n+\tif (!transaction)\n+\t\treturn;\n+\n \tfor (i = 0; i < transaction->nr; i++)\n \t\tfree(transaction->updates[i]);\n \n-- \n2.0.1.546.gf603308\n"},{"id":"246018","messageId":"1405371541-32121-6-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 05/19] refs.c: add a strbuf argument to ref_transaction_commit for error logging","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:47Z","receivedAt":"2014-07-14T20:58:47Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a strbuf argument to _commit so that we can pass an error string back to\nthe caller. So that we can do error logging from the caller instead of from\n_commit.\n\nLonger term plan is to first convert all callers to use onerr==QUIET_ON_ERR\nand craft any log messages from the callers themselves and finally remove the\nonerr argument completely.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c | 2 +-\n refs.c               | 6 +++++-\n refs.h               | 5 ++++-\n 3 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 1fd7a89..22617af 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -367,7 +367,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tret = ref_transaction_commit(transaction, msg,\n+\t\tret = ref_transaction_commit(transaction, msg, NULL,\n \t\t\t\t\t     UPDATE_REFS_DIE_ON_ERR);\n \t\tref_transaction_free(transaction);\n \t\treturn ret;\ndiff --git a/refs.c b/refs.c\nindex 1d6dece..db05602 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3444,7 +3444,8 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t   const char *msg, enum action_on_err onerr)\n+\t\t\t   const char *msg, struct strbuf *err,\n+\t\t\t   enum action_on_err onerr)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3473,6 +3474,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t       update->flags,\n \t\t\t\t\t       &update->type, onerr);\n \t\tif (!update->lock) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n+\t\t\t\t\t    update->refname);\n \t\t\tret = 1;\n \t\t\tgoto cleanup;\n \t\t}\ndiff --git a/refs.h b/refs.h\nindex 8c7f9c4..09d3564 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -269,9 +269,12 @@ void ref_transaction_delete(struct ref_transaction *transaction,\n  * Commit all of the changes that have been queued in transaction, as\n  * atomically as possible.  Return a nonzero value if there is a\n  * problem.\n+ * If err is non-NULL we will add an error string to it to explain why\n+ * the transaction failed. The string does not end in newline.\n  */\n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t   const char *msg, enum action_on_err onerr);\n+\t\t\t   const char *msg, struct strbuf *err,\n+\t\t\t   enum action_on_err onerr);\n \n /*\n  * Free an existing transaction and all associated data.\n-- \n2.0.1.546.gf603308\n"},{"id":"246016","messageId":"1405371541-32121-7-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 06/19] lockfile.c: add a new public function unable_to_lock_message","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:48Z","receivedAt":"2014-07-14T20:58:48Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Introducing a new unable_to_lock_message helper, which has nicer\nsemantics than unable_to_lock_error and cleans up lockfile.c a little.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h    |  2 ++\n lockfile.c | 22 ++++++++++++----------\n 2 files changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex df65231..8a46294 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -559,6 +559,8 @@ struct lock_file {\n #define LOCK_DIE_ON_ERROR 1\n #define LOCK_NODEREF 2\n extern int unable_to_lock_error(const char *path, int err);\n+extern void unable_to_lock_message(const char *path, int err,\n+\t\t\t\t   struct strbuf *buf);\n extern NORETURN void unable_to_lock_index_die(const char *path, int err);\n extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);\n extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);\ndiff --git a/lockfile.c b/lockfile.c\nindex 8fbcb6a..464031b 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -157,33 +157,35 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \treturn lk->fd;\n }\n \n-static char *unable_to_lock_message(const char *path, int err)\n+void unable_to_lock_message(const char *path, int err, struct strbuf *buf)\n {\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n \tif (err == EEXIST) {\n-\t\tstrbuf_addf(&buf, \"Unable to create '%s.lock': %s.\\n\\n\"\n+\t\tstrbuf_addf(buf, \"Unable to create '%s.lock': %s.\\n\\n\"\n \t\t    \"If no other git process is currently running, this probably means a\\n\"\n \t\t    \"git process crashed in this repository earlier. Make sure no other git\\n\"\n \t\t    \"process is running and remove the file manually to continue.\",\n \t\t\t    absolute_path(path), strerror(err));\n \t} else\n-\t\tstrbuf_addf(&buf, \"Unable to create '%s.lock': %s\",\n+\t\tstrbuf_addf(buf, \"Unable to create '%s.lock': %s\",\n \t\t\t    absolute_path(path), strerror(err));\n-\treturn strbuf_detach(&buf, NULL);\n }\n \n int unable_to_lock_error(const char *path, int err)\n {\n-\tchar *msg = unable_to_lock_message(path, err);\n-\terror(\"%s\", msg);\n-\tfree(msg);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tunable_to_lock_message(path, err, &buf);\n+\terror(\"%s\", buf.buf);\n+\tstrbuf_release(&buf);\n \treturn -1;\n }\n \n NORETURN void unable_to_lock_index_die(const char *path, int err)\n {\n-\tdie(\"%s\", unable_to_lock_message(path, err));\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tunable_to_lock_message(path, err, &buf);\n+\tdie(\"%s\", buf.buf);\n }\n \n int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)\n-- \n2.0.1.546.gf603308\n"},{"id":"246013","messageId":"1405371541-32121-8-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 07/19] lockfile.c: make lock_file return a meaningful errno on failure","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:49Z","receivedAt":"2014-07-14T20:58:49Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno when returning from lock_file() meaningful, which should\nfix\n\n * an existing almost-bug in lock_ref_sha1_basic where it assumes\n   errno==ENOENT is meaningful and could waste some work on retries\n\n * an existing bug in repack_without_refs where it prints\n   strerror(errno) and picks advice based on errno, despite errno\n   potentially being zero and potentially having been clobbered by\n   that point\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n lockfile.c | 17 ++++++++++++-----\n refs.c     |  1 +\n refs.h     |  1 +\n 3 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/lockfile.c b/lockfile.c\nindex 464031b..a921d77 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -121,7 +121,7 @@ static char *resolve_symlink(char *p, size_t s)\n \treturn p;\n }\n \n-\n+/* Make sure errno contains a meaningful value on error */\n static int lock_file(struct lock_file *lk, const char *path, int flags)\n {\n \t/*\n@@ -130,8 +130,10 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \t */\n \tstatic const size_t max_path_len = sizeof(lk->filename) - 5;\n \n-\tif (strlen(path) >= max_path_len)\n+\tif (strlen(path) >= max_path_len) {\n+\t\terrno = ENAMETOOLONG;\n \t\treturn -1;\n+\t}\n \tstrcpy(lk->filename, path);\n \tif (!(flags & LOCK_NODEREF))\n \t\tresolve_symlink(lk->filename, max_path_len);\n@@ -148,9 +150,13 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \t\t\tlock_file_list = lk;\n \t\t\tlk->on_list = 1;\n \t\t}\n-\t\tif (adjust_shared_perm(lk->filename))\n-\t\t\treturn error(\"cannot fix permission bits on %s\",\n-\t\t\t\t     lk->filename);\n+\t\tif (adjust_shared_perm(lk->filename)) {\n+\t\t\tint save_errno = errno;\n+\t\t\terror(\"cannot fix permission bits on %s\",\n+\t\t\t      lk->filename);\n+\t\t\terrno = save_errno;\n+\t\t\treturn -1;\n+\t\t}\n \t}\n \telse\n \t\tlk->filename[0] = 0;\n@@ -188,6 +194,7 @@ NORETURN void unable_to_lock_index_die(const char *path, int err)\n \tdie(\"%s\", buf.buf);\n }\n \n+/* This should return a meaningful errno on failure */\n int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)\n {\n \tint fd = lock_file(lk, path, flags);\ndiff --git a/refs.c b/refs.c\nindex db05602..e9d53e4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2212,6 +2212,7 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)\n \treturn 0;\n }\n \n+/* This should return a meaningful errno on failure */\n int lock_packed_refs(int flags)\n {\n \tstruct packed_ref_cache *packed_ref_cache;\ndiff --git a/refs.h b/refs.h\nindex 09d3564..64f25d9 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -82,6 +82,7 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st\n /*\n  * Lock the packed-refs file for writing.  Flags is passed to\n  * hold_lock_file_for_update().  Return 0 on success.\n+ * Errno is set to something meaningful on error.\n  */\n extern int lock_packed_refs(int flags);\n \n-- \n2.0.1.546.gf603308\n"},{"id":"246015","messageId":"1405371541-32121-9-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 08/19] refs.c: add an err argument to repack_without_refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:50Z","receivedAt":"2014-07-14T20:58:50Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update repack_without_refs to take an err argument and update it if there\nis a failure. Pass the err variable from ref_transaction_commit to this\nfunction so that callers can print a meaningful error message if _commit\nfails due to this function.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/remote.c |  5 +++--\n refs.c           | 19 ++++++++++++++-----\n refs.h           |  3 ++-\n 3 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex c9102e8..401feb3 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -755,7 +755,7 @@ static int remove_branches(struct string_list *branches)\n \tbranch_names = xmalloc(branches->nr * sizeof(*branch_names));\n \tfor (i = 0; i < branches->nr; i++)\n \t\tbranch_names[i] = branches->items[i].string;\n-\tresult |= repack_without_refs(branch_names, branches->nr);\n+\tresult |= repack_without_refs(branch_names, branches->nr, NULL);\n \tfree(branch_names);\n \n \tfor (i = 0; i < branches->nr; i++) {\n@@ -1333,7 +1333,8 @@ static int prune_remote(const char *remote, int dry_run)\n \t\tfor (i = 0; i < states.stale.nr; i++)\n \t\t\tdelete_refs[i] = states.stale.items[i].util;\n \t\tif (!dry_run)\n-\t\t\tresult |= repack_without_refs(delete_refs, states.stale.nr);\n+\t\t\tresult |= repack_without_refs(delete_refs,\n+\t\t\t\t\t\t      states.stale.nr, NULL);\n \t\tfree(delete_refs);\n \t}\n \ndiff --git a/refs.c b/refs.c\nindex e9d53e4..67a0217 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2456,12 +2456,12 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)\n \treturn 0;\n }\n \n-int repack_without_refs(const char **refnames, int n)\n+int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n {\n \tstruct ref_dir *packed;\n \tstruct string_list refs_to_delete = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *ref_to_delete;\n-\tint i, removed = 0;\n+\tint i, ret, removed = 0;\n \n \t/* Look for a packed ref */\n \tfor (i = 0; i < n; i++)\n@@ -2473,6 +2473,11 @@ int repack_without_refs(const char **refnames, int n)\n \t\treturn 0; /* no refname exists in packed refs */\n \n \tif (lock_packed_refs(0)) {\n+\t\tif (err) {\n+\t\t\tunable_to_lock_message(git_path(\"packed-refs\"), errno,\n+\t\t\t\t\t       err);\n+\t\t\treturn -1;\n+\t\t}\n \t\tunable_to_lock_error(git_path(\"packed-refs\"), errno);\n \t\treturn error(\"cannot delete '%s' from packed refs\", refnames[i]);\n \t}\n@@ -2499,12 +2504,16 @@ int repack_without_refs(const char **refnames, int n)\n \t}\n \n \t/* Write what remains */\n-\treturn commit_packed_refs();\n+\tret = commit_packed_refs();\n+\tif (ret && err)\n+\t\tstrbuf_addf(err, \"unable to overwrite old ref-pack file: %s\",\n+\t\t\t    strerror(errno));\n+\treturn ret;\n }\n \n static int repack_without_ref(const char *refname)\n {\n-\treturn repack_without_refs(&refname, 1);\n+\treturn repack_without_refs(&refname, 1, NULL);\n }\n \n static int delete_ref_loose(struct ref_lock *lock, int flag)\n@@ -3508,7 +3517,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\tret |= repack_without_refs(delnames, delnum);\n+\tret |= repack_without_refs(delnames, delnum, err);\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n \tclear_loose_ref_cache(&ref_cache);\ndiff --git a/refs.h b/refs.h\nindex 64f25d9..65f7637 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -122,7 +122,8 @@ extern void rollback_packed_refs(void);\n  */\n int pack_refs(unsigned int flags);\n \n-extern int repack_without_refs(const char **refnames, int n);\n+extern int repack_without_refs(const char **refnames, int n,\n+\t\t\t       struct strbuf *err);\n \n extern int ref_exists(const char *);\n \n-- \n2.0.1.546.gf603308\n"},{"id":"246014","messageId":"1405371541-32121-10-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 09/19] refs.c: make sure log_ref_setup returns a meaningful errno","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:51Z","receivedAt":"2014-07-14T20:58:51Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno when returning from log_ref_setup() meaningful,\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 27 +++++++++++++++++++--------\n refs.h |  4 +++-\n 2 files changed, 22 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 67a0217..9ea519c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2751,6 +2751,7 @@ static int copy_msg(char *buf, const char *msg)\n \treturn cp - buf;\n }\n \n+/* This function must set a meaningful errno on failure */\n int log_ref_setup(const char *refname, char *logfile, int bufsize)\n {\n \tint logfd, oflags = O_APPEND | O_WRONLY;\n@@ -2761,9 +2762,12 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)\n \t     starts_with(refname, \"refs/remotes/\") ||\n \t     starts_with(refname, \"refs/notes/\") ||\n \t     !strcmp(refname, \"HEAD\"))) {\n-\t\tif (safe_create_leading_directories(logfile) < 0)\n-\t\t\treturn error(\"unable to create directory for %s\",\n-\t\t\t\t     logfile);\n+\t\tif (safe_create_leading_directories(logfile) < 0) {\n+\t\t\tint save_errno = errno;\n+\t\t\terror(\"unable to create directory for %s\", logfile);\n+\t\t\terrno = save_errno;\n+\t\t\treturn -1;\n+\t\t}\n \t\toflags |= O_CREAT;\n \t}\n \n@@ -2774,15 +2778,22 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)\n \n \t\tif ((oflags & O_CREAT) && errno == EISDIR) {\n \t\t\tif (remove_empty_directories(logfile)) {\n-\t\t\t\treturn error(\"There are still logs under '%s'\",\n-\t\t\t\t\t     logfile);\n+\t\t\t\tint save_errno = errno;\n+\t\t\t\terror(\"There are still logs under '%s'\",\n+\t\t\t\t      logfile);\n+\t\t\t\terrno = save_errno;\n+\t\t\t\treturn -1;\n \t\t\t}\n \t\t\tlogfd = open(logfile, oflags, 0666);\n \t\t}\n \n-\t\tif (logfd < 0)\n-\t\t\treturn error(\"Unable to append to %s: %s\",\n-\t\t\t\t     logfile, strerror(errno));\n+\t\tif (logfd < 0) {\n+\t\t\tint save_errno = errno;\n+\t\t\terror(\"Unable to append to %s: %s\", logfile,\n+\t\t\t      strerror(errno));\n+\t\t\terrno = save_errno;\n+\t\t\treturn -1;\n+\t\t}\n \t}\n \n \tadjust_shared_perm(logfile);\ndiff --git a/refs.h b/refs.h\nindex 65f7637..82cc5cb 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -158,7 +158,9 @@ extern void unlock_ref(struct ref_lock *lock);\n /** Writes sha1 into the ref specified by the lock. **/\n extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);\n \n-/** Setup reflog before using. **/\n+/*\n+ * Setup reflog before using. Set errno to something meaningful on failure.\n+ */\n int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n /** Reads log for the value of ref during at_time. **/\n-- \n2.0.1.546.gf603308\n"},{"id":"246021","messageId":"1405371541-32121-11-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 10/19] refs.c: verify_lock should set errno to something meaningful","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:52Z","receivedAt":"2014-07-14T20:58:52Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno when returning from verify_lock() meaningful, which\nshould almost but not completely fix\n\n * a bug in \"git fetch\"'s s_update_ref, which trusts the result of an\n   errno == ENOTDIR check to detect D/F conflicts\n\nENOTDIR makes sense as a sign that a file was in the way of a\ndirectory we wanted to create.  Should \"git fetch\" also look for\nENOTEMPTY or EEXIST to catch cases where a directory was in the way\nof a file to be created?\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 4 ++++\n refs.h | 6 +++++-\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9ea519c..a48f805 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1932,18 +1932,22 @@ int refname_match(const char *abbrev_name, const char *full_name)\n \treturn 0;\n }\n \n+/* This function should make sure errno is meaningful on error */\n static struct ref_lock *verify_lock(struct ref_lock *lock,\n \tconst unsigned char *old_sha1, int mustexist)\n {\n \tif (read_ref_full(lock->ref_name, lock->old_sha1, mustexist, NULL)) {\n+\t\tint save_errno = errno;\n \t\terror(\"Can't verify ref %s\", lock->ref_name);\n \t\tunlock_ref(lock);\n+\t\terrno = save_errno;\n \t\treturn NULL;\n \t}\n \tif (hashcmp(lock->old_sha1, old_sha1)) {\n \t\terror(\"Ref %s is at %s but expected %s\", lock->ref_name,\n \t\t\tsha1_to_hex(lock->old_sha1), sha1_to_hex(old_sha1));\n \t\tunlock_ref(lock);\n+\t\terrno = EBUSY;\n \t\treturn NULL;\n \t}\n \treturn lock;\ndiff --git a/refs.h b/refs.h\nindex 82cc5cb..8d6cac7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -137,11 +137,15 @@ extern int ref_exists(const char *);\n  */\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n-/** Locks a \"refs/\" ref returning the lock on success and NULL on failure. **/\n+/*\n+ * Locks a \"refs/\" ref returning the lock on success and NULL on failure.\n+ * On failure errno is set to something meaningful.\n+ */\n extern struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1);\n \n /** Locks any ref (for 'HEAD' type refs). */\n #define REF_NODEREF\t0x01\n+/* errno is set to something meaningful on failure */\n extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t\tconst unsigned char *old_sha1,\n \t\t\t\t\t\tint flags, int *type_p);\n-- \n2.0.1.546.gf603308\n"},{"id":"246022","messageId":"1405371541-32121-12-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 11/19] refs.c: make remove_empty_directories always set errno to something sane","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:53Z","receivedAt":"2014-07-14T20:58:53Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno when returning from remove_empty_directories() more\nobviously meaningful, which should provide some peace of mind for\npeople auditing lock_ref_sha1_basic.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex a48f805..cc69581 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1960,14 +1960,16 @@ static int remove_empty_directories(const char *file)\n \t * only empty directories), remove them.\n \t */\n \tstruct strbuf path;\n-\tint result;\n+\tint result, save_errno;\n \n \tstrbuf_init(&path, 20);\n \tstrbuf_addstr(&path, file);\n \n \tresult = remove_dir_recursively(&path, REMOVE_DIR_EMPTY_ONLY);\n+\tsave_errno = errno;\n \n \tstrbuf_release(&path);\n+\terrno = save_errno;\n \n \treturn result;\n }\n@@ -2056,6 +2058,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n \treturn logs_found;\n }\n \n+/* This function should make sure errno is meaningful on error */\n static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\t\t\t\t    const unsigned char *old_sha1,\n \t\t\t\t\t    int flags, int *type_p)\n-- \n2.0.1.546.gf603308\n"},{"id":"246017","messageId":"1405371541-32121-13-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 12/19] refs.c: commit_packed_refs to return a meaningful errno on failure","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:54Z","receivedAt":"2014-07-14T20:58:54Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno when returning from commit_packed_refs() meaningful,\nwhich should fix\n\n * a bug in \"git clone\" where it prints strerror(errno) based on\n   errno, despite errno possibly being zero and potentially having\n   been clobbered by that point\n * the same kind of bug in \"git pack-refs\"\n\nand prepares for repack_without_refs() to get a meaningful\nerror message when commit_packed_refs() fails without falling into\nthe same bug.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 10 +++++++++-\n refs.h |  1 +\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex cc69581..7a815be 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2239,11 +2239,16 @@ int lock_packed_refs(int flags)\n \treturn 0;\n }\n \n+/*\n+ * Commit the packed refs changes.\n+ * On error we must make sure that errno contains a meaningful value.\n+ */\n int commit_packed_refs(void)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\n \tint error = 0;\n+\tint save_errno = 0;\n \n \tif (!packed_ref_cache->lock)\n \t\tdie(\"internal error: packed-refs not locked\");\n@@ -2253,10 +2258,13 @@ int commit_packed_refs(void)\n \tdo_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),\n \t\t\t\t 0, write_packed_entry_fn,\n \t\t\t\t &packed_ref_cache->lock->fd);\n-\tif (commit_lock_file(packed_ref_cache->lock))\n+\tif (commit_lock_file(packed_ref_cache->lock)) {\n+\t\tsave_errno = errno;\n \t\terror = -1;\n+\t}\n \tpacked_ref_cache->lock = NULL;\n \trelease_packed_ref_cache(packed_ref_cache);\n+\terrno = save_errno;\n \treturn error;\n }\n \ndiff --git a/refs.h b/refs.h\nindex 8d6cac7..e588ff8 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -98,6 +98,7 @@ extern void add_packed_ref(const char *refname, const unsigned char *sha1);\n  * Write the current version of the packed refs cache from memory to\n  * disk.  The packed-refs file must already be locked for writing (see\n  * lock_packed_refs()).  Return zero on success.\n+ * Sets errno to something meaningful on error.\n  */\n extern int commit_packed_refs(void);\n \n-- \n2.0.1.546.gf603308\n"},{"id":"246027","messageId":"1405371541-32121-14-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 13/19] refs.c: make resolve_ref_unsafe set errno to something meaningful on error","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:55Z","receivedAt":"2014-07-14T20:58:55Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno when returning from resolve_ref_unsafe() meaningful,\nwhich should fix\n\n * a bug in lock_ref_sha1_basic, where it assumes EISDIR\n   means it failed due to a directory being in the way\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h |  2 +-\n refs.c  | 19 +++++++++++++++----\n 2 files changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 8a46294..4ca4583 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -979,7 +979,7 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n  * NULL.  If more than MAXDEPTH recursive symbolic lookups are needed,\n  * give up and return NULL.\n  *\n- * errno is sometimes set on errors, but not always.\n+ * errno is set to something meaningful on error.\n  */\n extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1, int reading, int *flag);\n extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);\ndiff --git a/refs.c b/refs.c\nindex 7a815be..211429d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1334,6 +1334,7 @@ static const char *handle_missing_loose_ref(const char *refname,\n \t}\n }\n \n+/* This function needs to return a meaningful errno on failure */\n const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int reading, int *flag)\n {\n \tint depth = MAXDEPTH;\n@@ -1344,8 +1345,10 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \tif (flag)\n \t\t*flag = 0;\n \n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n+\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\terrno = EINVAL;\n \t\treturn NULL;\n+\t}\n \n \tfor (;;) {\n \t\tchar path[PATH_MAX];\n@@ -1353,8 +1356,10 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\tchar *buf;\n \t\tint fd;\n \n-\t\tif (--depth < 0)\n+\t\tif (--depth < 0) {\n+\t\t\terrno = ELOOP;\n \t\t\treturn NULL;\n+\t\t}\n \n \t\tgit_snpath(path, sizeof(path), \"%s\", refname);\n \n@@ -1416,9 +1421,13 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\t\t\treturn NULL;\n \t\t}\n \t\tlen = read_in_full(fd, buffer, sizeof(buffer)-1);\n-\t\tclose(fd);\n-\t\tif (len < 0)\n+\t\tif (len < 0) {\n+\t\t\tint save_errno = errno;\n+\t\t\tclose(fd);\n+\t\t\terrno = save_errno;\n \t\t\treturn NULL;\n+\t\t}\n+\t\tclose(fd);\n \t\twhile (len && isspace(buffer[len-1]))\n \t\t\tlen--;\n \t\tbuffer[len] = '\\0';\n@@ -1435,6 +1444,7 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\t\t    (buffer[40] != '\\0' && !isspace(buffer[40]))) {\n \t\t\t\tif (flag)\n \t\t\t\t\t*flag |= REF_ISBROKEN;\n+\t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t\treturn refname;\n@@ -1447,6 +1457,7 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (flag)\n \t\t\t\t*flag |= REF_ISBROKEN;\n+\t\t\terrno = EINVAL;\n \t\t\treturn NULL;\n \t\t}\n \t\trefname = strcpy(refname_buffer, buf);\n-- \n2.0.1.546.gf603308\n"},{"id":"246026","messageId":"1405371541-32121-15-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 14/19] refs.c: log_ref_write should try to return meaningful errno","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:56Z","receivedAt":"2014-07-14T20:58:56Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Making errno from write_ref_sha1() meaningful, which should fix\n\n* a bug in \"git checkout -b\" where it prints strerror(errno)\n  despite errno possibly being zero or clobbered\n\n* a bug in \"git fetch\"'s s_update_ref, which trusts the result of an\n  errno == ENOTDIR check to detect D/F conflicts\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 28 +++++++++++++++++++++++-----\n 1 file changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 211429d..61570c9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2859,8 +2859,19 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n \twritten = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;\n \tfree(logrec);\n-\tif (close(logfd) != 0 || written != len)\n-\t\treturn error(\"Unable to append to %s\", log_file);\n+\tif (written != len) {\n+\t\tint save_errno = errno;\n+\t\tclose(logfd);\n+\t\terror(\"Unable to append to %s\", log_file);\n+\t\terrno = save_errno;\n+\t\treturn -1;\n+\t}\n+\tif (close(logfd)) {\n+\t\tint save_errno = errno;\n+\t\terror(\"Unable to append to %s\", log_file);\n+\t\terrno = save_errno;\n+\t\treturn -1;\n+\t}\n \treturn 0;\n }\n \n@@ -2869,14 +2880,17 @@ static int is_branch(const char *refname)\n \treturn !strcmp(refname, \"HEAD\") || starts_with(refname, \"refs/heads/\");\n }\n \n+/* This function must return a meaningful errno */\n int write_ref_sha1(struct ref_lock *lock,\n \tconst unsigned char *sha1, const char *logmsg)\n {\n \tstatic char term = '\\n';\n \tstruct object *o;\n \n-\tif (!lock)\n+\tif (!lock) {\n+\t\terrno = EINVAL;\n \t\treturn -1;\n+\t}\n \tif (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {\n \t\tunlock_ref(lock);\n \t\treturn 0;\n@@ -2886,19 +2900,23 @@ int write_ref_sha1(struct ref_lock *lock,\n \t\terror(\"Trying to write ref %s with nonexistent object %s\",\n \t\t\tlock->ref_name, sha1_to_hex(sha1));\n \t\tunlock_ref(lock);\n+\t\terrno = EINVAL;\n \t\treturn -1;\n \t}\n \tif (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {\n \t\terror(\"Trying to write non-commit object %s to branch %s\",\n \t\t\tsha1_to_hex(sha1), lock->ref_name);\n \t\tunlock_ref(lock);\n+\t\terrno = EINVAL;\n \t\treturn -1;\n \t}\n \tif (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||\n-\t    write_in_full(lock->lock_fd, &term, 1) != 1\n-\t\t|| close_ref(lock) < 0) {\n+\t    write_in_full(lock->lock_fd, &term, 1) != 1 ||\n+\t    close_ref(lock) < 0) {\n+\t\tint save_errno = errno;\n \t\terror(\"Couldn't write %s\", lock->lk->filename);\n \t\tunlock_ref(lock);\n+\t\terrno = save_errno;\n \t\treturn -1;\n \t}\n \tclear_loose_ref_cache(&ref_cache);\n-- \n2.0.1.546.gf603308\n"},{"id":"246029","messageId":"1405371541-32121-16-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 15/19] refs.c: make ref_update_reject_duplicates take a strbuf argument for errors","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:57Z","receivedAt":"2014-07-14T20:58:57Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Make ref_update_reject_duplicates return any error that occurs through a\nnew strbuf argument. This means that when a transaction commit fails in\nthis function we will now be able to pass a helpful error message back to the\ncaller.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 61570c9..115f143 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3488,6 +3488,7 @@ static int ref_update_compare(const void *r1, const void *r2)\n }\n \n static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n+\t\t\t\t\tstruct strbuf *err,\n \t\t\t\t\tenum action_on_err onerr)\n {\n \tint i;\n@@ -3495,6 +3496,9 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\tif (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {\n \t\t\tconst char *str =\n \t\t\t\t\"Multiple updates for ref '%s' not allowed.\";\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, str, updates[i]->refname);\n+\n \t\t\tswitch (onerr) {\n \t\t\tcase UPDATE_REFS_MSG_ON_ERR:\n \t\t\t\terror(str, updates[i]->refname); break;\n@@ -3525,7 +3529,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t/* Copy, sort, and reject duplicate refs */\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n-\tret = ref_update_reject_duplicates(updates, n, onerr);\n+\tret = ref_update_reject_duplicates(updates, n, err, onerr);\n \tif (ret)\n \t\tgoto cleanup;\n \n-- \n2.0.1.546.gf603308\n"},{"id":"246023","messageId":"1405371541-32121-17-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 16/19] refs.c: make update_ref_write update a strbuf on failure","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:58Z","receivedAt":"2014-07-14T20:58:58Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change update_ref_write to also update an error strbuf on failure.\nThis makes the error available to ref_transaction_commit callers if the\ntransaction failed due to update_ref_sha1/write_ref_sha1 failures.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 115f143..003b313 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3353,10 +3353,13 @@ static struct ref_lock *update_ref_lock(const char *refname,\n \n static int update_ref_write(const char *action, const char *refname,\n \t\t\t    const unsigned char *sha1, struct ref_lock *lock,\n-\t\t\t    enum action_on_err onerr)\n+\t\t\t    struct strbuf *err, enum action_on_err onerr)\n {\n \tif (write_ref_sha1(lock, sha1, action) < 0) {\n \t\tconst char *str = \"Cannot update the ref '%s'.\";\n+\t\tif (err)\n+\t\t\tstrbuf_addf(err, str, refname);\n+\n \t\tswitch (onerr) {\n \t\tcase UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;\n \t\tcase UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;\n@@ -3477,7 +3480,7 @@ int update_ref(const char *action, const char *refname,\n \tlock = update_ref_lock(refname, oldval, flags, NULL, onerr);\n \tif (!lock)\n \t\treturn 1;\n-\treturn update_ref_write(action, refname, sha1, lock, onerr);\n+\treturn update_ref_write(action, refname, sha1, lock, NULL, onerr);\n }\n \n static int ref_update_compare(const void *r1, const void *r2)\n@@ -3559,7 +3562,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\tret = update_ref_write(msg,\n \t\t\t\t\t       update->refname,\n \t\t\t\t\t       update->new_sha1,\n-\t\t\t\t\t       update->lock, onerr);\n+\t\t\t\t\t       update->lock, err, onerr);\n \t\t\tupdate->lock = NULL; /* freed by update_ref_write */\n \t\t\tif (ret)\n \t\t\t\tgoto cleanup;\n-- \n2.0.1.546.gf603308\n"},{"id":"246024","messageId":"1405371541-32121-18-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 17/19] update-ref: use err argument to get error from ref_transaction_commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:58:59Z","receivedAt":"2014-07-14T20:58:59Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Call ref_transaction_commit with QUIET_ON_ERR and use the strbuf that is\nreturned to print a log message if/after the transaction fails.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 22617af..aec9004 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -342,6 +342,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tconst char *refname, *oldval, *msg = NULL;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_STRING( 'm', NULL, &msg, N_(\"reason\"), N_(\"reason of the update\")),\n \t\tOPT_BOOL('d', NULL, &delete, N_(\"delete the reference\")),\n@@ -359,18 +360,17 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Refusing to perform update with empty message.\");\n \n \tif (read_stdin) {\n-\t\tint ret;\n \t\ttransaction = ref_transaction_begin();\n-\n \t\tif (delete || no_deref || argc > 0)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tret = ref_transaction_commit(transaction, msg, NULL,\n-\t\t\t\t\t     UPDATE_REFS_DIE_ON_ERR);\n+\t\tif (ref_transaction_commit(transaction, msg, &err,\n+\t\t\t\t\t   UPDATE_REFS_QUIET_ON_ERR))\n+\t\t\tdie(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n-\t\treturn ret;\n+\t\treturn 0;\n \t}\n \n \tif (end_null)\n-- \n2.0.1.546.gf603308\n"},{"id":"246028","messageId":"1405371541-32121-19-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 18/19] refs.c: remove the onerr argument to ref_transaction_commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:59:00Z","receivedAt":"2014-07-14T20:59:00Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr\nargument any more. Remove the onerr argument from the ref_transaction_commit\nsignature.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c |  3 +--\n refs.c               | 22 +++++++---------------\n refs.h               |  3 +--\n 3 files changed, 9 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex aec9004..88ab785 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -366,8 +366,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tif (ref_transaction_commit(transaction, msg, &err,\n-\t\t\t\t\t   UPDATE_REFS_QUIET_ON_ERR))\n+\t\tif (ref_transaction_commit(transaction, msg, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n \t\treturn 0;\ndiff --git a/refs.c b/refs.c\nindex 003b313..4f78bd9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3491,8 +3491,7 @@ static int ref_update_compare(const void *r1, const void *r2)\n }\n \n static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n-\t\t\t\t\tstruct strbuf *err,\n-\t\t\t\t\tenum action_on_err onerr)\n+\t\t\t\t\tstruct strbuf *err)\n {\n \tint i;\n \tfor (i = 1; i < n; i++)\n@@ -3502,22 +3501,13 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, str, updates[i]->refname);\n \n-\t\t\tswitch (onerr) {\n-\t\t\tcase UPDATE_REFS_MSG_ON_ERR:\n-\t\t\t\terror(str, updates[i]->refname); break;\n-\t\t\tcase UPDATE_REFS_DIE_ON_ERR:\n-\t\t\t\tdie(str, updates[i]->refname); break;\n-\t\t\tcase UPDATE_REFS_QUIET_ON_ERR:\n-\t\t\t\tbreak;\n-\t\t\t}\n \t\t\treturn 1;\n \t\t}\n \treturn 0;\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t   const char *msg, struct strbuf *err,\n-\t\t\t   enum action_on_err onerr)\n+\t\t\t   const char *msg, struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3532,7 +3522,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t/* Copy, sort, and reject duplicate refs */\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n-\tret = ref_update_reject_duplicates(updates, n, err, onerr);\n+\tret = ref_update_reject_duplicates(updates, n, err);\n \tif (ret)\n \t\tgoto cleanup;\n \n@@ -3544,7 +3534,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t       (update->have_old ?\n \t\t\t\t\t\tupdate->old_sha1 : NULL),\n \t\t\t\t\t       update->flags,\n-\t\t\t\t\t       &update->type, onerr);\n+\t\t\t\t\t       &update->type,\n+\t\t\t\t\t       UPDATE_REFS_QUIET_ON_ERR);\n \t\tif (!update->lock) {\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n@@ -3562,7 +3553,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\tret = update_ref_write(msg,\n \t\t\t\t\t       update->refname,\n \t\t\t\t\t       update->new_sha1,\n-\t\t\t\t\t       update->lock, err, onerr);\n+\t\t\t\t\t       update->lock, err,\n+\t\t\t\t\t       UPDATE_REFS_QUIET_ON_ERR);\n \t\t\tupdate->lock = NULL; /* freed by update_ref_write */\n \t\t\tif (ret)\n \t\t\t\tgoto cleanup;\ndiff --git a/refs.h b/refs.h\nindex e588ff8..163b45c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -282,8 +282,7 @@ void ref_transaction_delete(struct ref_transaction *transaction,\n  * the transaction failed. The string does not end in newline.\n  */\n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t   const char *msg, struct strbuf *err,\n-\t\t\t   enum action_on_err onerr);\n+\t\t\t   const char *msg, struct strbuf *err);\n \n /*\n  * Free an existing transaction and all associated data.\n-- \n2.0.1.546.gf603308\n"},{"id":"246025","messageId":"1405371541-32121-20-git-send-email-sahlberg@google.com","threadId":"37127","inReplyTo":"1405371541-32121-1-git-send-email-sahlberg@google.com","subject":"[PATCH v21 19/19] refs.c: change ref_transaction_update() to do error checking and return status","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-14T20:59:01Z","receivedAt":"2014-07-14T20:59:01Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update ref_transaction_update() do some basic error checking and return\nnon-zero on error. Update all callers to check ref_transaction_update() for\nerror. There are currently no conditions in _update that will return error but\nthere will be in the future. Add an err argument that will be updated on\nfailure. In future patches we will start doing both locking and checking\nfor name conflicts in _update instead of _commit at which time this function\nwill start returning errors for these conditions.\n\nAlso check for BUGs during update and die(BUG:...) if we are calling\n_update with have_old but the old_sha1 pointer is NULL.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Michael Haggerty <mhagger@alum.mit.edu>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c | 12 +++++++-----\n refs.c               | 18 ++++++++++++------\n refs.h               | 14 +++++++++-----\n 3 files changed, 28 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 88ab785..3067b11 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -16,6 +16,7 @@ static struct ref_transaction *transaction;\n \n static char line_termination = '\\n';\n static int update_flags;\n+static struct strbuf err = STRBUF_INIT;\n \n /*\n  * Parse one whitespace- or NUL-terminated, possibly C-quoted argument\n@@ -197,8 +198,9 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \tif (*next != line_termination)\n \t\tdie(\"update %s: extra input: %s\", refname, next);\n \n-\tref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t       update_flags, have_old);\n+\tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t   update_flags, have_old, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -286,8 +288,9 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \tif (*next != line_termination)\n \t\tdie(\"verify %s: extra input: %s\", refname, next);\n \n-\tref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t       update_flags, have_old);\n+\tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t   update_flags, have_old, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -342,7 +345,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tconst char *refname, *oldval, *msg = NULL;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n-\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_STRING( 'm', NULL, &msg, N_(\"reason\"), N_(\"reason of the update\")),\n \t\tOPT_BOOL('d', NULL, &delete, N_(\"delete the reference\")),\ndiff --git a/refs.c b/refs.c\nindex 4f78bd9..3f05e88 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3428,19 +3428,25 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n \treturn update;\n }\n \n-void ref_transaction_update(struct ref_transaction *transaction,\n-\t\t\t    const char *refname,\n-\t\t\t    const unsigned char *new_sha1,\n-\t\t\t    const unsigned char *old_sha1,\n-\t\t\t    int flags, int have_old)\n+int ref_transaction_update(struct ref_transaction *transaction,\n+\t\t\t   const char *refname,\n+\t\t\t   const unsigned char *new_sha1,\n+\t\t\t   const unsigned char *old_sha1,\n+\t\t\t   int flags, int have_old,\n+\t\t\t   struct strbuf *err)\n {\n-\tstruct ref_update *update = add_update(transaction, refname);\n+\tstruct ref_update *update;\n+\n+\tif (have_old && !old_sha1)\n+\t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n+\tupdate = add_update(transaction, refname);\n \thashcpy(update->new_sha1, new_sha1);\n \tupdate->flags = flags;\n \tupdate->have_old = have_old;\n \tif (have_old)\n \t\thashcpy(update->old_sha1, old_sha1);\n+\treturn 0;\n }\n \n void ref_transaction_create(struct ref_transaction *transaction,\ndiff --git a/refs.h b/refs.h\nindex 163b45c..c5376ce 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -246,12 +246,16 @@ struct ref_transaction *ref_transaction_begin(void);\n  * be deleted.  If have_old is true, then old_sha1 holds the value\n  * that the reference should have had before the update, or zeros if\n  * it must not have existed beforehand.\n+ * Function returns 0 on success and non-zero on failure. A failure to update\n+ * means that the transaction as a whole has failed and will need to be\n+ * rolled back. On failure the err buffer will be updated.\n  */\n-void ref_transaction_update(struct ref_transaction *transaction,\n-\t\t\t    const char *refname,\n-\t\t\t    const unsigned char *new_sha1,\n-\t\t\t    const unsigned char *old_sha1,\n-\t\t\t    int flags, int have_old);\n+int ref_transaction_update(struct ref_transaction *transaction,\n+\t\t\t   const char *refname,\n+\t\t\t   const unsigned char *new_sha1,\n+\t\t\t   const unsigned char *old_sha1,\n+\t\t\t   int flags, int have_old,\n+\t\t\t   struct strbuf *err);\n \n /*\n  * Add a reference creation to transaction.  new_sha1 is the value\n-- \n2.0.1.546.gf603308\n"}]}