Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 13, 2009, 06:35 UTC
Message-ID: <7v7i3ug7y9.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1234503271-78569-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

Show 5 quoted lines

> +	if (prefix && !prefixcmp(dst, prefix))
> +		return xstrdup(skip_prefix(dst, prefix));
> +	else
> +		return xstrdup(dst);
> +}

I wonder modern compilers are clever enough to optimze the above to something more like:

	pfxlen = prefix ? strlen(prefix) : 0;
        if (pfxlen && !strncmp(dst, prefix, pfxlen))
        	return xstrdup(dst + pfxlen);
	else
        	return xstrdup(dst);

given that skip_prefix is an inline function but prefixcmp is not (anymore), perhaps not.

Show 20 quoted lines

>  static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
>  {
>  	struct ref_list *ref_list = (struct ref_list*)(cb_data);
>  	struct ref_item *newitem;
>  	struct commit *commit;
>  	int kind;
> -	int len;
> +	const char *prefix, *orig_refname = refname;
>  
>  	/* Detect kind */
>  	if (!prefixcmp(refname, "refs/heads/")) {
>  		kind = REF_LOCAL_BRANCH;
>  		refname += 11;
> +		prefix = "refs/heads/";
>  	} else if (!prefixcmp(refname, "refs/remotes/")) {
>  		kind = REF_REMOTE_BRANCH;
>  		refname += 13;
> +		prefix = "refs/remotes/";
>  	} else
>  		return 0;

Once you start making each case arm do more things, it might make sense to rewrite the above unrolled loop into something like this:

	static struct {
        	int kind;
                const char *prefix;
                int pfxlen;
	} ref_kind[] = {
        	{ REF_LOCAL_BRANCH, "refs/heads/", 11 },
        	{ REF_REMOTE_BRANCH, "refs/remotes/", 13 },
	};

	for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
                prefix = ref_kind[i].prefix;
        	if (strncmp(refname, prefix, ref_kind[i].pfxlen))
			continue;
		kind = ref_kind[i].kind;
                refname += ref_kind[i].pfxlen;
                break;
	}
	if (ARRAY_SIZE(ref_kind) <= i)
		return 0;

Then we can later add new elements more easily, e.g.

                { REF_TOPGIT_BASE, "refs/top-base/", 14 },
;-)