Junio C Hamano <gitster@xxxxxxxxx> writes:
> The immediate cause of the problem is that handle_options() returns a
> wrong count when it sees a "-c <config-specification>". If you look at its
> implementation, you see "handled++" paired with "(*argv)++" everywhere but
> the place it parsed "-c".
I said "immediate" above, because I think the "real" reason why we
suffered is because handle_options() helper function was created by
refactoring existing code with a poor taste in designing an API.
It is understandable that it is more convenent for one callsite, which is
git.c::main(), to make the helper automatically move the argc/argv
variables of the caller, and it certainly would have made the initial
patch look smaller when the helper was introduced, but as an API for a
more general callers, it is easier to follow the resulting code of the
caller if it didn't.
Refactored in a more reasonable way, the result may look like this, on top
of the previous patch.
*CAUTION* I was not very careful checking sites that increment these
variables and rely on the result when I was doing this, so there may be
places that need to be adjusted by adding or subtracting option_count)
git.c | 51 +++++++++++++++++++++++++++------------------------
1 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/git.c b/git.c
index df4306d..3596f5b 100644
--- a/git.c
+++ b/git.c
@@ -64,12 +64,12 @@ static void commit_pager_choice(void) {
}
}
-static int handle_options(const char ***argv, int *argc, int *envchanged)
+static int handle_options(int argc, const char **argv, int *envchanged)
{
- const char **orig_argv = *argv;
+ const char **orig_argv = argv;
- while (*argc > 0) {
- const char *cmd = (*argv)[0];
+ while (argc > 0) {
+ const char *cmd = argv[0];
if (cmd[0] != '-')
break;
@@ -107,29 +107,29 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--git-dir")) {
- if (*argc < 2) {
+ if (argc < 2) {
fprintf(stderr, "No directory given for --git-dir.\n" );
usage(git_usage_string);
}
- setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
+ setenv(GIT_DIR_ENVIRONMENT, argv[1], 1);
if (envchanged)
*envchanged = 1;
- (*argv)++;
- (*argc)--;
+ argv++;
+ argc--;
} else if (!prefixcmp(cmd, "--git-dir=")) {
setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--work-tree")) {
- if (*argc < 2) {
+ if (argc < 2) {
fprintf(stderr, "No directory given for --work-tree.\n" );
usage(git_usage_string);
}
- setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1);
+ setenv(GIT_WORK_TREE_ENVIRONMENT, argv[1], 1);
if (envchanged)
*envchanged = 1;
- (*argv)++;
- (*argc)--;
+ argv++;
+ argc--;
} else if (!prefixcmp(cmd, "--work-tree=")) {
setenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
if (envchanged)
@@ -141,22 +141,22 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "-c")) {
- if (*argc < 2) {
+ if (argc < 2) {
fprintf(stderr, "-c expects a configuration string\n" );
usage(git_usage_string);
}
- git_config_push_parameter((*argv)[1]);
- (*argv)++;
- (*argc)--;
+ git_config_push_parameter(argv[1]);
+ argv++;
+ argc--;
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
}
- (*argv)++;
- (*argc)--;
+ argv++;
+ argc--;
}
- return (*argv) - orig_argv;
+ return argv - orig_argv;
}
static int handle_alias(int *argcp, const char ***argv)
@@ -198,14 +198,14 @@ static int handle_alias(int *argcp, const char ***argv)
if (count < 0)
die("Bad alias.%s string: %s", alias_command,
split_cmdline_strerror(count));
- option_count = handle_options(&new_argv, &count, &envchanged);
+ option_count = handle_options(count, new_argv, &envchanged);
+ count -= option_count;
if (envchanged)
die("alias '%s' changes environment variables\n"
"You can use '!git' in the alias to do this.",
alias_command);
- memmove(new_argv - option_count, new_argv,
- count * sizeof(char *));
- new_argv -= option_count;
+ memmove(new_argv, new_argv + option_count,
+ count * sizeof(char *));
if (count < 1)
die("empty alias for %s", alias_command);
@@ -508,6 +508,7 @@ static int run_argv(int *argcp, const char ***argv)
int main(int argc, const char **argv)
{
const char *cmd;
+ int option_count;
startup_info = &git_startup_info;
@@ -535,7 +536,9 @@ int main(int argc, const char **argv)
/* Look for flags.. */
argv++;
argc--;
- handle_options(&argv, &argc, NULL);
+ option_count = handle_options(argc, argv, NULL);
+ argv += option_count;
+ argc -= option_count;
if (argc > 0) {
if (!prefixcmp(argv[0], "--"))
argv[0] += 2;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html