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

Junio C Hamano [off-list ref] writes:

Karthik Nayak [off-list ref] writes:

quoted

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.