From 54a8ad925cfac90bb4141c9904b1f97f0c5b83d4 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Jun 2007 23:22:58 -0700 Subject: remote.c: refactor match_explicit_refs() This does not change functionality; just splits one block that is deeply nested and indented out of a huge loop into a separate function. Signed-off-by: Junio C Hamano --- remote.c | 159 ++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 86 insertions(+), 73 deletions(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index 33c8e5055b..b53130fb56 100644 --- a/remote.c +++ b/remote.c @@ -406,90 +406,98 @@ static struct ref *try_explicit_object_name(const char *name) return ref; } -static int match_explicit_refs(struct ref *src, struct ref *dst, - struct ref ***dst_tail, struct refspec *rs, - int rs_nr) +static int match_explicit(struct ref *src, struct ref *dst, + struct ref ***dst_tail, + struct refspec *rs, + int errs) { - int i, errs; - for (i = errs = 0; i < rs_nr; i++) { - struct ref *matched_src, *matched_dst; + struct ref *matched_src, *matched_dst; - const char *dst_value = rs[i].dst; + const char *dst_value = rs->dst; - if (rs[i].pattern) - continue; + if (rs->pattern) + return errs; - if (dst_value == NULL) - dst_value = rs[i].src; + if (dst_value == NULL) + dst_value = rs->src; - matched_src = matched_dst = NULL; - switch (count_refspec_match(rs[i].src, src, &matched_src)) { - case 1: - break; - case 0: - /* The source could be in the get_sha1() format - * not a reference name. :refs/other is a - * way to delete 'other' ref at the remote end. - */ - matched_src = try_explicit_object_name(rs[i].src); - if (matched_src) - break; - errs = 1; - error("src refspec %s does not match any.", - rs[i].src); - break; - default: - errs = 1; - error("src refspec %s matches more than one.", - rs[i].src); - break; - } - switch (count_refspec_match(dst_value, dst, &matched_dst)) { - case 1: - break; - case 0: - if (!memcmp(dst_value, "refs/", 5)) { - int len = strlen(dst_value) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, dst_value, len); - link_dst_tail(matched_dst, dst_tail); - } - else if (!strcmp(rs[i].src, dst_value) && - matched_src) { - /* pushing "master:master" when - * remote does not have master yet. - */ - int len = strlen(matched_src->name) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, matched_src->name, - len); - link_dst_tail(matched_dst, dst_tail); - } - else { - errs = 1; - error("dst refspec %s does not match any " - "existing ref on the remote and does " - "not start with refs/.", dst_value); - } - break; - default: - errs = 1; - error("dst refspec %s matches more than one.", - dst_value); + matched_src = matched_dst = NULL; + switch (count_refspec_match(rs->src, src, &matched_src)) { + case 1: + break; + case 0: + /* The source could be in the get_sha1() format + * not a reference name. :refs/other is a + * way to delete 'other' ref at the remote end. + */ + matched_src = try_explicit_object_name(rs->src); + if (matched_src) break; + errs = 1; + error("src refspec %s does not match any.", + rs->src); + break; + default: + errs = 1; + error("src refspec %s matches more than one.", + rs->src); + break; + } + switch (count_refspec_match(dst_value, dst, &matched_dst)) { + case 1: + break; + case 0: + if (!memcmp(dst_value, "refs/", 5)) { + int len = strlen(dst_value) + 1; + matched_dst = xcalloc(1, sizeof(*dst) + len); + memcpy(matched_dst->name, dst_value, len); + link_dst_tail(matched_dst, dst_tail); } - if (errs) - continue; - if (matched_dst->peer_ref) { - errs = 1; - error("dst ref %s receives from more than one src.", - matched_dst->name); + else if (!strcmp(rs->src, dst_value) && + matched_src) { + /* pushing "master:master" when + * remote does not have master yet. + */ + int len = strlen(matched_src->name) + 1; + matched_dst = xcalloc(1, sizeof(*dst) + len); + memcpy(matched_dst->name, matched_src->name, + len); + link_dst_tail(matched_dst, dst_tail); } else { - matched_dst->peer_ref = matched_src; - matched_dst->force = rs[i].force; + errs = 1; + error("dst refspec %s does not match any " + "existing ref on the remote and does " + "not start with refs/.", dst_value); } + break; + default: + errs = 1; + error("dst refspec %s matches more than one.", + dst_value); + break; + } + if (errs) + return errs; + if (matched_dst->peer_ref) { + errs = 1; + error("dst ref %s receives from more than one src.", + matched_dst->name); } + else { + matched_dst->peer_ref = matched_src; + matched_dst->force = rs->force; + } + return errs; +} + +static int match_explicit_refs(struct ref *src, struct ref *dst, + struct ref ***dst_tail, struct refspec *rs, + int rs_nr) +{ + int i, errs; + for (i = errs = 0; i < rs_nr; i++) + errs |= match_explicit(src, dst, dst_tail, &rs[i], errs); return -errs; } @@ -513,6 +521,11 @@ static const struct refspec *check_pattern_match(const struct refspec *rs, return NULL; } +/* + * Note. This is used only by "push"; refspec matching rules for + * push and fetch are subtly different, so do not try to reuse it + * without thinking. + */ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, int nr_refspec, char **refspec, int all) { -- cgit v1.2.3 From 163f0ee5ad0a5cb7d862431479c270ae3fef79cf Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 00:07:34 -0700 Subject: remote.c: refactor creation of new dst ref This refactors open-coded sequence to create a new "struct ref" and link it to the tail of dst list into a new function. Signed-off-by: Junio C Hamano --- remote.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index b53130fb56..f469fb34e4 100644 --- a/remote.c +++ b/remote.c @@ -406,6 +406,18 @@ static struct ref *try_explicit_object_name(const char *name) return ref; } +static struct ref *make_dst(const char *name, struct ref ***dst_tail) +{ + struct ref *dst; + size_t len; + + len = strlen(name) + 1; + dst = xcalloc(1, sizeof(*dst) + len); + memcpy(dst->name, name, len); + link_dst_tail(dst, dst_tail); + return dst; +} + static int match_explicit(struct ref *src, struct ref *dst, struct ref ***dst_tail, struct refspec *rs, @@ -447,23 +459,13 @@ static int match_explicit(struct ref *src, struct ref *dst, case 1: break; case 0: - if (!memcmp(dst_value, "refs/", 5)) { - int len = strlen(dst_value) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, dst_value, len); - link_dst_tail(matched_dst, dst_tail); - } - else if (!strcmp(rs->src, dst_value) && - matched_src) { + if (!memcmp(dst_value, "refs/", 5)) + matched_dst = make_dst(dst_value, dst_tail); + else if (!strcmp(rs->src, dst_value) && matched_src) /* pushing "master:master" when * remote does not have master yet. */ - int len = strlen(matched_src->name) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, matched_src->name, - len); - link_dst_tail(matched_dst, dst_tail); - } + matched_dst = make_dst(matched_src->name, dst_tail); else { errs = 1; error("dst refspec %s does not match any " @@ -567,11 +569,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, goto free_name; if (!dst_peer) { /* Create a new one and link it */ - int len = strlen(dst_name) + 1; - dst_peer = xcalloc(1, sizeof(*dst_peer) + len); - memcpy(dst_peer->name, dst_name, len); + dst_peer = make_dst(dst_name, dst_tail); hashcpy(dst_peer->new_sha1, src->new_sha1); - link_dst_tail(dst_peer, dst_tail); } dst_peer->peer_ref = src; free_name: -- cgit v1.2.3 From 3c8b7df1ba6dd2093672afc460fd5de0e755f162 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 00:14:04 -0700 Subject: remote.c: minor clean-up of match_explicit() When checking what ref the source refspec matches, we have no business setting the default for the destination, so move that code lower. Also simplify the result from the code block that matches the source side by making it set matched_src only upon unambiguous match. Signed-off-by: Junio C Hamano --- remote.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index f469fb34e4..30abdbb4d9 100644 --- a/remote.c +++ b/remote.c @@ -430,9 +430,6 @@ static int match_explicit(struct ref *src, struct ref *dst, if (rs->pattern) return errs; - if (dst_value == NULL) - dst_value = rs->src; - matched_src = matched_dst = NULL; switch (count_refspec_match(rs->src, src, &matched_src)) { case 1: @@ -445,16 +442,22 @@ static int match_explicit(struct ref *src, struct ref *dst, matched_src = try_explicit_object_name(rs->src); if (matched_src) break; - errs = 1; error("src refspec %s does not match any.", rs->src); break; default: - errs = 1; + matched_src = NULL; error("src refspec %s matches more than one.", rs->src); break; } + + if (!matched_src) + errs = 1; + + if (dst_value == NULL) + dst_value = rs->src; + switch (count_refspec_match(dst_value, dst, &matched_dst)) { case 1: break; @@ -466,21 +469,19 @@ static int match_explicit(struct ref *src, struct ref *dst, * remote does not have master yet. */ matched_dst = make_dst(matched_src->name, dst_tail); - else { - errs = 1; + else error("dst refspec %s does not match any " "existing ref on the remote and does " "not start with refs/.", dst_value); - } break; default: - errs = 1; + matched_dst = NULL; error("dst refspec %s matches more than one.", dst_value); break; } - if (errs) - return errs; + if (errs || matched_dst == NULL) + return 1; if (matched_dst->peer_ref) { errs = 1; error("dst ref %s receives from more than one src.", -- cgit v1.2.3 From 6125796f7d6e8b84431f92c13d7e79bd30f94f53 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 01:23:53 -0700 Subject: remote.c: fix "git push" weak match disambiguation When "git push A:B" is given, and A (or B) is not a full refname that begins with refs/, we require an unambiguous match with an existing ref. For this purpose, a match with a local branch or a tag (i.e. refs/heads/A and refs/tags/A) is called a "strong match", and any other match is called a "weak match". A partial refname is unambiguous when there is only one strong match with any number of weak matches, or when there is only one weak match and no other match. However, as reported by Sparse with Ramsay Jones recently, count_refspec_match() function had a bug where a variable in an inner block masked a different variable of the same name, which caused the weak matches to be ignored. This fixes it, and adds tests for the fix. Signed-off-by: Junio C Hamano --- remote.c | 1 - 1 file changed, 1 deletion(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index 30abdbb4d9..120df36be0 100644 --- a/remote.c +++ b/remote.c @@ -333,7 +333,6 @@ static int count_refspec_match(const char *pattern, for (weak_match = match = 0; refs; refs = refs->next) { char *name = refs->name; int namelen = strlen(name); - int weak_match; if (namelen < patlen || memcmp(name + namelen - patlen, pattern, patlen)) -- cgit v1.2.3 From 1ed10b886bc69c129c06772ee4310c00e001657f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 01:37:14 -0700 Subject: remote.c: "git-push frotz" should update what matches at the source. Earlier, when the local repository has a branch "frotz" and the remote repository has a tag "frotz" (but not branch "frotz"), "git-push frotz" mistakenly updated the tag at the remote side. This was because the partial refname matching code was applied independently on both source and destination side. With this fix, when a colon-less refspec is given to git-push, we first match it with the refs in the source repository, and update the matching ref in the destination repository. Signed-off-by: Junio C Hamano --- remote.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index 120df36be0..754d5130e3 100644 --- a/remote.c +++ b/remote.c @@ -455,7 +455,7 @@ static int match_explicit(struct ref *src, struct ref *dst, errs = 1; if (dst_value == NULL) - dst_value = rs->src; + dst_value = matched_src->name; switch (count_refspec_match(dst_value, dst, &matched_dst)) { case 1: @@ -463,11 +463,6 @@ static int match_explicit(struct ref *src, struct ref *dst, case 0: if (!memcmp(dst_value, "refs/", 5)) matched_dst = make_dst(dst_value, dst_tail); - else if (!strcmp(rs->src, dst_value) && matched_src) - /* pushing "master:master" when - * remote does not have master yet. - */ - matched_dst = make_dst(matched_src->name, dst_tail); else error("dst refspec %s does not match any " "existing ref on the remote and does " -- cgit v1.2.3