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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2014, 23:01 UTC
Message-ID: <xmqqtxdc92ub.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1389192220-13913-4-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 24 quoted lines

> 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 <mhagger@alum.mit.edu>
> ---
> 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.

Show 7 quoted lines

>  		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);