Re: [PATCH v2 07/11] ls-refs: ignore very long ref-prefix counts
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Sep 15, 2021, 04:16 UTC
- Message-ID
- <YUFzhC5Eox8IND4O@nand.local>
- In-Reply-To
- <YUE1hz/KKM0XebCP@coredump.intra.peff.net>
On Tue, Sep 14, 2021 at 07:51:35PM -0400, Jeff King wrote:
Show 25 quoted lines
> @@ -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