git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] add: Use struct argv_array in run_add_interactive()

From
Thomas Rast <tr@thomasrast.ch>
Date
Mar 16, 2014, 11:42 UTC
Message-ID
<87a9cqxtcy.fsf@thomasrast.ch>
In-Reply-To
<53243620.8080401@gmail.com>
Fabian Ruch <bafain@gmail.com> writes:
Show 5 quoted lines
> run_add_interactive() in builtin/add.c manually computes array bounds
> and allocates a static args array to build the add--interactive command
> line, which is error-prone. Use the argv-array helper functions instead.
>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
Thanks, this is a nicely done cleanup.
Show 46 quoted lines
> ---
>  builtin/add.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 4b045ba..459208a 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -15,6 +15,7 @@
>  #include "diffcore.h"
>  #include "revision.h"
>  #include "bulk-checkin.h"
> +#include "argv-array.h"
>  
>  static const char * const builtin_add_usage[] = {
>  	N_("git add [options] [--] <pathspec>..."),
> @@ -141,23 +142,21 @@ static void refresh(int verbose, const struct pathspec *pathspec)
>  int run_add_interactive(const char *revision, const char *patch_mode,
>  			const struct pathspec *pathspec)
>  {
> +	int status, i;
> +	struct argv_array argv = ARGV_ARRAY_INIT;
>  
> -	args = xcalloc(sizeof(const char *), (pathspec->nr + 6));
> -	ac = 0;
> -	args[ac++] = "add--interactive";
> +	argv_array_push(&argv, "add--interactive");
>  	if (patch_mode)
> -		args[ac++] = patch_mode;
> +		argv_array_push(&argv, patch_mode);
>  	if (revision)
> -		args[ac++] = revision;
> -	args[ac++] = "--";
> +		argv_array_push(&argv, revision);
> +	argv_array_push(&argv, "--");
>  	for (i = 0; i < pathspec->nr; i++)
>  		/* pass original pathspec, to be re-parsed */
> -		args[ac++] = pathspec->items[i].original;
> +		argv_array_push(&argv, pathspec->items[i].original);
>  
> -	status = run_command_v_opt(args, RUN_GIT_CMD);
> -	free(args);
> +	status = run_command_v_opt(argv.argv, RUN_GIT_CMD);
> +	argv_array_clear(&argv);
>  	return status;
>  }
-- 
Thomas Rast
tr@thomasrast.ch
Previous: Fabian RuchNext: Eric Sunshine
Message 3 of 4 in “add: Use struct argv_array in run_add_interactive()”
  1. add: Use struct argv_array in run_add_interactive()Fabian Ruch, Mar 15, 2014
  2. Fabian RuchMar 15, 2014
  3. Thomas RastMar 16, 2014
  4. Eric SunshineMar 17, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.