From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 05 Jun 2024 16:54:02 GMT
Subject: Re: [PATCH v4 10/12] refs: implement removal of ref storages
Message-ID: <xmqq5xunuy8l.fsf@gitster.g>
In-Reply-To: <20240605101200.GA3440499@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> If removing either of the directories fails, we set ret to "-1". Make
> sense. But...
>
>> +	ret = for_each_root_ref(refs, remove_one_root_ref, &data);
>> +	if (ret < 0)
>> +		ret = -1;
>
> ...then we unconditionally overwrite it, forgetting the earlier error.

Ouch.

> Is that a mistake, or are we normalizing other negative values? Maybe
> just:
>
>   if (for_each_root_ref(refs, remove_one_root_ref, &data) < 0)
> 	ret = -1;
>
> would work?

Sounds sensible.

Thanks for carefully reading.