Some functions that take a strbuf argument to append an error to treat !err as an indication that the message should be suppressed (e.g., ref_update_reject_duplicates). Others write the message to stderr on !err (e.g., repack_without_refs). Others crash (e.g., ref_transaction_update). Some of these behaviors are for historical reasons and others were accidents. Luckily no callers pass err == NULL any more. Simplify by consistently requiring the strbuf argument. Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> --- refs.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/refs.c b/refs.c index 6faf0c8..550223c 100644 --- a/refs.c +++ b/refs.c @@ -2560,6 +2560,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) struct string_list_item *ref_to_delete; int i, ret, removed = 0; + assert(err); + /* Look for a packed ref */ for (i = 0; i < n; i++) if (get_packed_ref(refnames[i])) @@ -2570,13 +2572,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) return 0; /* no refname exists in packed refs */ if (lock_packed_refs(0)) { - if (err) { - unable_to_lock_message(git_path("packed-refs"), errno, - err); - return -1; - } - unable_to_lock_error(git_path("packed-refs"), errno); - return error("cannot delete '%s' from packed refs", refnames[i]); + unable_to_lock_message(git_path("packed-refs"), errno, err); + return -1; } packed = get_packed_refs(&ref_cache); @@ -2602,7 +2599,7 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) /* Write what remains */ ret = commit_packed_refs(); - if (ret && err) + if (ret) strbuf_addf(err, "unable to overwrite old ref-pack file: %s", strerror(errno)); return ret; @@ -2610,6 +2607,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) { + assert(err); + if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) { /* loose */ int res, i = strlen(lock->lk->filename) - 5; /* .lock */ @@ -3459,6 +3458,8 @@ struct ref_transaction { struct ref_transaction *ref_transaction_begin(struct strbuf *err) { + assert(err); + return xcalloc(1, sizeof(struct ref_transaction)); } @@ -3498,6 +3499,8 @@ int ref_transaction_update(struct ref_transaction *transaction, { struct ref_update *update; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: update called for transaction that is not open"); @@ -3530,6 +3533,8 @@ int ref_transaction_create(struct ref_transaction *transaction, { struct ref_update *update; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: create called for transaction that is not open"); @@ -3561,6 +3566,8 @@ int ref_transaction_delete(struct ref_transaction *transaction, { struct ref_update *update; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: delete called for transaction that is not open"); @@ -3623,13 +3630,14 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, struct strbuf *err) { int i; + + assert(err); + for (i = 1; i < n; i++) if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) { - const char *str = - "Multiple updates for ref '%s' not allowed."; - if (err) - strbuf_addf(err, str, updates[i]->refname); - + strbuf_addf(err, + "Multiple updates for ref '%s' not allowed.", + updates[i]->refname); return 1; } return 0; @@ -3643,6 +3651,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, int n = transaction->nr; struct ref_update **updates = transaction->updates; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: commit called for transaction that is not open"); @@ -3675,9 +3685,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, if (!update->lock) { int df_conflict = (errno == ENOTDIR); - if (err) - strbuf_addf(err, "Cannot lock the ref '%s'.", - update->refname); + strbuf_addf(err, "Cannot lock the ref '%s'.", + update->refname); ret = df_conflict ? UPDATE_REFS_NAME_CONFLICT : -1; goto cleanup; } @@ -3692,9 +3701,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, update->msg); update->lock = NULL; /* freed by write_ref_sha1 */ if (ret) { - if (err) - strbuf_addf(err, "Cannot update the ref '%s'.", - update->refname); + strbuf_addf(err, "Cannot update the ref '%s'.", + update->refname); ret = -1; goto cleanup; } -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html