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

[PATCH v4 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!

Change since V3:

 - In repo_protect_hfs() and repo_protect_ntfs(), change repo->gitdir to
 using (repo && repo->initialized).

[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/20260612160527.167203-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: 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 v4 1/1] environment: 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..f34f6fc750 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 && repo->initialized) ?
+		repo_config_values(repo)->protect_ntfs :
+		PROTECT_NTFS_DEFAULT;
+}
+
+int repo_protect_hfs(struct repository *repo)
+{
+	return (repo && repo->initialized) ?
+		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..b1ae4a70de 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->initialized` to prevent calling `repo_config_values()`
+ * before the repository setup is fully complete or in non-git 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 v4 0/1] environment: move protect_hfs and protect_ntfs into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> This series continues the ongoing libification effort by moving the
> global filesystem variables, 'protect_hfs' and 'protect_ntfs', into
> 'struct repo_config_values'.
> ...
> Change since V3:
>
>  - In repo_protect_hfs() and repo_protect_ntfs(), change repo->gitdir to
>  using (repo && repo->initialized).

While I think that it is a good change for consistency with other
two topics, the hfs/ntfs topic is already in 'next', so it needs to
be handled differently.  Namely, a topic in 'next' should not be
replaced, but be improved by additional patches on top.

In this particular case case, I think it would be good to have "to
match how we refrain from calling repo_config_values() on an
uninitialized instance of a repository object in other two topics
that deal with X bit and Y bit, check the repo->initialized bit
instead of the repo->gitdir member" or something like that in the
log message to explain why we are making the change, perhaps.

The patch text may look like this.

 environment.c | 4 ++--
 environment.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git c/environment.c w/environment.c
index 683fe1b4d3..f34f6fc750 100644
--- c/environment.c
+++ w/environment.c
@@ -142,14 +142,14 @@ int is_bare_repository(void)
 
 int repo_protect_ntfs(struct repository *repo)
 {
-	return repo->gitdir ?
+	return (repo && repo->initialized) ?
 		repo_config_values(repo)->protect_ntfs :
 		PROTECT_NTFS_DEFAULT;
 }
 
 int repo_protect_hfs(struct repository *repo)
 {
-	return repo->gitdir ?
+	return (repo && repo->initialized) ?
 		repo_config_values(repo)->protect_hfs :
 		PROTECT_HFS_DEFAULT;
 }
diff --git c/environment.h w/environment.h
index fdd9775900..b1ae4a70de 100644
--- c/environment.h
+++ w/environment.h
@@ -127,8 +127,8 @@ int git_default_core_config(const char *var, const char *value,
 
 /*
  * 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.
+ * They check `repo->initialized` to prevent calling `repo_config_values()`
+ * before the repository setup is fully complete or in non-git environments.
  */
 int repo_protect_hfs(struct repository *repo);
 int repo_protect_ntfs(struct repository *repo);

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> diff --git c/environment.h w/environment.h
> index fdd9775900..b1ae4a70de 100644
> --- c/environment.h
> +++ w/environment.h
> @@ -127,8 +127,8 @@ int git_default_core_config(const char *var, const char *value,
>  
>  /*
>   * 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.
> + * They check `repo->initialized` to prevent calling `repo_config_values()`
> + * before the repository setup is fully complete or in non-git environments.
>   */
>  int repo_protect_hfs(struct repository *repo);
>  int repo_protect_ntfs(struct repository *repo);

Another thing we should remember (but should *NOT* do while these
topics are still in flight) to do is to consolidate these comments
into one.  The hfs and htfs getters are covered by the same single
comment, but ignorecase and trustexecutable bit getters have their
own comments, only because they came in different topics.  We should
conslidate them into a single comment block once all of these have
landed in 'master', which may happen soon after 2.55 final gets
tagged.

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

[Tian Yuchen]

On 6/20/26 01:25, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> diff --git c/environment.h w/environment.h
>> index fdd9775900..b1ae4a70de 100644
>> --- c/environment.h
>> +++ w/environment.h
>> @@ -127,8 +127,8 @@ int git_default_core_config(const char *var, const char *value,
>>   
>>   /*
>>    * 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.
>> + * They check `repo->initialized` to prevent calling `repo_config_values()`
>> + * before the repository setup is fully complete or in non-git environments.
>>    */
>>   int repo_protect_hfs(struct repository *repo);
>>   int repo_protect_ntfs(struct repository *repo);
> 
> Another thing we should remember (but should *NOT* do while these
> topics are still in flight) to do is to consolidate these comments
> into one.  The hfs and htfs getters are covered by the same single
> comment, but ignorecase and trustexecutable bit getters have their
> own comments, only because they came in different topics.  We should
> conslidate them into a single comment block once all of these have
> landed in 'master', which may happen soon after 2.55 final gets
> tagged.
> 

I understand. I’ll send the hfs/ntfs fix patch right away, and will 
clean up the comments once all of the associated patches (there will be 
more in the next few weeks) have been merged into master. ;)

Thanks, yuchen

[PATCH] environment: use 'repo->initialized' for repo_protect_hfs() and repo_protect_ntfs()

[Tian Yuchen]

To match how we refrain from calling repo_config_values() on an
uninitialized instance of a repository object in other two topics
that deal with ignore_case and trust_executable_bit, check the
repo->initialized bit instead of the repo->gitdir member.

Base commit: 43192e7977f5f05138abcdb3212a3f87ab513bef

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>
---
 environment.c | 4 ++--
 environment.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/environment.c b/environment.c
index 6ee11e9fc8..8f0c1c4f25 100644
--- a/environment.c
+++ b/environment.c
@@ -130,14 +130,14 @@ int is_bare_repository(struct repository *repo)
 
 int repo_protect_ntfs(struct repository *repo)
 {
-	return repo->gitdir ?
+	return (repo && repo->initialized) ?
 		repo_config_values(repo)->protect_ntfs :
 		PROTECT_NTFS_DEFAULT;
 }
 
 int repo_protect_hfs(struct repository *repo)
 {
-	return repo->gitdir ?
+	return (repo && repo->initialized) ?
 		repo_config_values(repo)->protect_hfs :
 		PROTECT_HFS_DEFAULT;
 }
diff --git a/environment.h b/environment.h
index d188955f5b..8aaedcfea3 100644
--- a/environment.h
+++ b/environment.h
@@ -137,8 +137,8 @@ int git_default_core_config(const char *var, const char *value,
 
 /*
  * 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.
+ * They check `repo->initialized` to prevent calling `repo_config_values()`
+ * before the repository setup is fully complete or in non-git environments.
  */
 int repo_protect_hfs(struct repository *repo);
 int repo_protect_ntfs(struct repository *repo);
-- 
2.43.0

Re: [PATCH] environment: use 'repo->initialized' for repo_protect_hfs() and repo_protect_ntfs()

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> To match how we refrain from calling repo_config_values() on an
> uninitialized instance of a repository object in other two topics
> that deal with ignore_case and trust_executable_bit, check the
> repo->initialized bit instead of the repo->gitdir member.

OK.

> Base commit: 43192e7977f5f05138abcdb3212a3f87ab513bef

This line does not belong here.  Besides, you do not build directly
on top of 'next', ever.

> 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>
> ---
>  environment.c | 4 ++--
>  environment.h | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

I'll queue the change directly on top of ty/move-protect-hfs-ntfs
topic, which will be merged to 'next'.

Thanks.

> diff --git a/environment.c b/environment.c
> index 6ee11e9fc8..8f0c1c4f25 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -130,14 +130,14 @@ int is_bare_repository(struct repository *repo)
>  
>  int repo_protect_ntfs(struct repository *repo)
>  {
> -	return repo->gitdir ?
> +	return (repo && repo->initialized) ?
>  		repo_config_values(repo)->protect_ntfs :
>  		PROTECT_NTFS_DEFAULT;
>  }
>  
>  int repo_protect_hfs(struct repository *repo)
>  {
> -	return repo->gitdir ?
> +	return (repo && repo->initialized) ?
>  		repo_config_values(repo)->protect_hfs :
>  		PROTECT_HFS_DEFAULT;
>  }
> diff --git a/environment.h b/environment.h
> index d188955f5b..8aaedcfea3 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -137,8 +137,8 @@ int git_default_core_config(const char *var, const char *value,
>  
>  /*
>   * 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.
> + * They check `repo->initialized` to prevent calling `repo_config_values()`
> + * before the repository setup is fully complete or in non-git environments.
>   */
>  int repo_protect_hfs(struct repository *repo);
>  int repo_protect_ntfs(struct repository *repo);