Re: [PATCH v6 03/42] refs.c: add a strbuf argument to ref_transaction_commit for error logging
From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:01:08
Ronnie Sahlberg wrote:
Add a strbuf argument to _commit so that we can pass an error string back to the caller. So that we can do error logging from the caller instead of from _commit. Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR and craft any log messages from the callers themselves and finally remove the onerr argument completely.
Very nice. [...]
quoted hunk ↗ jump to hunk
+++ b/refs.c
[...]
quoted hunk ↗ jump to hunk
@@ -3443,6 +3444,9 @@ int ref_transaction_commit(struct ref_transaction *transaction, update->flags, &update->type, onerr); if (!update->lock) { + if (err) + strbuf_addf(err ,"Cannot lock the ref '%s'.", + update->refname);
Tiny nit: whitespace. [...]
quoted hunk ↗ jump to hunk
--- a/refs.h +++ b/refs.h@@ -268,9 +268,12 @@ void ref_transaction_delete(struct ref_transaction *transaction, * Commit all of the changes that have been queued in transaction, as * atomically as possible. Return a nonzero value if there is a * problem. The ref_transaction is freed by this function. + * If err is non-NULL we will add an error string to it to explain why + * the transaction failed.
Probably worth mentioning the error string doesn't end with a newline so the caller knows how to use it. With the whitespace fix and with or without the comment tweak, Reviewed-by: Jonathan Nieder <redacted>
diff --git i/refs.c w/refs.c
index 64e8feb..2ca3169 100644
--- i/refs.c
+++ w/refs.c@@ -3445,7 +3445,7 @@ int ref_transaction_commit(struct ref_transaction *transaction, &update->type, onerr); if (!update->lock) { if (err) - strbuf_addf(err ,"Cannot lock the ref '%s'.", + strbuf_addf(err, "Cannot lock the ref '%s'.", update->refname); ret = 1; goto cleanup;
diff --git i/refs.h w/refs.h
index ff87e14..d45212f 100644
--- i/refs.h
+++ w/refs.h@@ -268,8 +268,8 @@ void ref_transaction_delete(struct ref_transaction *transaction, * Commit all of the changes that have been queued in transaction, as * atomically as possible. Return a nonzero value if there is a * problem. The ref_transaction is freed by this function. - * If err is non-NULL we will add an error string to it to explain why - * the transaction failed. + * If err is non-NULL we will append an error string (with no trailing + * newline) to it to explain why the transaction failed. */ int ref_transaction_commit(struct ref_transaction *transaction, const char *msg, struct strbuf *err,