| Message ID | RFC-patch-1.5-cde038825d0-20221215T090226Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | strvec: add a "nodup" mode, fix memory leaks | expand |
Am 15.12.22 um 10:11 schrieb Ævar Arnfjörð Bjarmason: > When this code was migrated to "struct strvec" (or rather, its > predecessor API) in 8c2cfa55446 (annotate: use argv_array, 2014-07-16) > it didn't take full advantage of what we were given: > > * We are always passed the name "annotate" as argv[0] here, so we > don't need to re-hardcode it. We've already declared it in "struct > cmd_struct commands" in git.c. This change has nothing to do with strvec; it could have been done by e68989a739 (annotate: fix for cvsserver., 2007-02-06) already. I don't see much of a benefit to it because the string is already there, but if it's needed then I think it deserves its own patch. > * We are guaranteed to get at least one argument, so rather than > looping here ourselves let's have strvec_pushv() handle that. If we > only have one argument we'll pass the terminating NULL to it, making > it a NOOP. 85b343245b (argv-array: implement argv_array_pushv(), 2015-06-14) added the _pushv() function, so 8c2cfa55446 could not have used it. > This change helps to make the subsequent commit smaller, and as a > bonus removes the type discrepancy between "int" and "size_t". This type mismatch is benign as long as we get a valid argc value, as positive int fit into size_t. Getting rid of the loop is nice, though. > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/annotate.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/builtin/annotate.c b/builtin/annotate.c > index 58ff977a231..e37b269196f 100644 > --- a/builtin/annotate.c > +++ b/builtin/annotate.c > @@ -7,16 +7,12 @@ > #include "builtin.h" > #include "strvec.h" > > -int cmd_annotate(int argc, const char **argv, const char *prefix) > +int cmd_annotate(int argc UNUSED, const char **argv, const char *prefix) > { > struct strvec args = STRVEC_INIT; > - int i; > > - strvec_pushl(&args, "annotate", "-c", NULL); > - > - for (i = 1; i < argc; i++) { > - strvec_push(&args, argv[i]); > - } > + strvec_pushl(&args, argv[0], "-c", NULL); > + strvec_pushv(&args, &argv[1]); ^^^^^^^^ "argv + 1" would be easier to read. > > return cmd_blame(args.nr, args.v, prefix); > }
diff --git a/builtin/annotate.c b/builtin/annotate.c index 58ff977a231..e37b269196f 100644 --- a/builtin/annotate.c +++ b/builtin/annotate.c @@ -7,16 +7,12 @@ #include "builtin.h" #include "strvec.h" -int cmd_annotate(int argc, const char **argv, const char *prefix) +int cmd_annotate(int argc UNUSED, const char **argv, const char *prefix) { struct strvec args = STRVEC_INIT; - int i; - strvec_pushl(&args, "annotate", "-c", NULL); - - for (i = 1; i < argc; i++) { - strvec_push(&args, argv[i]); - } + strvec_pushl(&args, argv[0], "-c", NULL); + strvec_pushv(&args, &argv[1]); return cmd_blame(args.nr, args.v, prefix); }
When this code was migrated to "struct strvec" (or rather, its predecessor API) in 8c2cfa55446 (annotate: use argv_array, 2014-07-16) it didn't take full advantage of what we were given: * We are always passed the name "annotate" as argv[0] here, so we don't need to re-hardcode it. We've already declared it in "struct cmd_struct commands" in git.c. * We are guaranteed to get at least one argument, so rather than looping here ourselves let's have strvec_pushv() handle that. If we only have one argument we'll pass the terminating NULL to it, making it a NOOP. This change helps to make the subsequent commit smaller, and as a bonus removes the type discrepancy between "int" and "size_t". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/annotate.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)