Add tests and improve param checks

Fixed some minor `git_repository_hashfile` issues:

- Fixed incorrect doc (saying that repo could be NULL)
- Added checking of object type value to acceptable ones
- Added more tests for various parameter permutations

include/git2/repository.h

@@ -490,8 +490,7 @@ GIT_EXTERN(int) git_repository_message_remove(git_repository *repo);
  * crlf filters) before generating the SHA, then use this function.
  *
  * @param out Output value of calculated SHA
- * @param repo Repository pointer.  NULL is allowed to just use global and
- *             system attributes for choosing filters.
+ * @param repo Repository pointer
  * @param path Path to file on disk whose contents should be hashed. If the
  *             repository is not NULL, this can be a relative path.
  * @param type The object type to hash as (e.g. GIT_OBJ_BLOB)

src/odb.c

@@ -117,6 +117,11 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 	git_hash_ctx *ctx;
 	ssize_t read_len;
 
+	if (!git_object_typeisloose(type)) {
+		giterr_set(GITERR_INVALID, "Invalid object type for hash");
+		return -1;
+	}
+
 	hdr_len = format_object_header(hdr, sizeof(hdr), size, type);
 
 	ctx = git_hash_new_ctx();

src/repository.c

@@ -1388,7 +1388,12 @@ int git_repository_hashfile(
 	git_off_t len;
 	git_buf full_path = GIT_BUF_INIT;
 
-	assert(out && path); /* repo and as_path can be NULL */
+	assert(out && path && repo); /* as_path can be NULL */
+
+	/* At some point, it would be nice if repo could be NULL to just
+	 * apply filter rules defined in system and global files, but for
+	 * now that is not possible because git_filters_load() needs it.
+	 */
 
 	error = git_path_join_unrooted(
 		&full_path, path, repo ? git_repository_workdir(repo) : NULL, NULL);

tests-clar/repo/hashfile.c

@@ -19,16 +19,22 @@ void test_repo_hashfile__simple(void)
 	git_oid a, b;
 	git_buf full = GIT_BUF_INIT;
 
+	/* hash with repo relative path */
 	cl_git_pass(git_odb_hashfile(&a, "status/current_file", GIT_OBJ_BLOB));
 	cl_git_pass(git_repository_hashfile(&b, _repo, "current_file", GIT_OBJ_BLOB, NULL));
 	cl_assert(git_oid_equal(&a, &b));
 
 	cl_git_pass(git_buf_joinpath(&full, git_repository_workdir(_repo), "current_file"));
 
+	/* hash with full path */
 	cl_git_pass(git_odb_hashfile(&a, full.ptr, GIT_OBJ_BLOB));
 	cl_git_pass(git_repository_hashfile(&b, _repo, full.ptr, GIT_OBJ_BLOB, NULL));
 	cl_assert(git_oid_equal(&a, &b));
 
+	/* hash with invalid type */
+	cl_git_fail(git_odb_hashfile(&a, full.ptr, GIT_OBJ_ANY));
+	cl_git_fail(git_repository_hashfile(&b, _repo, full.ptr, GIT_OBJ_OFS_DELTA, NULL));
+
 	git_buf_free(&full);
 }
 
@@ -43,13 +49,40 @@ void test_repo_hashfile__filtered(void)
 
 	cl_git_append2file("status/.gitattributes", "*.txt text\n*.bin binary\n\n");
 
-	cl_git_mkfile("status/testfile.txt", "content\r\n"); /* Content with CRLF */
+	/* create some sample content with CRLF in it */
+	cl_git_mkfile("status/testfile.txt", "content\r\n");
+	cl_git_mkfile("status/testfile.bin", "other\r\nstuff\r\n");
 
+	/* not equal hashes because of filtering */
 	cl_git_pass(git_odb_hashfile(&a, "status/testfile.txt", GIT_OBJ_BLOB));
 	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.txt", GIT_OBJ_BLOB, NULL));
-	cl_assert(git_oid_cmp(&a, &b)); /* not equal */
+	cl_assert(git_oid_cmp(&a, &b));
+
+	/* equal hashes because filter is binary */
+	cl_git_pass(git_odb_hashfile(&a, "status/testfile.bin", GIT_OBJ_BLOB));
+	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.bin", GIT_OBJ_BLOB, NULL));
+	cl_assert(git_oid_equal(&a, &b));
 
+	/* equal hashes when 'as_file' points to binary filtering */
 	cl_git_pass(git_odb_hashfile(&a, "status/testfile.txt", GIT_OBJ_BLOB));
-	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.txt", GIT_OBJ_BLOB, "testfile.bin"));
-	cl_assert(git_oid_equal(&a, &b)); /* equal when 'binary' 'as_file' name is used */
+	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.txt", GIT_OBJ_BLOB, "foo.bin"));
+	cl_assert(git_oid_equal(&a, &b));
+
+	/* not equal hashes when 'as_file' points to text filtering */
+	cl_git_pass(git_odb_hashfile(&a, "status/testfile.bin", GIT_OBJ_BLOB));
+	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.bin", GIT_OBJ_BLOB, "foo.txt"));
+	cl_assert(git_oid_cmp(&a, &b));
+
+	/* equal hashes when 'as_file' is empty and turns off filtering */
+	cl_git_pass(git_odb_hashfile(&a, "status/testfile.txt", GIT_OBJ_BLOB));
+	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.txt", GIT_OBJ_BLOB, ""));
+	cl_assert(git_oid_equal(&a, &b));
+
+	cl_git_pass(git_odb_hashfile(&a, "status/testfile.bin", GIT_OBJ_BLOB));
+	cl_git_pass(git_repository_hashfile(&b, _repo, "testfile.bin", GIT_OBJ_BLOB, ""));
+	cl_assert(git_oid_equal(&a, &b));
+
+	/* some hash type failures */
+	cl_git_fail(git_odb_hashfile(&a, "status/testfile.txt", 0));
+	cl_git_fail(git_repository_hashfile(&b, _repo, "testfile.txt", GIT_OBJ_ANY, NULL));
 }