Throughout most of parse_args(), the variable 'i' remains at 0. Many
references are still made to the variable even when it could only have
the value 0. This made at least me, who has relatively little
experience with C programming styles, think that parts of the function
was meant to be part of a loop. To avoid such confusion, remove the
variable and also the 'argc' parameter and check for NULL trailing
argv instead.
Signed-off-by: Martin von Zweigbergk <martinvonz@xxxxxxxxx>
---
I explained a bit more why I was confused by the current style, but
I'm also perfectly happy if you just drop the patch (there would be
some minor conflicts in a later patch, though).
builtin/reset.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 58f0f61..d89cf4d 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,9 +198,8 @@ static void die_if_unmerged_cache(int reset_type)
}
-static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret)
+static const char **parse_args(const char **argv, const char *prefix, const char **rev_ret)
{
- int i = 0;
const char *rev = "HEAD";
unsigned char unused[20];
/*
@@ -211,34 +210,34 @@ static const char **parse_args(int argc, const char **argv, const char *prefix,
* git reset [-opts] -- <paths>...
* git reset [-opts] <paths>...
*
- * At this point, argv[i] points immediately after [-opts].
+ * At this point, argv points immediately after [-opts].
*/
- if (i < argc) {
- if (!strcmp(argv[i], "--")) {
- i++; /* reset to HEAD, possibly with paths */
- } else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
- rev = argv[i];
- i += 2;
+ if (argv[0]) {
+ if (!strcmp(argv[0], "--")) {
+ argv++; /* reset to HEAD, possibly with paths */
+ } else if (argv[1] && !strcmp(argv[1], "--")) {
+ rev = argv[0];
+ argv += 2;
}
/*
- * Otherwise, argv[i] could be either <rev> or <paths> and
+ * Otherwise, argv[0] could be either <rev> or <paths> and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], unused)) {
+ else if (!get_sha1_committish(argv[0], unused)) {
/*
- * Ok, argv[i] looks like a rev; it should not
+ * Ok, argv[0] looks like a rev; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[i]);
- rev = argv[i++];
+ verify_non_filename(prefix, argv[0]);
+ rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[i], 1);
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
- return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+ return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
int cmd_reset(int argc, const char **argv, const char *prefix)
@@ -270,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
- pathspec = parse_args(argc, argv, prefix, &rev);
+ pathspec = parse_args(argv, prefix, &rev);
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid ref."), rev);
--
1.8.1.1.454.gce43f05
--
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