@matheusbsbs / matheus-bagliano / commits / 9a7bbd1dd1
Fork 0

clean up error conventions of remote.c:match_explicit

match_explicit is called for each push refspec to try to fully resolve the source and destination sides of the refspec. Currently, we look at each refspec and report errors on both the source and the dest side before aborting. It makes sense to report errors for each refspec, since an error in one is independent of an error in the other. However, reporting errors on the 'dst' side of a refspec if there has been an error on the 'src' side does not necessarily make sense, since the interpretation of the 'dst' side depends on the 'src' side (for example, when creating a new unqualified remote ref, we use the same type as the src ref). This patch lets match_explicit return early when the src side of the refspec is bogus. We still look at all of the refspecs before aborting the push, though. At the same time, we clean up the call signature, which previously took an extra "errs" flag. This was pointless, as we didn't act on that flag, but rather just passed it back to the caller. Instead, we now use the more traditional "return -1" to signal an error, and the caller aggregates the error count. This change fixes two bugs, as well: - the early return avoids a segfault when passing a NULL matched_src to guess_ref() - the check for multiple sources pointing to a single dest aborted if the "err" flag was set. Presumably the intent was not to bother with the check if we had no matched_src. However, since the err flag was passed in from the caller, we might abort the check just because a previous refspec had a problem, which doesn't make sense. In practice, this didn't matter, since due to the error flag we end up aborting the push anyway. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 16, 2008 at 12:15 UTC 9a7bbd1dd1db45b474576ca328aa7056cdd081e0
2 files changed +20 -22
remote.c
+11 -21
@@ -867,8 +867,7 @@ static char *guess_ref(const char *name, struct ref *peer)
867
868 static int match_explicit(struct ref *src, struct ref *dst,
869 struct ref ***dst_tail,
870 - struct refspec *rs,
871 - int errs)
870 + struct refspec *rs)
871 {
872 struct ref *matched_src, *matched_dst;
873
@@ -876,7 +875,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
875 char *dst_guess;
876
877 if (rs->pattern || rs->matching)
879 - return errs;
878 + return 0;
879
880 matched_src = matched_dst = NULL;
881 switch (count_refspec_match(rs->src, src, &matched_src)) {
@@ -889,23 +888,16 @@ static int match_explicit(struct ref *src, struct ref *dst,
888 */
889 matched_src = try_explicit_object_name(rs->src);
890 if (!matched_src)
892 - error("src refspec %s does not match any.", rs->src);
891 + return error("src refspec %s does not match any.", rs->src);
892 break;
893 default:
895 - matched_src = NULL;
896 - error("src refspec %s matches more than one.", rs->src);
897 - break;
894 + return error("src refspec %s matches more than one.", rs->src);
895 }
896
900 - if (!matched_src)
901 - errs = 1;
902 -
897 if (!dst_value) {
898 unsigned char sha1[20];
899 int flag;
900
907 - if (!matched_src)
908 - return errs;
901 dst_value = resolve_ref(matched_src->name, sha1, 1, &flag);
902 if (!dst_value ||
903 ((flag & REF_ISSYMREF) &&
@@ -936,18 +928,16 @@ static int match_explicit(struct ref *src, struct ref *dst,
928 dst_value);
929 break;
930 }
939 - if (errs || !matched_dst)
940 - return 1;
941 - if (matched_dst->peer_ref) {
942 - errs = 1;
943 - error("dst ref %s receives from more than one src.",
931 + if (!matched_dst)
932 + return -1;
933 + if (matched_dst->peer_ref)
934 + return error("dst ref %s receives from more than one src.",
935 matched_dst->name);
945 - }
936 else {
937 matched_dst->peer_ref = matched_src;
938 matched_dst->force = rs->force;
939 }
950 - return errs;
940 + return 0;
941 }
942
943 static int match_explicit_refs(struct ref *src, struct ref *dst,
@@ -956,8 +946,8 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
946 {
947 int i, errs;
948 for (i = errs = 0; i < rs_nr; i++)
959 - errs |= match_explicit(src, dst, dst_tail, &rs[i], errs);
960 - return -errs;
949 + errs += match_explicit(src, dst, dst_tail, &rs[i]);
950 + return errs;
951 }
952
953 static const struct refspec *check_pattern_match(const struct refspec *rs,
t/t5510-fetch.sh
+9 -1
@@ -37,7 +37,8 @@ test_expect_success "clone and setup child repos" '
37 echo "Pull: refs/heads/one:refs/heads/one"
38 } >.git/remotes/two &&
39 cd .. &&
40 - git clone . bundle
40 + git clone . bundle &&
41 + git clone . seven
42 '
43
44 test_expect_success "fetch test" '
@@ -295,4 +296,11 @@ test_expect_success 'configured fetch updates tracking' '
296 )
297 '
298
299 +test_expect_success 'pushing nonexistent branch by mistake should not segv' '
300 +
301 + cd "$D" &&
302 + test_must_fail git push seven no:no
303 +
304 +'
305 +
306 test_done