Re: [PATCH 01/26] remote-curl.c: convert fetch_git() to use argv_array

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 25, 2016, 06:41 UTC
Message-ID: <CAPig+cQjteE3vgT=UoOne1PX7zmE5JhkqpXUbkCwheRvzkUJgw@mail.gmail.com>
In-Reply-To: <1460552110-5554-2-git-send-email-pclouds@gmail.com>

On Wed, Apr 13, 2016 at 8:54 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 15 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/remote-curl.c b/remote-curl.c
> @@ -726,37 +726,30 @@ static int fetch_git(struct discovery *heads,
>         char *depth_arg = NULL;
> -       int argc = 0, i, err;
> -       const char *argv[17];
> +       int i, err;
> +       struct argv_array args = ARGV_ARRAY_INIT;
> [...]
>         if (options.verbosity >= 3) {
> -               argv[argc++] = "-v";
> -               argv[argc++] = "-v";
> +               argv_array_push(&args, "-v");
> +               argv_array_push(&args, "-v");

A bit more natural might have been:

    argv_array_pushl(&args, "-v", "-v", NULL);

though the diff becomes noisier when the braces get dropped. Not worth a re-roll, of course.

Show 13 quoted lines

>         }
> [...]
> -       if (options.depth) {
> -               struct strbuf buf = STRBUF_INIT;
> -               strbuf_addf(&buf, "--depth=%lu", options.depth);
> -               depth_arg = strbuf_detach(&buf, NULL);
> -               argv[argc++] = depth_arg;
> -       }
> -       argv[argc++] = url.buf;
> -       argv[argc++] = NULL;
> +               argv_array_push(&args, "--no-progress");
> +       if (options.depth)
> +               argv_array_pushf(&args, "--depth=%lu", options.depth);

'depth_arg' becomes unused with this change and can be retired...

Show 5 quoted lines

> +       argv_array_push(&args, url.buf);
> @@ -779,6 +772,7 @@ static int fetch_git(struct discovery *heads,
>         strbuf_release(&rpc.result);
>         strbuf_release(&preamble);
>         free(depth_arg);

...along with this unnecessary free().

> +       argv_array_clear(&args);
>         return err;
>  }