add: Use struct argv_array in run_add_interactive()
PATCH 1/1 add: Use struct argv_array in run_add_interactive()
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.
--- builtin/add.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
builtin/add.c +10 -11
@@ -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, ac, i; -const char **args; +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; }
Signature · 1 line
1.9.0
Quoted 8 lines · Fabian Ruch
run_add_interactive()inbuiltin/add.cmanually 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 <[email protected]>--- builtin/add.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
I should mention that I am applying to this year's edition of Google's Summer of Code.
Quoted 5 lines · Fabian Ruch
run_add_interactive()inbuiltin/add.cmanually 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 <[email protected]>
Thanks, this is a nicely done cleanup.
Quoted 46 lines
--- builtin/add.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)builtin/add.c +10
-9@@ -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; }
Signature · 2 lines
Thomas Rast [email protected]
Quoted 9 lines · Thomas Rast
Quoted 5 lines · Fabian Ruch
run_add_interactive()inbuiltin/add.cmanually 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 <[email protected]>Thanks, this is a nicely done cleanup.
I second that. Nicely done. You didn't give reviewers any opportunity to provide constructive feedback. :-)
Quoted 46 lines
Quoted 46 lines
--- builtin/add.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)builtin/add.c +10
-9@@ -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; }