Re: [PATCH v3 2/5] strvec: add a strvec_pushvec()

From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>,
	Derrick Stolee <stolee@gmail.com>
Subject: Re: [PATCH v3 2/5] strvec: add a strvec_pushvec()
Date: Fri, 27 Aug 2021 18:29:30 -0700	[thread overview]
Message-ID: <xmqq4kba9x1h.fsf@gitster.g> (raw)
In-Reply-To: <xmqq8s0m9xbl.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 27 Aug 2021 18:23:26 -0700")

Junio C Hamano <gitster@pobox.com> writes:

>> +void strvec_pushvec(struct strvec *array, const struct strvec *items)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < items->nr; i++)
>> +		strvec_push(array, items->v[i]);
>> +}
>
> This implementation is not wrong per-se, but is somewhat
> disappointing.  When items->nr is large, especially relative to the
> original array->alloc, it would incur unnecessary reallocations that
> we can easily avoid by pre-sizing the array before pushing the
> elements of items from it.
>
> In the original code that became the first user of this helper, it
> may not have made much difference, but now it is becoming a more
> generally reusable API function, we should care.

And if we do not care, you can rewrite the code that became the
first user of this helper to instead call strvec_pushv() on the
items->v array that is guaranteed to be NULL terminated, without
inventing this new helper.

I think I am fine with either way.