Re: [PATCH 04/23] prefix_ref_iterator: don't trim too much

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

Michael Haggerty <mhagger@alum.mit.edu> writes:

> The `trim` parameter can be set independently of `prefix`. So if some
> caller were to set `trim` to be greater than `strlen(prefix)`, we
> could end up pointing the `refname` field of the iterator past the NUL
> of the actual reference name string.
>
> That can't happen currently, because `trim` is always set either to
> zero or to `strlen(prefix)`. But even the latter could lead to
> confusion, if a refname is exactly equal to the prefix, because then
> we would set the outgoing `refname` to the empty string.
>
> And we're about to decouple the `prefix` and `trim` arguments even
> more, so let's be cautious here. Skip over any references whose names
> are not longer than `trim`.

Should we be silently continuing, or should we use die("BUG") here
instead, if the motivation is to be cautions?  Personally, I do not
find this memchr() implementation too bad, especially when our
objective is to play cautious, but strlen() based one is fine, too.

It's not like refname field would point at a run of non-NUL bytes at
the end of the last-mapped page and taking strlen() would segfault,
right?

> +		if (iter->trim) {
> +			/*
> +			 * If there wouldn't be at least one character
> +			 * left in the refname after trimming, skip
> +			 * over it:
> +			 */
> +			if (memchr(iter->iter0->refname, '\0', iter->trim + 1))
> +				continue;
> +			iter->base.refname = iter->iter0->refname + iter->trim;
> +		} else {
> +			iter->base.refname = iter->iter0->refname;
> +		}
> +
>  		iter->base.oid = iter->iter0->oid;
>  		iter->base.flags = iter->iter0->flags;
>  		return ITER_OK;