Re: [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()

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

 



On Tue, Apr 07, 2009 at 03:44:35AM -0400, Jeff King wrote:

> +		if (1 != sscanf(ref, scanf_fmts[i], short_name))
> [...]
> +		if (j == i) {
> +			ref += strlen(ref) - strlen(short_name);
> +			break;
> +		}

Actually, I am not sure this is correct, either. It is making the
assumption that the short_name is always a suffix of the ref. But one of
the rev_parse_rules is:

     "refs/remotes/%.*s/HEAD"

for which this is not true. _But_ as it happens, this rule doesn't
actually work in reverse because of the way scanf works with %s. The
code:

    sscanf("refs/remotes/origin/HEAD", "refs/remotes/%s/HEAD", buf);

will put "origin/HEAD" in buf, not "origin"; %s eats until it sees
whitespace (which should not be occuring in a ref, fortunately).

So it actually _is_ correct to assume with the current code that the
short_name is always a suffix, but I am not sure if that is what we
actually want. We will always see "$remote/HEAD" instead of "$remote".

Part of me actually thinks the "incorrect" behavior we are doing now is
actually more explicit and readable. But if that is the case, we should
perhaps simply be excluding that final rule explicitly, and then our
"suffix" assumption will hold.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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]