On Sun, Feb 28, 2016 at 07:22:24PM -0300, Gabriel Souza Franco wrote:
> Commit 58f2ed0 (remote-curl: pass ref SHA-1 to fetch-pack as well,
> 2013-12-05) added support for specifying a SHA-1 as well as a ref name.
> Add support for specifying just a SHA-1 and set the ref name to the same
> value in this case.
>
> Signed-off-by: Gabriel Souza Franco <[email protected]>
> ---
>
> Not the cleanest conditional I've ever written, but it should handle
> all cases correctly.
I think it does. But I wonder if it wouldn't be more readable to cover
the three formats independently, like:
if (!get_oid_hex(name, &ref->old_oid) && name[GIT_SHA1_HEXSZ] == ' ') {
/* <sha1> <ref>, find refname */
name += GIT_SHA1_HEXSZ + 1;
} else if (!get_oid_hex(name, &ref->old_oid) && name[GIT_SHA1_HEXSZ] == '\0')
{
/* <sha1>, leave sha1 as name */
} else {
/* <ref>, clear any cruft from get_oid_hex */
oidclr(&ref->old_oid);
}
And as a bonus you get rid of the separate "oid". That does call into
get_oid_hex twice, but I doubt the performance impact is measurable.
We could also do:
if (!get_oid_hex(name, &ref->old_oid)) {
if (name[GIT_SHA1_HEXSZ] == ' ') {
/* <sha1> <ref>, find refname */
name += GIT_SHA1_HEXSZ + 1;
} else if (name[GIT_SHA1_HEXSZ] == '\0') {
/* <sha1>, leave sha1 as name */
} else {
/* <ref>, clear cruft from oid */
oidclr(&ref->old_oid);
}
} else {
/* <ref>, clear cruft from get_oid_hex */
oidclr(&ref->old_oid);
}
if you want to minimize the calls at the expense of having to repeat the
oidclr().
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html