Re: [RFC PATCH 1/5] builtin/annotate.c: simplify for strvec API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> ---
>  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);
>  }




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux