refs.c: do not permit err == NULL
Some functions that take a strbuf argument to append an error 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@gmail.com> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Nieder committed
Aug 28, 2014 at 16:42 UTC
5a603b046351894a3c892d5bd6948fcee1bee5ab
1 file changed
+27
-19
refs.c
+27
-19
@@ -2646,6 +2646,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
2646
struct string_list_item *ref_to_delete;
2647
int i, ret, removed = 0;
2648
2649
+ assert(err);
2650
+
2651
/* Look for a packed ref */
2652
for (i = 0; i < n; i++)
2653
if (get_packed_ref(refnames[i]))
@@ -2656,13 +2658,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
2658
return 0; /* no refname exists in packed refs */
2659
2660
if (lock_packed_refs(0)) {
2659
- if (err) {
2660
- unable_to_lock_message(git_path("packed-refs"), errno,
2661
- err);
2662
- return -1;
2663
- }
2664
- unable_to_lock_error(git_path("packed-refs"), errno);
2665
- return error("cannot delete '%s' from packed refs", refnames[i]);
2661
+ unable_to_lock_message(git_path("packed-refs"), errno, err);
2662
+ return -1;
2663
}
2664
packed = get_packed_refs(&ref_cache);
2665
@@ -2688,7 +2685,7 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
2685
2686
/* Write what remains */
2687
ret = commit_packed_refs();
2691
- if (ret && err)
2688
+ if (ret)
2689
strbuf_addf(err, "unable to overwrite old ref-pack file: %s",
2690
strerror(errno));
2691
return ret;
@@ -2696,6 +2693,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
2693
2694
static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
2695
{
2696
+ assert(err);
2697
+
2698
if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
2699
/*
2700
* loose. The loose file name is the same as the
@@ -3551,6 +3550,8 @@ struct ref_transaction {
3550
3551
struct ref_transaction *ref_transaction_begin(struct strbuf *err)
3552
{
3553
+ assert(err);
3554
+
3555
return xcalloc(1, sizeof(struct ref_transaction));
3556
}
3557
@@ -3590,6 +3591,8 @@ int ref_transaction_update(struct ref_transaction *transaction,
3591
{
3592
struct ref_update *update;
3593
3594
+ assert(err);
3595
+
3596
if (transaction->state != REF_TRANSACTION_OPEN)
3597
die("BUG: update called for transaction that is not open");
3598
@@ -3622,6 +3625,8 @@ int ref_transaction_create(struct ref_transaction *transaction,
3625
{
3626
struct ref_update *update;
3627
3628
+ assert(err);
3629
+
3630
if (transaction->state != REF_TRANSACTION_OPEN)
3631
die("BUG: create called for transaction that is not open");
3632
@@ -3653,6 +3658,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,
3658
{
3659
struct ref_update *update;
3660
3661
+ assert(err);
3662
+
3663
if (transaction->state != REF_TRANSACTION_OPEN)
3664
die("BUG: delete called for transaction that is not open");
3665
@@ -3715,13 +3722,14 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
3722
struct strbuf *err)
3723
{
3724
int i;
3725
+
3726
+ assert(err);
3727
+
3728
for (i = 1; i < n; i++)
3729
if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
3720
- const char *str =
3721
- "Multiple updates for ref '%s' not allowed.";
3722
- if (err)
3723
- strbuf_addf(err, str, updates[i]->refname);
3724
-
3730
+ strbuf_addf(err,
3731
+ "Multiple updates for ref '%s' not allowed.",
3732
+ updates[i]->refname);
3733
return 1;
3734
}
3735
return 0;
@@ -3735,6 +3743,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3743
int n = transaction->nr;
3744
struct ref_update **updates = transaction->updates;
3745
3746
+ assert(err);
3747
+
3748
if (transaction->state != REF_TRANSACTION_OPEN)
3749
die("BUG: commit called for transaction that is not open");
3750
@@ -3771,9 +3781,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3781
ret = (errno == ENOTDIR)
3782
? TRANSACTION_NAME_CONFLICT
3783
: TRANSACTION_GENERIC_ERROR;
3774
- if (err)
3775
- strbuf_addf(err, "Cannot lock the ref '%s'.",
3776
- update->refname);
3784
+ strbuf_addf(err, "Cannot lock the ref '%s'.",
3785
+ update->refname);
3786
goto cleanup;
3787
}
3788
}
@@ -3786,9 +3795,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3795
if (write_ref_sha1(update->lock, update->new_sha1,
3796
update->msg)) {
3797
update->lock = NULL; /* freed by write_ref_sha1 */
3789
- if (err)
3790
- strbuf_addf(err, "Cannot update the ref '%s'.",
3791
- update->refname);
3798
+ strbuf_addf(err, "Cannot update the ref '%s'.",
3799
+ update->refname);
3800
ret = TRANSACTION_GENERIC_ERROR;
3801
goto cleanup;
3802
}