Re: [Outreachy PATCH v5 3/3] environment: move "branch.autoSetupMerge" into `struct repo_config_values`

[Junio C Hamano <gitster@pobox.com>]

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

> So, we need to take it as a given that repo_config_values_init()
> needs to exist.  Under that condition, I wonder if we can somehow
> have a cheap way to assert the following two things:
>
>  * Before a repository instance is used, repo_config_values_init()
>    has been called on it, as using an instance without initializing
>    is a no-no.
>
>  * repo_config_values_init() is never called twice on a repository
>    instance, as the second call will wipe what the first call and
>    subsequent reading of the configuration files have done.

Something like this squashed into your [1/3] would give us these two
assertions, but I am not sure if it is a good idea or if I am overly
paranoid.

The main ideas are:

 * "struct repository" now knows if it has been initialized via its
   "bool initialized" member.

 * "initialize_repository()" detects double initialization of the
   repository struct itself.

 * "repo_config_values" member in "struct repository" has been
   renamed to make it clear it is "private", and there is an
   accessor function of the same name.  It barfs if you ask the
   address of repo_config_values in a repository instance that
   hasn't been initialized.

 * Any code outside what implement the above are supposed to call
   repo_config_values() on the repository they are working in, to
   request the address of the repo_config_values instance to use.

It didn't barf when I ran all the tests, which means there isn't
anybody who calls initialize_repository() twice in the current code.

I am not sure if this is being overly paranoid, or exercising a
reasonable caution, but anyway,...


 attr.c        |  3 ++-
 environment.c |  2 +-
 environment.h |  3 +++
 repository.c  | 13 ++++++++++++-
 repository.h  |  5 ++++-
 5 files changed, 22 insertions(+), 4 deletions(-)

diff --git c/attr.c w/attr.c
index b8b70e6dce..cd39e6d2bf 100644
--- c/attr.c
+++ w/attr.c
@@ -881,7 +881,8 @@ const char *git_attr_system_file(void)
 
 const char *git_attr_global_file(void)
 {
-	struct repo_config_values *cfg = &the_repository->config_values;
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (!cfg->attributes_file)
 		cfg->attributes_file = xdg_config_home("attributes");
 
diff --git c/environment.c w/environment.c
index c876589b05..208a52ce11 100644
--- c/environment.c
+++ w/environment.c
@@ -302,7 +302,7 @@ static enum fsync_component parse_fsync_components(const char *var, const char *
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb)
 {
-	struct repo_config_values *cfg = &the_repository->config_values;
+	struct repo_config_values *cfg = repo_config_values(the_repository);
 
 	/* This needs a better name */
 	if (!strcmp(var, "core.filemode")) {
diff --git c/environment.h w/environment.h
index 2b861a61de..254fec6b7c 100644
--- c/environment.h
+++ w/environment.h
@@ -84,11 +84,14 @@ extern const char * const local_repo_env[];
 
 struct strvec;
 
+struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
 };
 
+struct repo_config_values *repo_config_values(struct repository *);
+
 /*
  * Wrapper of getenv() that returns a strdup value. This value is kept
  * in argv to be freed later.
diff --git c/repository.c w/repository.c
index d308cd78bf..4cb487b5b2 100644
--- c/repository.c
+++ w/repository.c
@@ -50,14 +50,25 @@ static void set_default_hash_algo(struct repository *repo)
 	repo_set_hash_algo(repo, algo);
 }
 
+struct repo_config_values *repo_config_values(struct repository *repo)
+{
+	if (!repo->initialized)
+		BUG("config values from uninitialied repository?");
+	return &repo->config_values_private_;
+}
+
 void initialize_repository(struct repository *repo)
 {
+	if (repo->initialized)
+		BUG("repository initialized already!");
+	repo->initialized = true;
+
 	repo->remote_state = remote_state_new();
 	repo->parsed_objects = parsed_object_pool_new(repo);
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
 	repo->check_deprecated_config = true;
-	repo_config_values_init(&repo->config_values);
+	repo_config_values_init(repo_config_values(repo));
 
 	/*
 	 * When a command runs inside a repository, it learns what
diff --git c/repository.h w/repository.h
index 638a142577..9717e45000 100644
--- c/repository.h
+++ w/repository.h
@@ -150,7 +150,7 @@ struct repository {
 	const struct git_hash_algo *compat_hash_algo;
 
 	/* Repository's config values parsed by git_default_config() */
-	struct repo_config_values config_values;
+	struct repo_config_values config_values_private_;
 
 	/* Repository's reference storage format, as serialized on disk. */
 	enum ref_storage_format ref_storage_format;
@@ -175,6 +175,9 @@ struct repository {
 
 	/* Should repo_config() check for deprecated settings */
 	bool check_deprecated_config;
+
+	/* Has this repository instance been initialized? */
+	bool initialized;
 };
 
 #ifdef USE_THE_REPOSITORY_VARIABLE