Skip to content

Commit 92159bd

Browse files
pks-tpks-t
committed
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.
1 parent 771dfd1 commit 92159bd

File tree

3 files changed

+76
-134
lines changed

3 files changed

+76
-134
lines changed

‎src/path.c‎

Lines changed: 34 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,14 +1766,14 @@ static bool verify_component(
17661766
if (flags & GIT_PATH_REJECT_DOT_GIT_HFS) {
17671767
if (!verify_dotgit_hfs(component, len))
17681768
return false;
1769-
if (S_ISLNK(mode) && git_path_is_hfs_dotgit_modules(component, len))
1769+
if (S_ISLNK(mode) && git_path_is_gitfile(component, len, GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_HFS))
17701770
return false;
17711771
}
17721772

17731773
if (flags & GIT_PATH_REJECT_DOT_GIT_NTFS) {
17741774
if (!verify_dotgit_ntfs(repo, component, len))
17751775
return false;
1776-
if (S_ISLNK(mode) && git_path_is_ntfs_dotgit_modules(component, len))
1776+
if (S_ISLNK(mode) && git_path_is_gitfile(component, len, GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_NTFS))
17771777
return false;
17781778
}
17791779

@@ -1872,64 +1872,40 @@ int git_path_normalize_slashes(git_buf *out, const char *path)
18721872
return 0;
18731873
}
18741874

1875-
static int verify_dotgit_generic(const char *name, size_t len, const char *dotgit_name, size_t dotgit_len, const char *shortname_pfix)
1876-
{
1877-
if (!verify_dotgit_ntfs_generic(name, len, dotgit_name, dotgit_len, shortname_pfix))
1878-
return false;
1879-
1880-
return verify_dotgit_hfs_generic(name, len, dotgit_name, dotgit_len);
1881-
}
1882-
1883-
int git_path_is_ntfs_dotgit_modules(const char *name, size_t len)
1884-
{
1885-
return !verify_dotgit_ntfs_generic(name, len, "gitmodules", CONST_STRLEN("gitmodules"), "gi7eba");
1886-
}
1887-
1888-
int git_path_is_hfs_dotgit_modules(const char *name, size_t len)
1889-
{
1890-
return !verify_dotgit_hfs_generic(name, len, "gitmodules", CONST_STRLEN("gitmodules"));
1891-
}
1892-
1893-
int git_path_is_dotgit_modules(const char *name, size_t len)
1894-
{
1895-
if (git_path_is_hfs_dotgit_modules(name, len))
1896-
return 1;
1897-
1898-
return git_path_is_ntfs_dotgit_modules(name, len);
1899-
}
1900-
1901-
int git_path_is_ntfs_dotgit_ignore(const char *name, size_t len)
1902-
{
1903-
return !verify_dotgit_ntfs_generic(name, len, "gitignore", CONST_STRLEN("gitignore"), "gi250a");
1904-
}
1875+
static const struct {
1876+
const char *file;
1877+
const char *hash;
1878+
size_t filelen;
1879+
} gitfiles[] = {
1880+
{ "gitignore", "gi250a", CONST_STRLEN("gitignore") },
1881+
{ "gitmodules", "gi7eba", CONST_STRLEN("gitmodules") },
1882+
{ "gitattributes", "gi7d29", CONST_STRLEN("gitattributes") }
1883+
};
19051884

1906-
int git_path_is_hfs_dotgit_ignore(const char *name, size_t len)
1885+
extern int git_path_is_gitfile(const char *path, size_t pathlen, git_path_gitfile gitfile, git_path_fs fs)
19071886
{
1908-
return !verify_dotgit_hfs_generic(name, len, "gitignore", CONST_STRLEN("gitignore"));
1909-
}
1910-
1911-
int git_path_is_dotgit_ignore(const char *name, size_t len)
1912-
{
1913-
if (git_path_is_hfs_dotgit_ignore(name, len))
1914-
return 1;
1915-
1916-
return git_path_is_ntfs_dotgit_ignore(name, len);
1917-
}
1887+
const char *file, *hash;
1888+
size_t filelen;
19181889

1919-
int git_path_is_hfs_dotgit_attributes(const char *name, size_t len)
1920-
{
1921-
return !verify_dotgit_hfs_generic(name, len, "gitattributes", CONST_STRLEN("gitattributes"));
1922-
}
1923-
1924-
int git_path_is_ntfs_dotgit_attributes(const char *name, size_t len)
1925-
{
1926-
return !verify_dotgit_ntfs_generic(name, len, "gitattributes", CONST_STRLEN("gitattributes"), "gi7d29");
1927-
}
1928-
1929-
int git_path_is_dotgit_attributes(const char *name, size_t len)
1930-
{
1931-
if (git_path_is_hfs_dotgit_attributes(name, len))
1932-
return 1;
1890+
if (gitfile < 0 && gitfile >= ARRAY_SIZE(gitfiles)) {
1891+
giterr_set(GITERR_OS, "invalid gitfile for path validation");
1892+
return -1;
1893+
}
19331894

1934-
return git_path_is_ntfs_dotgit_attributes(name, len);
1895+
file = gitfiles[gitfile].file;
1896+
filelen = gitfiles[gitfile].filelen;
1897+
hash = gitfiles[gitfile].hash;
1898+
1899+
switch (fs) {
1900+
case GIT_PATH_FS_GENERIC:
1901+
return !verify_dotgit_ntfs_generic(path, pathlen, file, filelen, hash) ||
1902+
!verify_dotgit_hfs_generic(path, pathlen, file, filelen);
1903+
case GIT_PATH_FS_NTFS:
1904+
return !verify_dotgit_ntfs_generic(path, pathlen, file, filelen, hash);
1905+
case GIT_PATH_FS_HFS:
1906+
return !verify_dotgit_hfs_generic(path, pathlen, file, filelen);
1907+
default:
1908+
giterr_set(GITERR_OS, "invalid filesystem for path validation");
1909+
return -1;
1910+
}
19351911
}

‎src/path.h‎

Lines changed: 37 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -645,76 +645,42 @@ extern bool git_path_isvalid(
645645
*/
646646
int git_path_normalize_slashes(git_buf *out, const char *path);
647647

648-
/**
649-
* Check whether a path component corresponds to a .gitmodules file
650-
*
651-
* @param name the path component to check
652-
* @param len the length of `name`
653-
*/
654-
extern int git_path_is_dotgit_modules(const char *name, size_t len);
655-
656-
/**
657-
* Check whether a path component corresponds to a .gitmodules file in NTFS
658-
*
659-
* @param name the path component to check
660-
* @param len the length of `name`
661-
*/
662-
extern int git_path_is_ntfs_dotgit_modules(const char *name, size_t len);
663-
664-
/**
665-
* Check whether a path component corresponds to a .gitmodules file in HFS+
666-
*
667-
* @param name the path component to check
668-
* @param len the length of `name`
669-
*/
670-
extern int git_path_is_hfs_dotgit_modules(const char *name, size_t len);
671-
672-
/**
673-
* Check whether a path component corresponds to a .gitignore file
674-
*
675-
* @param name the path component to check
676-
* @param len the length of `name`
677-
*/
678-
extern int git_path_is_dotgit_ignore(const char *name, size_t len);
679-
680-
/**
681-
* Check whether a path component corresponds to a .gitignore file in NTFS
682-
*
683-
* @param name the path component to check
684-
* @param len the length of `name`
685-
*/
686-
extern int git_path_is_ntfs_dotgit_ignore(const char *name, size_t len);
687-
688-
/**
689-
* Check whether a path component corresponds to a .gitignore file in HFS+
690-
*
691-
* @param name the path component to check
692-
* @param len the length of `name`
693-
*/
694-
extern int git_path_is_hfs_dotgit_ignore(const char *name, size_t len);
695-
696-
/**
697-
* Check whether a path component corresponds to a .gitignore file
698-
*
699-
* @param name the path component to check
700-
* @param len the length of `name`
701-
*/
702-
extern int git_path_is_dotgit_attributes(const char *name, size_t len);
703-
704-
/**
705-
* Check whether a path component corresponds to a .gitattributes file in NTFS
706-
*
707-
* @param name the path component to check
708-
* @param len the length of `name`
709-
*/
710-
extern int git_path_is_ntfs_dotgit_attributes(const char *name, size_t len);
711-
712-
/**
713-
* Check whether a path component corresponds to a .gitattributes file in HFS+
714-
*
715-
* @param name the path component to check
716-
* @param len the length of `name`
717-
*/
718-
extern int git_path_is_hfs_dotgit_attributes(const char *name, size_t len);
648+
/*
649+
* The order needs to stay the same to not break the `gitfiles`
650+
* array in path.c
651+
*/
652+
typedef enum {
653+
GIT_PATH_GITFILE_GITIGNORE,
654+
GIT_PATH_GITFILE_GITMODULES,
655+
GIT_PATH_GITFILE_GITATTRIBUTES
656+
} git_path_gitfile;
657+
658+
typedef enum {
659+
/* Do both NTFS- and HFS-specific checks */
660+
GIT_PATH_FS_GENERIC,
661+
/* Do NTFS-specific checks only */
662+
GIT_PATH_FS_NTFS,
663+
/* Do HFS-specific checks only */
664+
GIT_PATH_FS_HFS
665+
} git_path_fs;
666+
667+
/**
668+
* Check whether a path component corresponds to a .git$SUFFIX
669+
* file.
670+
*
671+
* As some filesystems do special things to filenames when
672+
* writing files to disk, you cannot always do a plain string
673+
* comparison to verify whether a file name matches an expected
674+
* path or not. This function can do the comparison for you,
675+
* depending on the filesystem you're on.
676+
*
677+
* @param path the path component to check
678+
* @param pathlen the length of `path` that is to be checked
679+
* @param gitfile which file to check against
680+
* @param fs which filesystem-specific checks to use
681+
* @return 0 in case the file does not match, a positive value if
682+
* it does; -1 in case of an error
683+
*/
684+
extern int git_path_is_gitfile(const char *path, size_t pathlen, git_path_gitfile gitfile, git_path_fs fs);
719685

720686
#endif

‎tests/path/dotgit.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,21 @@ static char *gitmodules_not_altnames[] = {
9494
void test_path_dotgit__dotgit_modules(void)
9595
{
9696
size_t i;
97-
cl_assert_equal_i(1, git_path_is_dotgit_modules(".gitmodules", strlen(".gitmodules")));
98-
cl_assert_equal_i(1, git_path_is_dotgit_modules(".git\xe2\x80\x8cmodules", strlen(".git\xe2\x80\x8cmodules")));
97+
98+
cl_assert_equal_i(1, git_path_is_gitfile(".gitmodules", strlen(".gitmodules"), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC));
99+
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));
99100

100101
for (i = 0; i < ARRAY_SIZE(gitmodules_altnames); i++) {
101102
const char *name = gitmodules_altnames[i];
102-
if (!git_path_is_dotgit_modules(name, strlen(name)))
103+
if (!git_path_is_gitfile(name, strlen(name), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC))
103104
cl_fail(name);
104105
}
105106

106107
for (i = 0; i < ARRAY_SIZE(gitmodules_not_altnames); i++) {
107108
const char *name = gitmodules_not_altnames[i];
108-
if (git_path_is_dotgit_modules(name, strlen(name)))
109+
if (git_path_is_gitfile(name, strlen(name), GIT_PATH_GITFILE_GITMODULES, GIT_PATH_FS_GENERIC))
109110
cl_fail(name);
110111
}
111-
112112
}
113113

114114
void test_path_dotgit__dotgit_modules_symlink(void)

0 commit comments

Comments
 (0)