On Tue, Sep 14, 2021 at 07:51:35PM -0400, Jeff King wrote:
> @@ -156,15 +162,25 @@ int ls_refs(struct repository *r, struct packet_reader *request)
> data.peel = 1;
> else if (!strcmp("symrefs", arg))
> data.symrefs = 1;
> - else if (skip_prefix(arg, "ref-prefix ", &out))
> - strvec_push(&data.prefixes, out);
> + else if (skip_prefix(arg, "ref-prefix ", &out)) {
> + if (data.prefixes.nr < TOO_MANY_PREFIXES)
> + strvec_push(&data.prefixes, out);
> + }
> else if (!strcmp("unborn", arg))
> data.unborn = allow_unborn;
> }
>
> if (request->status != PACKET_READ_FLUSH)
> die(_("expected flush after ls-refs arguments"));
>
> + /*
> + * If we saw too many prefixes, we must avoid using them at all; as
> + * soon as we have any prefix, they are meant to form a comprehensive
> + * list.
> + */
> + if (data.prefixes.nr >= TOO_MANY_PREFIXES)
> + strvec_clear(&data.prefixes);
> +
Great, I find this version even better than what I suggested where the
case where the list already has too many prefixes `continue`s through
the loop before calling strvec_add().
Just noting aloud, the `>` part of this conditional will never happen
(since data.prefixes.nr will be at most equal to TOO_MANY_PREFIXES, but
never larger than it).
Obviously not wrong, but I figured it'd be worth mentioning in case it
caught the attention of other reviewers.
Thanks,
Taylor
