From: Junio C Hamano <gitster@pobox.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic
Date: Thu, 09 Jan 2014 15:01:48 -0800 [thread overview]
Message-ID: <xmqqtxdc92ub.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1389192220-13913-4-git-send-email-mhagger@alum.mit.edu> (Michael Haggerty's message of "Wed, 8 Jan 2014 15:43:40 +0100")
Michael Haggerty <mhagger@alum.mit.edu> 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 <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.
> 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);
next prev parent reply other threads:[~2014-01-09 23:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-08 14:43 [PATCH 0/3] Generate scanf_fmts more simply Michael Haggerty
2014-01-08 14:43 ` [PATCH 1/3] shorten_unambiguous_ref(): introduce a new local variable Michael Haggerty
2014-01-08 14:43 ` [PATCH 2/3] gen_scanf_fmt(): delete function and use snprintf() instead Michael Haggerty
2014-01-09 22:56 ` Junio C Hamano
2014-01-08 14:43 ` [PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic Michael Haggerty
2014-01-09 23:01 ` Junio C Hamano [this message]
2014-01-10 14:16 ` Michael Haggerty
2014-01-10 18:03 ` Junio C Hamano
2014-01-14 3:16 ` [PATCH] refname_match(): always use the rules in ref_rev_parse_rules Michael Haggerty
2014-01-14 22:16 ` Junio C Hamano
2014-01-15 16:54 ` Michael Haggerty
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqtxdc92ub.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mhagger@alum.mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.