Jeff King <peff@xxxxxxxx> writes:
> On Wed, Mar 05, 2025 at 06:39:01PM +0100, Karthik Nayak wrote:
>
>> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
>> index 0132b8b06a..dd9912d637 100644
>> --- a/refs/reftable-backend.c
>> +++ b/refs/reftable-backend.c
>> @@ -1371,8 +1371,15 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
>> transaction->updates[i],
>> &refnames_to_check, head_type,
>> &head_referent, &referent, err);
>> - if (ret)
>> + if (ret) {
>> + if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
>> + strbuf_setlen(err, 0);
>> + ret = 0;
>> +
>> + continue;
>> + }
>> goto done;
>> + }
>> }
>>
>> string_list_sort(&refnames_to_check);
>
> Coverity complains that this "ret = 0" is a dead store. I think it's
> right, because either:
>
> 1. Our continue loops again, and we overwrite "ret" with the next call
> to prepare_single_update().
>
> 2. We leave the loop (because this is the final entry in the
> transaction update array), and then we overwrite "ret" with the
> result of refs_verify_refnames_available().
>
> But it may be better to leave it in place as a defensive measure against
> the rest of the function changing.
>
Yes agreed with your analysis, and also your inference. So I'll let this stay.
Thanks for reporting!
> -Peff
Attachment:
signature.asc
Description: PGP signature
