Re: [PATCH v5 0/5] for-each-ref: add '--include-root-refs' option

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2024, 20:13 UTC
Message-ID: <xmqqv86frlag.fsf@gitster.g>
In-Reply-To: <xmqqzfvrrpju.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 26 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Changes from v4:
>> 1. Fixed erratic whitespace
>> 2. Remove braces from single line block
>> 3. Starting the comments with a capital and also adding more context.
>> 4. Removed a duplicate check.
>
> Does #4 refer to this removal?
>
>  	if (filter->kind == FILTER_REFS_KIND_MASK && kind == FILTER_REFS_DETACHED_HEAD)
>  		kind = FILTER_REFS_PSEUDOREFS;
>  	else if (!(kind & filter->kind))
>  		return NULL;
>  
> -	if (!(kind & filter->kind))
> -		return NULL;
> -
>  	if (!filter_pattern_match(filter, refname))
>  		return NULL;
>  
>
> If filter->kind is MASK and kind is set to filter detached HEAD, we
> assign to and change the value of kind to FILTER_REFS_PSEUDOREFS,
> so it is unclear if the freestanding "if kind and filter->kind does
> not overlap, return NULL" was redundant or outright buggy.

Ah, of course this is simply redundant. The assignment to limit the kind to PSEUDOREFS happens only when filter->kind has all bits, and after assigning a different non-zero value to kind, (kind & filter->kind) will still overlap, so a freestanding "if no overlap between kind and filter-kind then return NULL" will not trigger for such a case.

Thanks.