Re: [PATCH 2/5] upload-archive: use regular "struct child_process" pattern

On Mon, Nov 22 2021, Jeff King wrote:

On Mon, Nov 22, 2021 at 09:53:34PM +0100, Ævar Arnfjörð Bjarmason wrote:

quoted

But as to skipping the "argc > 1" test I've got this still:
    
    @@ -89,9 +89,11 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
             * multiplexed out to our fd#1.  If the child dies, we tell the other
             * end over channel #3.
             */
    -       argv[0] = "upload-archive--writer";
            writer.out = writer.err = -1;
            writer.git_cmd = 1;
    +       strvec_push(&writer.args, "upload-archive--writer");
    +       if (argc > 1)
    +               strvec_pushv(&writer.args, argv + 1);
            if (start_command(&writer)) {
                    int err = errno;
                    packet_write_fmt(1, "NACK unable to spawn subprocess\n");

We'll segfault if we give NULL to strvec_pushv() so we still need that
check. Were you thinking of strvec_pushl(), or am I missing something?

We wouldn't be giving NULL to strvec_pushv(). We'd be giving argv+1,
which is guaranteed non-NULL, but may point to NULL. In that case the
loop condition in strvec_pushv() would turn it into a noop.

D'oh. Thanks. Yes I was stupidly conflating a case where I'd tested it
with strvec_pushl() and moved that dereferenced version to
strvec_pushv(), sorry.