On 05/18/2017 06:19 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@xxxxxxxxxxxx> 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.
True; it would not make much sense to trim off more characters than a
prefix that you have selected for. This also implies that callers should
only check-and-trim a prefix that ends with an explicit "/". Otherwise,
say for example if a caller tries to check-and-trim the prefix
"refs/bisect" and the user has a reference named "refs/bisect", the
result could be the empty string.
I'll change this to die("BUG") and document the above.
> 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?
No, refname should be a legit string.
Michael