diff options
| -rw-r--r-- | include/git2/revparse.h | 3 | ||||
| -rw-r--r-- | src/common.h | 4 | ||||
| -rw-r--r-- | src/errors.c | 9 | ||||
| -rw-r--r-- | src/revparse.c | 85 | ||||
| -rw-r--r-- | tests-clar/refs/revparse.c | 81 |
5 files changed, 104 insertions, 78 deletions
diff --git a/include/git2/revparse.h b/include/git2/revparse.h index 4567027e5..3df6fef7f 100644 --- a/include/git2/revparse.h +++ b/include/git2/revparse.h @@ -27,7 +27,8 @@ GIT_BEGIN_DECL * @param out pointer to output object * @param repo the repository to search in * @param spec the textual specification for an object - * @return on success, GIT_ERROR otherwise (use git_error_last for information about the error) + * @return 0 on success, GIT_ENOTFOUND, GIT_EAMBIGUOUS, + * GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_revparse_single(git_object **out, git_repository *repo, const char *spec); diff --git a/src/common.h b/src/common.h index a35239e3d..b3b551508 100644 --- a/src/common.h +++ b/src/common.h @@ -61,9 +61,9 @@ void giterr_set(int error_class, const char *string, ...); /** * Set the error message for a regex failure, using the internal regex - * error code lookup. + * error code lookup and return a libgit error code. */ -void giterr_set_regex(const regex_t *regex, int error_code); +int giterr_set_regex(const regex_t *regex, int error_code); /* NOTE: other giterr functions are in the public errors.h header file */ diff --git a/src/errors.c b/src/errors.c index ac7fa934d..e62507216 100644 --- a/src/errors.c +++ b/src/errors.c @@ -93,11 +93,18 @@ void giterr_set_str(int error_class, const char *string) set_error(error_class, message); } -void giterr_set_regex(const regex_t *regex, int error_code) +int giterr_set_regex(const regex_t *regex, int error_code) { char error_buf[1024]; regerror(error_code, regex, error_buf, sizeof(error_buf)); giterr_set_str(GITERR_REGEX, error_buf); + + if (error_code == REG_NOMATCH) + return GIT_ENOTFOUND; + else if (error_code > REG_BADPAT) + return GIT_EINVALIDSPEC; + else + return -1; } void giterr_clear(void) diff --git a/src/revparse.c b/src/revparse.c index 308b92923..ade03d0e4 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -13,12 +13,6 @@ #include "git2.h" -static int revspec_error(const char *revspec) -{ - giterr_set(GITERR_INVALID, "Failed to parse revision specifier - Invalid pattern '%s'", revspec); - return -1; -} - static int disambiguate_refname(git_reference **out, git_repository *repo, const char *refname) { int error, i; @@ -51,7 +45,7 @@ static int disambiguate_refname(git_reference **out, git_repository *repo, const goto cleanup; if (!git_reference_is_valid_name(git_buf_cstr(&refnamebuf))) { - error = GIT_ENOTFOUND; + error = GIT_EINVALIDSPEC; continue; } @@ -90,17 +84,18 @@ static int build_regex(regex_t *regex, const char *pattern) if (*pattern == '\0') { giterr_set(GITERR_REGEX, "Empty pattern"); - return -1; + return GIT_EINVALIDSPEC; } error = regcomp(regex, pattern, REG_EXTENDED); if (!error) return 0; - giterr_set_regex(regex, error); + error = giterr_set_regex(regex, error); + regfree(regex); - return -1; + return error; } static int maybe_describe(git_object**out, git_repository *repo, const char *spec) @@ -174,7 +169,7 @@ static int try_parse_numeric(int *n, const char *curly_braces_content) return 0; } -static int retrieve_previously_checked_out_branch_or_revision(git_object **out, git_reference **base_ref, git_repository *repo, const char *spec, const char *identifier, size_t position) +static int retrieve_previously_checked_out_branch_or_revision(git_object **out, git_reference **base_ref, git_repository *repo, const char *identifier, size_t position) { git_reference *ref = NULL; git_reflog *reflog = NULL; @@ -189,7 +184,7 @@ static int retrieve_previously_checked_out_branch_or_revision(git_object **out, cur = position; if (*identifier != '\0' || *base_ref != NULL) - return revspec_error(spec); + return GIT_EINVALIDSPEC; if (build_regex(&preg, "checkout: moving from (.*) to .*") < 0) return -1; @@ -332,6 +327,11 @@ static int retrieve_remote_tracking_reference(git_reference **base_ref, const ch *base_ref = NULL; } + if (!git_reference_is_branch(ref)) { + error = GIT_EINVALIDSPEC; + goto cleanup; + } + if ((error = git_branch_tracking(&tracking, ref)) < 0) goto cleanup; @@ -357,13 +357,13 @@ static int handle_at_syntax(git_object **out, git_reference **ref, const char *s is_numeric = !try_parse_numeric(&parsed, curly_braces_content); if (*curly_braces_content == '-' && (!is_numeric || parsed == 0)) { - error = revspec_error(spec); + error = GIT_EINVALIDSPEC; goto cleanup; } if (is_numeric) { if (parsed < 0) - error = retrieve_previously_checked_out_branch_or_revision(out, ref, repo, spec, git_buf_cstr(&identifier), -parsed); + error = retrieve_previously_checked_out_branch_or_revision(out, ref, repo, git_buf_cstr(&identifier), -parsed); else error = retrieve_revobject_from_reflog(out, ref, repo, git_buf_cstr(&identifier), parsed); @@ -416,8 +416,9 @@ static int handle_caret_parent_syntax(git_object **out, git_object *obj, int n) git_object *temp_commit = NULL; int error; - if (git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT) < 0) - return -1; + if ((error = git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT)) < 0) + return (error == GIT_EAMBIGUOUS || error == GIT_ENOTFOUND) ? + GIT_EINVALIDSPEC : error; if (n == 0) { *out = temp_commit; @@ -435,8 +436,9 @@ static int handle_linear_syntax(git_object **out, git_object *obj, int n) git_object *temp_commit = NULL; int error; - if (git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT) < 0) - return -1; + if ((error = git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT)) < 0) + return (error == GIT_EAMBIGUOUS || error == GIT_ENOTFOUND) ? + GIT_EINVALIDSPEC : error; error = git_commit_nth_gen_ancestor((git_commit **)out, (git_commit*)temp_commit, n); @@ -453,8 +455,8 @@ static int handle_colon_syntax( int error = -1; git_tree_entry *entry = NULL; - if (git_object_peel(&tree, obj, GIT_OBJ_TREE) < 0) - return -1; + if ((error = git_object_peel(&tree, obj, GIT_OBJ_TREE)) < 0) + return error == GIT_ENOTFOUND ? GIT_EINVALIDSPEC : error; if (*path == '\0') { *out = tree; @@ -507,21 +509,21 @@ static int handle_grep_syntax(git_object **out, git_repository *repo, const git_ { regex_t preg; git_revwalk *walk = NULL; - int error = -1; + int error; - if (build_regex(&preg, pattern) < 0) - return -1; + if ((error = build_regex(&preg, pattern)) < 0) + return error; - if (git_revwalk_new(&walk, repo) < 0) + if ((error = git_revwalk_new(&walk, repo)) < 0) goto cleanup; git_revwalk_sorting(walk, GIT_SORT_TIME); if (spec_oid == NULL) { // TODO: @carlosmn: The glob should be refs/* but this makes git_revwalk_next() fails - if (git_revwalk_push_glob(walk, GIT_REFS_HEADS_DIR "*") < 0) + if ((error = git_revwalk_push_glob(walk, GIT_REFS_HEADS_DIR "*")) < 0) goto cleanup; - } else if (git_revwalk_push(walk, spec_oid) < 0) + } else if ((error = git_revwalk_push(walk, spec_oid)) < 0) goto cleanup; error = walk_and_search(out, walk, &preg); @@ -546,7 +548,7 @@ static int handle_caret_curly_syntax(git_object **out, git_object *obj, const ch expected_type = parse_obj_type(curly_braces_content); if (expected_type == GIT_OBJ_BAD) - return -1; + return GIT_EINVALIDSPEC; return git_object_peel(out, obj, expected_type); } @@ -560,13 +562,13 @@ static int extract_curly_braces_content(git_buf *buf, const char *spec, size_t * (*pos)++; if (spec[*pos] == '\0' || spec[*pos] != '{') - return revspec_error(spec); + return GIT_EINVALIDSPEC; (*pos)++; while (spec[*pos] != '}') { if (spec[*pos] == '\0') - return revspec_error(spec); + return GIT_EINVALIDSPEC; git_buf_putc(buf, spec[(*pos)++]); } @@ -610,7 +612,7 @@ static int extract_how_many(int *n, const char *spec, size_t *pos) if (git__isdigit(spec[*pos])) { if ((git__strtol32(&parsed, spec + *pos, &end_ptr, 10) < 0) < 0) - return revspec_error(spec); + return GIT_EINVALIDSPEC; accumulated += (parsed - 1); *pos = end_ptr - spec; @@ -655,7 +657,7 @@ static int ensure_base_rev_loaded(git_object **object, git_reference **reference } if (!allow_empty_identifier && identifier_len == 0) - return revspec_error(spec); + return GIT_EINVALIDSPEC; if (git_buf_put(&identifier, spec, identifier_len) < 0) return -1; @@ -666,12 +668,12 @@ static int ensure_base_rev_loaded(git_object **object, git_reference **reference return error; } -static int ensure_base_rev_is_not_known_yet(git_object *object, const char *spec) +static int ensure_base_rev_is_not_known_yet(git_object *object) { if (object == NULL) return 0; - return revspec_error(spec); + return GIT_EINVALIDSPEC; } static bool any_left_hand_identifier(git_object *object, git_reference *reference, size_t identifier_len) @@ -688,12 +690,12 @@ static bool any_left_hand_identifier(git_object *object, git_reference *referenc return false; } -static int ensure_left_hand_identifier_is_not_known_yet(git_object *object, git_reference *reference, const char *spec) +static int ensure_left_hand_identifier_is_not_known_yet(git_object *object, git_reference *reference) { - if (!ensure_base_rev_is_not_known_yet(object, spec) && reference == NULL) + if (!ensure_base_rev_is_not_known_yet(object) && reference == NULL) return 0; - return revspec_error(spec); + return GIT_EINVALIDSPEC; } int git_revparse_single(git_object **out, git_repository *repo, const char *spec) @@ -800,7 +802,7 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec if ((error = extract_curly_braces_content(&buf, spec, &pos)) < 0) goto cleanup; - if ((error = ensure_base_rev_is_not_known_yet(base_rev, spec)) < 0) + if ((error = ensure_base_rev_is_not_known_yet(base_rev)) < 0) goto cleanup; if ((error = handle_at_syntax(&temp_object, &reference, spec, identifier_len, repo, git_buf_cstr(&buf))) < 0) @@ -815,7 +817,7 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec } default: - if ((error = ensure_left_hand_identifier_is_not_known_yet(base_rev, reference, spec)) < 0) + if ((error = ensure_left_hand_identifier_is_not_known_yet(base_rev, reference)) < 0) goto cleanup; pos++; @@ -830,8 +832,13 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec error = 0; cleanup: - if (error) + if (error) { + if (error == GIT_EINVALIDSPEC) + giterr_set(GITERR_INVALID, + "Failed to parse revision specifier - Invalid pattern '%s'", spec); + git_object_free(base_rev); + } git_reference_free(reference); git_buf_free(&buf); return error; diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c index 3fb5c3f72..81a6bc469 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -49,13 +49,17 @@ void test_refs_revparse__nonexistant_object(void) test_object("this-does-not-exist~2", NULL); } -void test_refs_revparse__invalid_reference_name(void) +static void assert_invalid_spec(const char *invalid_spec) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense^1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense~2")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "")); + cl_assert_equal_i( + GIT_EINVALIDSPEC, git_revparse_single(&g_obj, g_repo, invalid_spec)); +} +void test_refs_revparse__invalid_reference_name(void) +{ + assert_invalid_spec("this doesn't make sense"); + assert_invalid_spec("Inv@{id"); + assert_invalid_spec(""); } void test_refs_revparse__shas(void) @@ -94,9 +98,11 @@ void test_refs_revparse__describe_output(void) void test_refs_revparse__nth_parent(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "be3563a^-1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "^")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "be3563a^{tree}^")); + assert_invalid_spec("be3563a^-1"); + assert_invalid_spec("^"); + assert_invalid_spec("be3563a^{tree}^"); + assert_invalid_spec("point_to_blob^{blob}^"); + assert_invalid_spec("this doesn't make sense^1"); test_object("be3563a^1", "9fd738e8f7967c078dceed8190330fc8648ee56a"); test_object("be3563a^", "9fd738e8f7967c078dceed8190330fc8648ee56a"); @@ -123,8 +129,10 @@ void test_refs_revparse__not_tag(void) void test_refs_revparse__to_type(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{blob}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{trip}")); + assert_invalid_spec("wrapped_tag^{trip}"); + test_object("point_to_blob^{commit}", NULL); + cl_assert_equal_i( + GIT_EAMBIGUOUS, git_revparse_single(&g_obj, g_repo, "wrapped_tag^{blob}")); test_object("wrapped_tag^{commit}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("wrapped_tag^{tree}", "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162"); @@ -134,11 +142,15 @@ void test_refs_revparse__to_type(void) void test_refs_revparse__linear_history(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "~")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "foo~bar")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master~bar")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master~-1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master~0bar")); + assert_invalid_spec("~"); + test_object("foo~bar", NULL); + + assert_invalid_spec("master~bar"); + assert_invalid_spec("master~-1"); + assert_invalid_spec("master~0bar"); + assert_invalid_spec("this doesn't make sense~2"); + assert_invalid_spec("be3563a^{tree}~"); + assert_invalid_spec("point_to_blob^{blob}~"); test_object("master~0", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("master~1", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -149,10 +161,10 @@ void test_refs_revparse__linear_history(void) void test_refs_revparse__chaining(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{0}@{0}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{u}@{-1}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-1}@{-1}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-3}@{0}")); + assert_invalid_spec("master@{0}@{0}"); + assert_invalid_spec("@{u}@{-1}"); + assert_invalid_spec("@{-1}@{-1}"); + assert_invalid_spec("@{-3}@{0}"); test_object("master@{0}~1^1", "9fd738e8f7967c078dceed8190330fc8648ee56a"); test_object("@{u}@{0}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -168,8 +180,9 @@ void test_refs_revparse__chaining(void) void test_refs_revparse__upstream(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "e90810b@{u}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "refs/tags/e90810b@{u}")); + assert_invalid_spec("e90810b@{u}"); + assert_invalid_spec("refs/tags/e90810b@{u}"); + test_object("refs/heads/e90810b@{u}", NULL); test_object("master@{upstream}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); test_object("@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -180,7 +193,7 @@ void test_refs_revparse__upstream(void) void test_refs_revparse__ordinal(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{-2}")); + assert_invalid_spec("master@{-2}"); /* TODO: make the test below actually fail * cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{1a}")); @@ -202,9 +215,9 @@ void test_refs_revparse__ordinal(void) void test_refs_revparse__previous_head(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-1b}")); + assert_invalid_spec("@{-xyz}"); + assert_invalid_spec("@{-0}"); + assert_invalid_spec("@{-1b}"); test_object("@{-42}", NULL); @@ -261,9 +274,9 @@ void test_refs_revparse__reflog_of_a_ref_under_refs(void) void test_refs_revparse__revwalk(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/not found in any commit}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/merge}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/((}")); + test_object("master^{/not found in any commit}", NULL); + test_object("master^{/merge}", NULL); + assert_invalid_spec("master^{/((}"); test_object("master^{/anoth}", "5b5b025afb0b4c913b4c338a42934a3863bf3644"); test_object("master^{/Merge}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -344,8 +357,9 @@ void test_refs_revparse__date(void) void test_refs_revparse__colon(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, ":/")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, ":2:README")); + assert_invalid_spec(":/"); + assert_invalid_spec("point_to_blob:readme.txt"); + cl_git_fail(git_revparse_single(&g_obj, g_repo, ":2:README")); /* Not implemented */ test_object(":/not found in any commit", NULL); test_object("subtrees:ab/42.txt", NULL); @@ -435,11 +449,8 @@ void test_refs_revparse__disambiguation(void) void test_refs_revparse__a_too_short_objectid_returns_EAMBIGUOUS(void) { - int result; - - result = git_revparse_single(&g_obj, g_repo, "e90"); - - cl_assert_equal_i(GIT_EAMBIGUOUS, result); + cl_assert_equal_i( + GIT_EAMBIGUOUS, git_revparse_single(&g_obj, g_repo, "e90")); } void test_refs_revparse__issue_994(void) |