On Wed, Mar 23 2022, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> +static void release_revisions_commit_list(struct rev_info *revs)
>> +{
>> + struct commit_list *commits = revs->commits;
>> +
>> + if (!commits)
>> + return;
>> + free_commit_list(commits);
>> + revs->commits = NULL;
>> +}
>
> It makes sense to have this as a separate helper, but the original
> implementation this was lifted from is much easier to follow than
> this version with an unnecessary rewrite, I would think.
>
>> @@ -4080,10 +4090,7 @@ static void create_boundary_commit_list(struct rev_info *revs)
>> * boundary commits anyway. (This is what the code has always
>> * done.)
>> */
>> - if (revs->commits) {
>> - free_commit_list(revs->commits);
>> - revs->commits = NULL;
>> - }
>> + release_revisions_commit_list(revs);
>
> IOW
>
> static void release_revisions_commit_list(struct rev_info *revs)
> {
> if (revs->commits) {
> free_commit_list(revs->commits);
> revs->commits = NULL;
> }
> }
Sure, will change it.
I think the pre-image is preferrable in context, i.e. to have them all
use the same early abort pattern, but not enough to argue the point :)
