Commit 92159bd46568870264d72741390e387ce5dbe271

Patrick Steinhardt 2018-05-30T12:18:04

path: unify `git_path_is_*` APIs Right now, there's quite a lot of different function calls to determine whether a path component matches a specific name after normalization from the filesystem. We have a function for each of {gitattributes, gitmodules, gitignore} multiplicated with {generic, NTFS, HFS} checks. In the long time, this is unmaintainable in case there are e.g. new filesystems with specific semantics, blowing up the number of functions we need to implement. Replace all functions with a simple `git_path_is_gitfile` function, which accepts an enum pointing out the filename that is to be checked against as well as the filesystem normalizations to check for. This greatly simplifies implementation at the expense of the caller having to invoke a somewhat longer function call.

diff --git a/src/path.c b/src/path.c
index 58a5aaf..77833ee 100644
--- a/src/path.c
+++ b/src/path.c
@@ -1766,14 +1766,14 @@ static bool verify_component(
 	if (flags & GIT_PATH_REJECT_DOT_GIT_HFS) {
 		if (!verify_dotgit_hfs(component, len))
 			return false;
-		if (S_ISLNK(mode) && git_path_is_hfs_dotgit_modules(component, len))
+		if (S_ISLNK(mode) && git_path_is_gitfile(component, len, GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_HFS))
 			return false;
 	}
 
 	if (flags & GIT_PATH_REJECT_DOT_GIT_NTFS) {
 		if (!verify_dotgit_ntfs(repo, component, len))
 			return false;
-		if (S_ISLNK(mode) && git_path_is_ntfs_dotgit_modules(component, len))
+		if (S_ISLNK(mode) && git_path_is_gitfile(component, len, GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_NTFS))
 			return false;
 	}
 
@@ -1872,64 +1872,40 @@ int git_path_normalize_slashes(git_buf *out, const char *path)
 	return 0;
 }
 
-static int verify_dotgit_generic(const char *name, size_t len, const char *dotgit_name, size_t dotgit_len, const char *shortname_pfix)
-{
-	if (!verify_dotgit_ntfs_generic(name, len, dotgit_name, dotgit_len, shortname_pfix))
-		return false;
-
-	return verify_dotgit_hfs_generic(name, len, dotgit_name, dotgit_len);
-}
-
-int git_path_is_ntfs_dotgit_modules(const char *name, size_t len)
-{
-	return !verify_dotgit_ntfs_generic(name, len, "gitmodules", CONST_STRLEN("gitmodules"), "gi7eba");
-}
-
-int git_path_is_hfs_dotgit_modules(const char *name, size_t len)
-{
-	return !verify_dotgit_hfs_generic(name, len, "gitmodules", CONST_STRLEN("gitmodules"));
-}
-
-int git_path_is_dotgit_modules(const char *name, size_t len)
-{
-	if (git_path_is_hfs_dotgit_modules(name, len))
-		return 1;
-
-	return git_path_is_ntfs_dotgit_modules(name, len);
-}
-
-int git_path_is_ntfs_dotgit_ignore(const char *name, size_t len)
-{
-	return !verify_dotgit_ntfs_generic(name, len, "gitignore", CONST_STRLEN("gitignore"), "gi250a");
-}
+static const struct {
+	const char *file;
+	const char *hash;
+	size_t filelen;
+} gitfiles[] = {
+	{ "gitignore", "gi250a", CONST_STRLEN("gitignore") },
+	{ "gitmodules", "gi7eba", CONST_STRLEN("gitmodules") },
+	{ "gitattributes", "gi7d29", CONST_STRLEN("gitattributes") }
+};
 
-int git_path_is_hfs_dotgit_ignore(const char *name, size_t len)
+extern int git_path_is_gitfile(const char *path, size_t pathlen, git_path_gitfile gitfile, git_path_fs fs)
 {
-	return !verify_dotgit_hfs_generic(name, len, "gitignore", CONST_STRLEN("gitignore"));
-}
-
-int git_path_is_dotgit_ignore(const char *name, size_t len)
-{
-	if (git_path_is_hfs_dotgit_ignore(name, len))
-		return 1;
-
-	return git_path_is_ntfs_dotgit_ignore(name, len);
-}
+	const char *file, *hash;
+	size_t filelen;
 
-int git_path_is_hfs_dotgit_attributes(const char *name, size_t len)
-{
-	return !verify_dotgit_hfs_generic(name, len, "gitattributes", CONST_STRLEN("gitattributes"));
-}
-
-int git_path_is_ntfs_dotgit_attributes(const char *name, size_t len)
-{
-	return !verify_dotgit_ntfs_generic(name, len, "gitattributes", CONST_STRLEN("gitattributes"), "gi7d29");
-}
-
-int git_path_is_dotgit_attributes(const char *name, size_t len)
-{
-	if (git_path_is_hfs_dotgit_attributes(name, len))
-		return 1;
+	if (gitfile < 0 && gitfile >= ARRAY_SIZE(gitfiles)) {
+		giterr_set(GITERR_OS, "invalid gitfile for path validation");
+		return -1;
+	}
 
-	return git_path_is_ntfs_dotgit_attributes(name, len);
+	file = gitfiles[gitfile].file;
+	filelen = gitfiles[gitfile].filelen;
+	hash = gitfiles[gitfile].hash;
+
+	switch (fs) {
+	case GIT_PATH_FS_GENERIC:
+		return !verify_dotgit_ntfs_generic(path, pathlen, file, filelen, hash) ||
+		       !verify_dotgit_hfs_generic(path, pathlen, file, filelen);
+	case GIT_PATH_FS_NTFS:
+		return !verify_dotgit_ntfs_generic(path, pathlen, file, filelen, hash);
+	case GIT_PATH_FS_HFS:
+		return !verify_dotgit_hfs_generic(path, pathlen, file, filelen);
+	default:
+		giterr_set(GITERR_OS, "invalid filesystem for path validation");
+		return -1;
+	}
 }
diff --git a/src/path.h b/src/path.h
index fd515c8..fb41f96 100644
--- a/src/path.h
+++ b/src/path.h
@@ -645,76 +645,42 @@ extern bool git_path_isvalid(
  */
 int git_path_normalize_slashes(git_buf *out, const char *path);
 
-/**
- * Check whether a path component corresponds to a .gitmodules file
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_dotgit_modules(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitmodules file in NTFS
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_ntfs_dotgit_modules(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitmodules file in HFS+
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_hfs_dotgit_modules(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitignore file
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_dotgit_ignore(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitignore file in NTFS
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_ntfs_dotgit_ignore(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitignore file in HFS+
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_hfs_dotgit_ignore(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitignore file
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_dotgit_attributes(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitattributes file in NTFS
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_ntfs_dotgit_attributes(const char *name, size_t len);
-
-/**
- * Check whether a path component corresponds to a .gitattributes file in HFS+
- *
- * @param name the path component to check
- * @param len the length of `name`
- */
-extern int git_path_is_hfs_dotgit_attributes(const char *name, size_t len);
+/*
+ * The order needs to stay the same to not break the `gitfiles`
+ * array in path.c
+ */
+typedef enum {
+	GIT_PATH_GITFILE_GITIGNORE,
+	GIT_PATH_GITFILE_GITMODULES,
+	GIT_PATH_GITFILE_GITATTRIBUTES
+} git_path_gitfile;
+
+typedef enum {
+	/* Do both NTFS- and HFS-specific checks */
+	GIT_PATH_FS_GENERIC,
+	/* Do NTFS-specific checks only */
+	GIT_PATH_FS_NTFS,
+	/* Do HFS-specific checks only */
+	GIT_PATH_FS_HFS
+} git_path_fs;
+
+/**
+ * Check whether a path component corresponds to a .git$SUFFIX
+ * file.
+ *
+ * As some filesystems do special things to filenames when
+ * writing files to disk, you cannot always do a plain string
+ * comparison to verify whether a file name matches an expected
+ * path or not. This function can do the comparison for you,
+ * depending on the filesystem you're on.
+ *
+ * @param path the path component to check
+ * @param pathlen the length of `path` that is to be checked
+ * @param gitfile which file to check against
+ * @param fs which filesystem-specific checks to use
+ * @return 0 in case the file does not match, a positive value if
+ *         it does; -1 in case of an error
+ */
+extern int git_path_is_gitfile(const char *path, size_t pathlen, git_path_gitfile gitfile, git_path_fs fs);
 
 #endif
diff --git a/tests/path/dotgit.c b/tests/path/dotgit.c
index 20e585e..3099669 100644
--- a/tests/path/dotgit.c
+++ b/tests/path/dotgit.c
@@ -94,21 +94,21 @@ static char *gitmodules_not_altnames[] = {
 void test_path_dotgit__dotgit_modules(void)
 {
 	size_t i;
-	cl_assert_equal_i(1, git_path_is_dotgit_modules(".gitmodules", strlen(".gitmodules")));
-	cl_assert_equal_i(1, git_path_is_dotgit_modules(".git\xe2\x80\x8cmodules", strlen(".git\xe2\x80\x8cmodules")));
+
+	cl_assert_equal_i(1, git_path_is_gitfile(".gitmodules", strlen(".gitmodules"), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC));
+	cl_assert_equal_i(1, git_path_is_gitfile(".git\xe2\x80\x8cmodules", strlen(".git\xe2\x80\x8cmodules"), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC));
 
 	for (i = 0; i < ARRAY_SIZE(gitmodules_altnames); i++) {
 		const char *name = gitmodules_altnames[i];
-		if (!git_path_is_dotgit_modules(name, strlen(name)))
+		if (!git_path_is_gitfile(name, strlen(name), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC))
 			cl_fail(name);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(gitmodules_not_altnames); i++) {
 		const char *name = gitmodules_not_altnames[i];
-		if (git_path_is_dotgit_modules(name, strlen(name)))
+		if (git_path_is_gitfile(name, strlen(name), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC))
 			cl_fail(name);
 	}
-
 }
 
 void test_path_dotgit__dotgit_modules_symlink(void)