From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id BAD051FA00 for ; Mon, 22 Nov 2021 18:47:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240555AbhKVSuu (ORCPT ); Mon, 22 Nov 2021 13:50:50 -0500 Received: from cloud.peff.net ([104.130.231.41]:36568 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240431AbhKVSum (ORCPT ); Mon, 22 Nov 2021 13:50:42 -0500 Received: (qmail 22018 invoked by uid 109); 22 Nov 2021 18:47:35 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 22 Nov 2021 18:47:35 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 23613 invoked by uid 111); 22 Nov 2021 18:47:35 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 22 Nov 2021 13:47:35 -0500 Authentication-Results: peff.net; auth=none Date: Mon, 22 Nov 2021 13:47:34 -0500 From: Jeff King To: =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason Cc: git@vger.kernel.org, Junio C Hamano , Enzo Matsumiya Subject: Re: [PATCH 5/5] run-command API: remove "argv" member, always use "args" Message-ID: References: <211122.86wnl0xd24.gmgdl@evledraar.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <211122.86wnl0xd24.gmgdl@evledraar.gmail.com> Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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). > 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