Thread (4 messages) 4 messages, 2 authors, 21d ago

Re: [PATCH v1 2/2] environment: move excludes_file into repo_config_values

From: Junio C Hamano <hidden>
Date: 2026-06-25 20:40:11

Tian Yuchen [off-list ref] writes:
Continue the libification effor by moving the 'excludes_file' global
"effort"?
quoted hunk ↗ jump to hunk
variable into 'struct repo_config_values'.

Since 'excludes_file' is a dynamically allocated string (char *), it
requires proper memory management. Introduce repo_config_values_clear()
to safely free the heap memory when repository instance is destroyed.

Note: 'if (repo != the_repository)' fallback logic is temporarily added
in both the getter and the clear function. This prevents calling
repo_config_values() on uninitialized submodules, which triggers BUG().

Mentored-by: Christian Couder [off-list ref]
Mentored-by: Ayush Chandekar [off-list ref]
Mentored-by: Olamide Caleb Bello [off-list ref]
Signed-off-by: Tian Yuchen <redacted>
---
 environment.c | 28 ++++++++++++++++++++++------
 environment.h | 15 +++++++++++----
 repository.c  |  1 +
 3 files changed, 34 insertions(+), 10 deletions(-)
...
@@ -733,3 +736,16 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->sparse_expect_files_outside_of_patterns = 0;
 	cfg->warn_on_object_refname_ambiguity = 1;
Shouldn't cfg->excludes_file be explicitly initialized to NULL here
for completeness?  There are other explicit but redundant 0 assignment
to the members of this struct in the same function.
quoted hunk ↗ jump to hunk
 }
+
+void repo_config_values_clear(struct repository *repo)
+{
+	struct repo_config_values *cfg;
+
+	if (repo != the_repository)
+		return;
+
+	cfg = repo_config_values(repo);
+	if (!cfg)
+		return;
+	FREE_AND_NULL(cfg->excludes_file);
+}
diff --git a/environment.h b/environment.h
index 52d531e4ea..2839913551 100644
--- a/environment.h
+++ b/environment.h
@@ -98,6 +98,7 @@ struct repo_config_values {
 	int precomposed_unicode;
 	int core_sparse_checkout_cone;
 	int warn_on_object_refname_ambiguity;
+	char *excludes_file;
 
 	/* section "sparse" config values */
 	int sparse_expect_files_outside_of_patterns;
@@ -133,13 +134,20 @@ 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);
 
-/*
- * TODO: This still relies on the global state.
- */
 const char *repo_excludes_file(struct repository *repo);
Good.
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * Note: `excludes_file` is currently the only heap-allocated field in
+ * this struct. As other dynamically allocated variables are migrated,
+ * their FREE_AND_NULL() calls should be appended here.
Isn't attributes_file also heap-allocated member in this struct as well?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help