Re: [PATCH 06/21] convert manual allocations to argv_array

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Feb 19, 2016 at 6:23 AM, Jeff King <peff@peff.net> wrote:
> There are many manual argv allocations that predate the
> argv_array API. Switching to that API brings a few
> advantages:
>
>   1. We no longer have to manually compute the correct final
>      array size (so it's one less thing we can screw up).
>
>   2. In many cases we had to make a separate pass to count,
>      then allocate, then fill in the array. Now we can do it
>      in one pass, making the code shorter and easier to
>      follow.
>
>   3. argv_array handles memory ownership for us, making it
>      more obvious when things should be free()d and and when
>      not.
>
> Most of these cases are pretty straightforward. In some, we
> switch from "run_command_v" to "run_command" which lets us
> directly use the argv_array embedded in "struct
> child_process".
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/line-log.c b/line-log.c
> @@ -746,23 +747,16 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list
>         if (!rev->diffopt.detect_rename) {
> -               int i, count = 0;
> -               struct line_log_data *r = range;
> -               const char **paths;
> -               while (r) {
> -                       count++;
> -                       r = r->next;
> -               }
> -               paths = xmalloc((count+1)*sizeof(char *));
> -               r = range;
> -               for (i = 0; i < count; i++) {
> -                       paths[i] = xstrdup(r->path);
> -                       r = r->next;
> -               }
> -               paths[count] = NULL;
> +               struct line_log_data *r;
> +               struct argv_array paths = ARGV_ARRAY_INIT;
> +
> +               for (r = range; r; r = r->next)
> +                       argv_array_push(&paths, r->path);
>                 parse_pathspec(&rev->diffopt.pathspec, 0,
> -                              PATHSPEC_PREFER_FULL, "", paths);
> -               free(paths);
> +                              PATHSPEC_PREFER_FULL, "", paths.argv);
> +               /* argv strings are now owned by pathspec */
> +               paths.argc = 0;
> +               argv_array_clear(&paths);

This overly intimate knowledge of the internal implementation of
argv_array_clear() is rather ugly.

>         }
>  }