[PATCH v1 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'

[PATCH v1 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'

[Tian Yuchen]

Move the global 'protect_hfs' and 'protect_ntfs' configurations
into the repository-specific 'repo_config_values' struct.
This will help with the elimination of 'the_repository'

For now, associated functions access this configuration by
explicitly falling back to 'the_repository', which needs to
be addressed in the future.

Note: In 't/helper/test-path-utils.c', there is a function
'protect_ntfs_hfs_benchmark()' where these two global
variables are used as loop iterators. New local variables
have been created to replace them.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 compat/mingw.c             |  2 +-
 environment.c              |  8 ++++----
 environment.h              |  4 ++--
 read-cache.c               |  7 ++++---
 t/helper/test-path-utils.c | 26 ++++++++++++++++----------
 5 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..c77696ba8a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3392,7 +3392,7 @@ int is_valid_win32_path(const char *path, int allow_literal_nul)
 	const char *p = path;
 	int preceding_space_or_period = 0, i = 0, periods = 0;
 
-	if (!protect_ntfs)
+	if (!(the_repository->gitdir ? repo_config_values(the_repository)->protect_ntfs : 1))
 		return 1;
 
 	skip_dos_drive_prefix((char **)&path);
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..0730bfcbba 100644
--- a/environment.c
+++ b/environment.c
@@ -82,12 +82,10 @@ unsigned long pack_size_limit_cfg;
 #ifndef PROTECT_HFS_DEFAULT
 #define PROTECT_HFS_DEFAULT 0
 #endif
-int protect_hfs = PROTECT_HFS_DEFAULT;
 
 #ifndef PROTECT_NTFS_DEFAULT
 #define PROTECT_NTFS_DEFAULT 1
 #endif
-int protect_ntfs = PROTECT_NTFS_DEFAULT;
 
 /*
  * The character that begins a commented line in user-editable file
@@ -541,12 +539,12 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.protecthfs")) {
-		protect_hfs = git_config_bool(var, value);
+		cfg->protect_hfs = git_config_bool(var, value);
 		return 0;
 	}
 
 	if (!strcmp(var, "core.protectntfs")) {
-		protect_ntfs = git_config_bool(var, value);
+		cfg->protect_ntfs = git_config_bool(var, value);
 		return 0;
 	}
 
@@ -720,5 +718,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
+	cfg->protect_hfs = PROTECT_HFS_DEFAULT;
+	cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 }
diff --git a/environment.h b/environment.h
index 9eb97b3869..d48fd2719c 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,8 @@ struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
 	int apply_sparse_checkout;
+	int protect_hfs;
+	int protect_ntfs;
 
 	/* section "branch" config values */
 	enum branch_track branch_track;
@@ -173,8 +175,6 @@ extern int pack_compression_level;
 extern unsigned long pack_size_limit_cfg;
 
 extern int precomposed_unicode;
-extern int protect_hfs;
-extern int protect_ntfs;
 
 extern int core_sparse_checkout_cone;
 extern int sparse_expect_files_outside_of_patterns;
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..b64a5629ef 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1002,7 +1002,7 @@ static enum verify_path_result verify_path_internal(const char *path,
 			return PATH_OK;
 		if (is_dir_sep(c)) {
 inside:
-			if (protect_hfs) {
+			if ((the_repository->gitdir ? repo_config_values(the_repository)->protect_hfs : 0)) {
 
 				if (is_hfs_dotgit(path))
 					return PATH_INVALID;
@@ -1011,7 +1011,7 @@ static enum verify_path_result verify_path_internal(const char *path,
 						return PATH_INVALID;
 				}
 			}
-			if (protect_ntfs) {
+			if ((the_repository->gitdir ? repo_config_values(the_repository)->protect_ntfs : 1)) {
 #if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__
 				if (c == '\\')
 					return PATH_INVALID;
@@ -1035,7 +1035,8 @@ static enum verify_path_result verify_path_internal(const char *path,
 			if (c == '\0')
 				return S_ISDIR(mode) ? PATH_DIR_WITH_SEP :
 						       PATH_INVALID;
-		} else if (c == '\\' && protect_ntfs) {
+		} else if (c == '\\' &&
+			   (the_repository->gitdir ? repo_config_values(the_repository)->protect_ntfs : 1)) {
 			if (is_ntfs_dotgit(path))
 				return PATH_INVALID;
 			if (S_ISLNK(mode)) {
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 15eb44485c..4455a68903 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -250,6 +250,7 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 	double m[3][2], v[3][2];
 	uint64_t cumul;
 	double cumul2;
+	int ntfs, hfs;
 
 	if (argc > 1 && !strcmp(argv[1], "--with-symlink-mode")) {
 		file_mode = 0120000;
@@ -275,9 +276,14 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 		while (len > 0)
 			names[i][--len] = (char)(' ' + (my_random() % ('\x7f' - ' ')));
 	}
-
-	for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
-		for (protect_hfs = 0; protect_hfs < 2; protect_hfs++) {
+	
+	if (!the_repository->gitdir)
+		the_repository->gitdir = xstrdup(".git");
+
+	for (ntfs = 0; ntfs < 2; ntfs++)
+		for (hfs = 0; hfs < 2; hfs++) {
+			repo_config_values(the_repository)->protect_ntfs = ntfs;
+			repo_config_values(the_repository)->protect_hfs = hfs;
 			cumul = 0;
 			cumul2 = 0;
 			for (i = 0; i < repetitions; i++) {
@@ -285,18 +291,18 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 				for (j = 0; j < nr; j++)
 					verify_path(names[j], file_mode);
 				end = getnanotime();
-				printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
+				printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", ntfs, hfs, (end-begin) / (double)1e6);
 				cumul += end - begin;
 				cumul2 += (end - begin) * (end - begin);
 			}
-			m[protect_ntfs][protect_hfs] = cumul / (double)repetitions;
-			v[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);
-			printf("mean: %lfms, stddev: %lfms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
+			m[ntfs][hfs] = cumul / (double)repetitions;
+			v[ntfs][hfs] = my_sqrt(cumul2 / (double)repetitions - m[ntfs][hfs] * m[ntfs][hfs]);
+			printf("mean: %lfms, stddev: %lfms\n", m[ntfs][hfs] / (double)1e6, v[ntfs][hfs] / (double)1e6);
 		}
 
-	for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
-		for (protect_hfs = 0; protect_hfs < 2; protect_hfs++)
-			printf("ntfs=%d/hfs=%d: %lf%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
+	for (ntfs = 0; ntfs < 2; ntfs++)
+		for (hfs = 0; hfs < 2; hfs++)
+			printf("ntfs=%d/hfs=%d: %lf%% slower\n", ntfs, hfs, (m[ntfs][hfs] - m[0][0]) * 100 / m[0][0]);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v1 0/1] environment: move protect_hfs and protect_ntfs

[Tian Yuchen]

Hi everyone,

This series continues the ongoing libification effort by moving the
global **filesystem** variables, protect_hfs and protect_ntfs, into
struct repo_config_values.

Place them within the **per-repository** configuration structure
aligns with our goal of removing global states.

RFC Questions:

1. Should we keep PROTECT_HFS_DEFAULT and PROTECT_NTFS_DEFAULT
in repo_config_values_init()?

	void repo_config_values_init(struct repo_config_values *cfg)
		{
			cfg->attributes_file = NULL;
			cfg->apply_sparse_checkout = 0;
			cfg->protect_hfs = PROTECT_HFS_DEFAULT;
			cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
			cfg->branch_track = BRANCH_TRACK_REMOTE;
		}

Or is it better if they are used anywhere other than in environment.c?

If so... 
2. Is it worth introducing a Macro or Getter for safe access?

	((the_repository->gitdir ? repo_config_values(the_repository)->protect_hfs : 0))

The current approach looks verbose and lacks readability, and
hard-coded 0 and 1 are used as fallback values. I wonder if a macro or a
getter could be introduced, for example... 

	#define SAFE_PROTECT_HFS(repo) \
		(((repo) && (repo)->gitdir && (repo) == the_repository) ? \ 
		repo_config_values(repo)->protect_hfs : PROTECT_HFS_DEFAULT)

...to improve the coding style a bit. Although I am aware that introducing
new macros is generally frowned upon, I would still like to know which
parts this might make difficult to maintain.

3. Note that Derrick attempted to use get_int_config_global to wrap
this kind of Filesystem Level global variables. This approach bypassed
struct repository, did not actually eliminate global state, and the
reviewer politely rejected it. Nevertheless, I am still curious as
to whether this approach might still be inspiring today.

https://lore.kernel.org/git/a42dd9397d07b2dc4a0d7e75bfe1af2e46cad262.1685716420.git.gitgitgadget@gmail.com/

Thanks!

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

Tian Yuchen (1):
  environment.c: move 'protect_hfs' and 'protect_ntfs' into
    'repo_config_values'

 compat/mingw.c             |  2 +-
 environment.c              |  8 ++++----
 environment.h              |  4 ++--
 read-cache.c               |  7 ++++---
 t/helper/test-path-utils.c | 26 ++++++++++++++++----------
 5 files changed, 27 insertions(+), 20 deletions(-)

-- 
2.43.0

Re: [PATCH v1 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'

[Christian Couder]

"environment.c:" in the subject could be replaced by just
"environment:". It's a bit shorter and it better describes the area of
the code where the main changes are made, as changes are not just made
in "environment.c" but also in "environment.h".

On Sat, Jun 6, 2026 at 4:34 PM Tian Yuchen <cat@malon.dev> wrote:
>
> Move the global 'protect_hfs' and 'protect_ntfs' configurations
> into the repository-specific 'repo_config_values' struct.
> This will help with the elimination of 'the_repository'
>
> For now, associated functions access this configuration by
> explicitly falling back to 'the_repository', which needs to
> be addressed in the future.
>
> Note: In 't/helper/test-path-utils.c', there is a function
> 'protect_ntfs_hfs_benchmark()' where these two global
> variables are used as loop iterators. New local variables
> have been created to replace them.


> diff --git a/compat/mingw.c b/compat/mingw.c
> index aa7525f419..c77696ba8a 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -3392,7 +3392,7 @@ int is_valid_win32_path(const char *path, int allow_literal_nul)
>         const char *p = path;
>         int preceding_space_or_period = 0, i = 0, periods = 0;
>
> -       if (!protect_ntfs)
> +       if (!(the_repository->gitdir ? repo_config_values(the_repository)->protect_ntfs : 1))
>                 return 1;

I think the code would benefit from functions like:

int repo_protect_ntfs(struct repository *repo)
{
    return repo->gitdir ?
        repo_config_values(repo)->protect_ntfs :
        PROTECT_NTFS_DEFAULT;
}

int repo_protect_hfs(struct repository *repo)
{
    return repo->gitdir ?
        repo_config_values(repo)->protect_hfs :
        PROTECT_HFS_DEFAULT;
}

They could be called by passing `the_repository` for now, but perhaps
later in future commits `istate->repo` or something like that could be
passed instead. Also a code comment could explain that the `gitdir`
check prevents calling `repo_config_values()` before config is loaded.

Thanks.

Re: [PATCH v1 0/1] environment: move protect_hfs and protect_ntfs

[Christian Couder]

On Sat, Jun 6, 2026 at 4:35 PM Tian Yuchen <cat@malon.dev> wrote:
>
> Hi everyone,
>
> This series continues the ongoing libification effort by moving the
> global **filesystem** variables, protect_hfs and protect_ntfs, into
> struct repo_config_values.
>
> Place them within the **per-repository** configuration structure
> aligns with our goal of removing global states.
>
> RFC Questions:
>
> 1. Should we keep PROTECT_HFS_DEFAULT and PROTECT_NTFS_DEFAULT
> in repo_config_values_init()?
>
>         void repo_config_values_init(struct repo_config_values *cfg)
>                 {
>                         cfg->attributes_file = NULL;
>                         cfg->apply_sparse_checkout = 0;
>                         cfg->protect_hfs = PROTECT_HFS_DEFAULT;
>                         cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
>                         cfg->branch_track = BRANCH_TRACK_REMOTE;
>                 }
>
> Or is it better if they are used anywhere other than in environment.c?

I think it's better to keep them in "environment.c". The
repo_protect_ntfs() and repo_protect_hfs() function I suggest adding
to "environment.c" in my reply to the patch should help with keeping
the macros in "environment.c".

> If so...
> 2. Is it worth introducing a Macro or Getter for safe access?
>
>         ((the_repository->gitdir ? repo_config_values(the_repository)->protect_hfs : 0))

Yes, it seems to me that a getter is enough.

> The current approach looks verbose and lacks readability, and
> hard-coded 0 and 1 are used as fallback values. I wonder if a macro or a
> getter could be introduced, for example...
>
>         #define SAFE_PROTECT_HFS(repo) \
>                 (((repo) && (repo)->gitdir && (repo) == the_repository) ? \
>                 repo_config_values(repo)->protect_hfs : PROTECT_HFS_DEFAULT)
>
> ...to improve the coding style a bit. Although I am aware that introducing
> new macros is generally frowned upon, I would still like to know which
> parts this might make difficult to maintain.

Unless there are features that we really want which a function can't
provide, a function is better as it provides type safety and is
usually easier to maintain.

> 3. Note that Derrick attempted to use get_int_config_global to wrap
> this kind of Filesystem Level global variables. This approach bypassed
> struct repository, did not actually eliminate global state, and the
> reviewer politely rejected it. Nevertheless, I am still curious as
> to whether this approach might still be inspiring today.
>
> https://lore.kernel.org/git/a42dd9397d07b2dc4a0d7e75bfe1af2e46cad262.1685716420.git.gitgitgadget@gmail.com/

To help your reviewers, it might be interesting if you could already
tell why this was rejected by Glen Choo (who reviewed Derrick Stolee's
work then). It seems to me that Glen said that using plain fields in a
struct should be better as long as the fields are always initialized
during the setup process.

And it seems to me that our patch follows the direction that Glen suggested.

Thanks.

[PATCH v2 0/1] environment: move protect_hfs and protect_ntfs into repo_config_values

[Tian Yuchen]

Hi everyone,

This series continues the ongoing libification effort by moving the
global filesystem variables, 'protect_hfs' and 'protect_ntfs', into
'struct repo_config_values'.

Place them within the per-repository configuration structure
aligns with our goal of removing global states.

For reviewers familiar with previous libification efforts, Derrick Stolee
attempted to wrap this kind of filesystem-level variable using a
lazy-loaded global accessor get_int_config_global() [1].

However, as Glen Choo pointed out in his review of that series [2],
it is strongly preferred to use plain fields in a repository-scoped
struct over global lazy-loaders, provided those fields are properly
initialized during the setup process.

By moving these variables into repo_config_values and parsing
them eagerly, we successfully tie the filesystem security flags
to the specific repository instance without altering the timing
of configuration warnings or introducing new global states.

Thanks!

Recent related patch (environment.c: migrate 'trust_executable_bit' into 'repo_config_values'): [3]

[1] https://lore.kernel.org/git/a42dd9397d07b2dc4a0d7e75bfe1af2e46cad262.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/kl6lbkhpzujf.fsf@chooglen-macbookpro.roam.corp.google.com/
[3] https://lore.kernel.org/git/20260610093635.139719-1-cat@malon.dev/

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

Tian Yuchen (1):
  environment.c: move 'protect_hfs' and 'protect_ntfs' into
    'repo_config_values'

 compat/mingw.c             |  2 +-
 environment.c              | 22 ++++++++++++++++++----
 environment.h              | 12 ++++++++++--
 read-cache.c               |  7 ++++---
 t/helper/test-path-utils.c | 24 +++++++++++++++---------
 5 files changed, 48 insertions(+), 19 deletions(-)

-- 
2.43.0

[PATCH v2 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'

[Tian Yuchen]

Move the global 'protect_hfs' and 'protect_ntfs' configurations
into the repository-specific 'repo_config_values' struct.
This will help with the elimination of 'the_repository'

To ensure code readability, the getter functions
'repo_protect_hfs()' and 'repo_protect_ntfs()'
have been introduced.

For now, associated functions access this configuration by
explicitly falling back to 'the_repository', which needs to
be addressed in the future.

Note: In 't/helper/test-path-utils.c', there is a function
'protect_ntfs_hfs_benchmark()' where these two global
variables are used as loop iterators. New local variables
have been created to replace them.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 compat/mingw.c             |  2 +-
 environment.c              | 22 ++++++++++++++++++----
 environment.h              | 12 ++++++++++--
 read-cache.c               |  7 ++++---
 t/helper/test-path-utils.c | 24 +++++++++++++++---------
 5 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..af87df77fd 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3392,7 +3392,7 @@ int is_valid_win32_path(const char *path, int allow_literal_nul)
 	const char *p = path;
 	int preceding_space_or_period = 0, i = 0, periods = 0;
 
-	if (!protect_ntfs)
+	if (!repo_protect_ntfs(the_repository))
 		return 1;
 
 	skip_dos_drive_prefix((char **)&path);
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..683fe1b4d3 100644
--- a/environment.c
+++ b/environment.c
@@ -82,12 +82,10 @@ unsigned long pack_size_limit_cfg;
 #ifndef PROTECT_HFS_DEFAULT
 #define PROTECT_HFS_DEFAULT 0
 #endif
-int protect_hfs = PROTECT_HFS_DEFAULT;
 
 #ifndef PROTECT_NTFS_DEFAULT
 #define PROTECT_NTFS_DEFAULT 1
 #endif
-int protect_ntfs = PROTECT_NTFS_DEFAULT;
 
 /*
  * The character that begins a commented line in user-editable file
@@ -142,6 +140,20 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+int repo_protect_ntfs(struct repository *repo)
+{
+	return repo->gitdir ?
+		repo_config_values(repo)->protect_ntfs :
+		PROTECT_NTFS_DEFAULT;
+}
+
+int repo_protect_hfs(struct repository *repo)
+{
+	return repo->gitdir ?
+		repo_config_values(repo)->protect_hfs :
+		PROTECT_HFS_DEFAULT;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
@@ -541,12 +553,12 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.protecthfs")) {
-		protect_hfs = git_config_bool(var, value);
+		cfg->protect_hfs = git_config_bool(var, value);
 		return 0;
 	}
 
 	if (!strcmp(var, "core.protectntfs")) {
-		protect_ntfs = git_config_bool(var, value);
+		cfg->protect_ntfs = git_config_bool(var, value);
 		return 0;
 	}
 
@@ -720,5 +732,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
+	cfg->protect_hfs = PROTECT_HFS_DEFAULT;
+	cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 }
diff --git a/environment.h b/environment.h
index 9eb97b3869..fdd9775900 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,8 @@ struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
 	int apply_sparse_checkout;
+	int protect_hfs;
+	int protect_ntfs;
 
 	/* section "branch" config values */
 	enum branch_track branch_track;
@@ -123,6 +125,14 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+/*
+ * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
+ * They check `repo->gitdir` to prevent calling repo_config_values()
+ * before the configuration is loaded or in bare environments.
+ */
+int repo_protect_hfs(struct repository *repo);
+int repo_protect_ntfs(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
 /*
@@ -173,8 +183,6 @@ extern int pack_compression_level;
 extern unsigned long pack_size_limit_cfg;
 
 extern int precomposed_unicode;
-extern int protect_hfs;
-extern int protect_ntfs;
 
 extern int core_sparse_checkout_cone;
 extern int sparse_expect_files_outside_of_patterns;
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..2c6a60c756 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1002,7 +1002,7 @@ static enum verify_path_result verify_path_internal(const char *path,
 			return PATH_OK;
 		if (is_dir_sep(c)) {
 inside:
-			if (protect_hfs) {
+			if (repo_protect_hfs(the_repository)) {
 
 				if (is_hfs_dotgit(path))
 					return PATH_INVALID;
@@ -1011,7 +1011,7 @@ static enum verify_path_result verify_path_internal(const char *path,
 						return PATH_INVALID;
 				}
 			}
-			if (protect_ntfs) {
+			if (repo_protect_ntfs(the_repository)) {
 #if defined GIT_WINDOWS_NATIVE || defined __CYGWIN__
 				if (c == '\\')
 					return PATH_INVALID;
@@ -1035,7 +1035,8 @@ static enum verify_path_result verify_path_internal(const char *path,
 			if (c == '\0')
 				return S_ISDIR(mode) ? PATH_DIR_WITH_SEP :
 						       PATH_INVALID;
-		} else if (c == '\\' && protect_ntfs) {
+		} else if (c == '\\' &&
+			   repo_protect_ntfs(the_repository)) {
 			if (is_ntfs_dotgit(path))
 				return PATH_INVALID;
 			if (S_ISLNK(mode)) {
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 15eb44485c..f77b3f9d70 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -250,6 +250,7 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 	double m[3][2], v[3][2];
 	uint64_t cumul;
 	double cumul2;
+	int ntfs, hfs;
 
 	if (argc > 1 && !strcmp(argv[1], "--with-symlink-mode")) {
 		file_mode = 0120000;
@@ -276,8 +277,13 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 			names[i][--len] = (char)(' ' + (my_random() % ('\x7f' - ' ')));
 	}
 
-	for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
-		for (protect_hfs = 0; protect_hfs < 2; protect_hfs++) {
+	if (!the_repository->gitdir)
+		the_repository->gitdir = xstrdup(".git");
+
+	for (ntfs = 0; ntfs < 2; ntfs++)
+		for (hfs = 0; hfs < 2; hfs++) {
+			repo_config_values(the_repository)->protect_ntfs = ntfs;
+			repo_config_values(the_repository)->protect_hfs = hfs;
 			cumul = 0;
 			cumul2 = 0;
 			for (i = 0; i < repetitions; i++) {
@@ -285,18 +291,18 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 				for (j = 0; j < nr; j++)
 					verify_path(names[j], file_mode);
 				end = getnanotime();
-				printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
+				printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", ntfs, hfs, (end-begin) / (double)1e6);
 				cumul += end - begin;
 				cumul2 += (end - begin) * (end - begin);
 			}
-			m[protect_ntfs][protect_hfs] = cumul / (double)repetitions;
-			v[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);
-			printf("mean: %lfms, stddev: %lfms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
+			m[ntfs][hfs] = cumul / (double)repetitions;
+			v[ntfs][hfs] = my_sqrt(cumul2 / (double)repetitions - m[ntfs][hfs] * m[ntfs][hfs]);
+			printf("mean: %lfms, stddev: %lfms\n", m[ntfs][hfs] / (double)1e6, v[ntfs][hfs] / (double)1e6);
 		}
 
-	for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
-		for (protect_hfs = 0; protect_hfs < 2; protect_hfs++)
-			printf("ntfs=%d/hfs=%d: %lf%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
+	for (ntfs = 0; ntfs < 2; ntfs++)
+		for (hfs = 0; hfs < 2; hfs++)
+			printf("ntfs=%d/hfs=%d: %lf%% slower\n", ntfs, hfs, (m[ntfs][hfs] - m[0][0]) * 100 / m[0][0]);
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v2 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> +int repo_protect_ntfs(struct repository *repo)
> +{
> +	return repo->gitdir ?
> +		repo_config_values(repo)->protect_ntfs :
> +		PROTECT_NTFS_DEFAULT;
> +}
> +
> +int repo_protect_hfs(struct repository *repo)
> +{
> +	return repo->gitdir ?
> +		repo_config_values(repo)->protect_hfs :
> +		PROTECT_HFS_DEFAULT;
> +}
> ...
> @@ -123,6 +125,14 @@ int git_default_config(const char *, const char *,
>  int git_default_core_config(const char *var, const char *value,
>  			    const struct config_context *ctx, void *cb);
>  
> +/*
> + * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
> + * They check `repo->gitdir` to prevent calling repo_config_values()
> + * before the configuration is loaded or in bare environments.
> + */
> +int repo_protect_hfs(struct repository *repo);
> +int repo_protect_ntfs(struct repository *repo);

I briefly wondered what *should* happen when repo->gitdir is not
ready, as it feels almost a bug for a caller to call these two
functions before the repository is ready to be used.

When repo is not ready, these return their respective default
values.  That's like the original code using the initial value of
these global variables.

IOW, this rewrite is bug-for-bug compatible, which is good.

Shall we declare victory and mark the topic for 'next' now?

Thanks.