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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.