Re: [PATCH 5/5] run-command API: remove "argv" member, always use "args"
From: Jeff King <hidden>
Date: 2021-11-22 18:47:45
On Mon, Nov 22, 2021 at 07:19:14PM +0100, Ævar Arnfjörð Bjarmason wrote:
I did have this with a strvec_attach() locally, but figured I'd get comments about growing scope & with just one caller.
I do think I'd rather see us avoiding have to do this attach() by refactoring the Windows code. But I sympathize with your pain in the guess-and-wait-for-CI loop with Windows (I have an unrelated series I've done that with, and it's rather awful).
quoted hunk ↗ jump to hunk
This version seems to be duplicating things in the existing API though, I just had the below, which I think works just as well without the duplication. Perhaps you missed strvec_push_nodup()?diff --git a/strvec.c b/strvec.c index 61a76ce6cb9..c10008d792f 100644 --- a/strvec.c +++ b/strvec.c@@ -106,3 +106,9 @@ const char **strvec_detach(struct strvec *array) return ret; } } + +void strvec_attach(struct strvec *array, const char **items) +{ + for (; *items; items++) + strvec_push_nodup(array, *items); +}
This isn't taking ownership of "items", though. We've attached the things it points to, but not the array itself. It would perhaps be OK to free() it here under the notion that we took ownership of it and it is ours to do with as we please. Potentially a caller might expect that we'd continue to use the attached buffer, but any such assumption is invalid once another strvec_push() is called, since that could replace the array anyway. It's also slightly less efficient (it grows a new array unnecessarily), but I doubt that matters much in practice. -Peff