Subject: Re: [PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

From: Junio C Hamano <[email-masked]>
Date: Thu, 09 Jan 2014 15:01:48 -0800

Michael Haggerty <[email-masked]> writes:

> As long as we're being pathologically stingy with mallocs, we might as
> well do the math right and save 6 (!) bytes.
>
> Signed-off-by: Michael Haggerty <[email-masked]>
> ---
> It is left to the reader to show how another 7 bytes could be saved
> (11 bytes on a 64-bit architecture!)
>
> It probably wouldn't kill performance to use a string_list here
> instead.
>
>  refs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index ef9cdea..63b3a71 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3351,10 +3351,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
>  		size_t total_len = 0;
>  		size_t offset = 0;
>  
> -		/* the rule list is NULL terminated, count them first */
> +		/* the rule list is NUL terminated, count them first */

I think this _is_ wrong; it talks about the NULL termination of the
ref_rev_parse_rules[] array, not each string that is an element of
the array being NUL terminated.

Output from "git grep -e refname_match -e ref_rev_parse_rules"
suggests me that we actually could make ref_rev_parse_rules[] a
file-scope static to refs.c, remove its NULL termination and convert
all the iterators of the array to use ARRAY_SIZE() on it, after
dropping the third parameter to refname_match().  That way, we do
not have to count them first here.

But that is obviously a separate topic.

>  		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
> -			/* no +1 because strlen("%s") < strlen("%.*s") */
> -			total_len += strlen(ref_rev_parse_rules[nr_rules]);
> +			/* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
> +			total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
>  
>  		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);