Re: [PATCH 01/22] sequencer: use repository parameter in short_commit_name()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 29, 2023, 01:16 UTC
Message-ID: <xmqqh6oify21.fsf@gitster.g>
In-Reply-To: <20230829004801.GA3876652@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

>> but that will no longer be true with the updated code.  I'd feel
>> safer if at the beginning of the function we create a local variable
>> "struct rev_info *repo" that is initialized to opts->revs->repo and
>> use it throughout the function instead of the_repository.
>
> I'm not sure how that makes it any safer, as it would mean using the
> suspect repo pointer in more places. Unless you are proposing to do:
>
>   struct repository *r = opts->revs->repo;
>   if (!r)
> 	r = the_repository; /* or BUG() ? */

Yup, the BUG() variant was exactly what I had in mind, but without the assert, by using opts->revs->repo more consistently, we would make sure nobody will call us with uninitialized opts->revs->repo now or in the future.