All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Derrick Stolee" <dstolee@microsoft.com>,
	"Jeff Hostetler" <git@jeffhostetler.com>,
	"Patrick Steinhardt" <ps@pks.im>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v3 5/5] repository.h: don't use a mix of int and bitfields
Date: Sun, 19 Sep 2021 10:47:19 +0200	[thread overview]
Message-ID: <patch-v3-5.5-3cc033b8864-20210919T084703Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v3-0.5-00000000000-20210919T084703Z-avarab@gmail.com>

Change the bitfield added in 58300f47432 (sparse-index: add
index.sparse config option, 2021-03-30) and 3964fc2aae7 (sparse-index:
add guard to ensure full index, 2021-03-30) to just use an "int"
boolean instead.

It might be smart to optimize the space here in the future, but by
consistently using an "int" we can take its address and pass it to
repo_cfg_bool(), and therefore don't need to handle "sparse_index" as
a special-case when reading the "index.sparse" setting.

There's no corresponding config for "command_requires_full_index", but
let's change it too for consistency and to prevent future bugs
creeping in due to one of these being "unsigned".

Using "int" consistently also prevents subtle bugs or undesired
control flow creeping in here. Before the preceding commit the
initialization of "command_requires_full_index" in
prepare_repo_settings() did nothing, i.e. this:

    r->settings.command_requires_full_index = 1

Was redundant to the earlier memset() to -1. Likewise for
"sparse_index" added in 58300f47432 (sparse-index: add index.sparse
config option, 2021-03-30) the code and comment added there was
misleading, we weren't initializing it to off, but re-initializing it
from "1" to "0", and then finally checking the config, and perhaps
setting it to "1" again. I.e. we could have applied this patch before
the preceding commit:

	+	assert(r->settings.command_requires_full_index == 1);
	 	r->settings.command_requires_full_index = 1;

	 	/*
	 	 * Initialize this as off.
	 	 */
	+	assert(r->settings.sparse_index == 1);
	 	r->settings.sparse_index = 0;
	 	if (!repo_config_get_bool(r, "index.sparse", &value) && value)
	 		r->settings.sparse_index = 1;

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 repo-settings.c | 8 +-------
 repository.h    | 5 ++---
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/repo-settings.c b/repo-settings.c
index 46b9d56aeac..b93e91a212e 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -45,6 +45,7 @@ void prepare_repo_settings(struct repository *r)
 	repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0);
 	repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1);
 	repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
+	repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
@@ -90,11 +91,4 @@ void prepare_repo_settings(struct repository *r)
 	 * removed.
 	 */
 	r->settings.command_requires_full_index = 1;
-
-	/*
-	 * Initialize this as off.
-	 */
-	r->settings.sparse_index = 0;
-	if (!repo_config_get_bool(r, "index.sparse", &value) && value)
-		r->settings.sparse_index = 1;
 }
diff --git a/repository.h b/repository.h
index bf36744e0a7..02599ae2c98 100644
--- a/repository.h
+++ b/repository.h
@@ -31,6 +31,8 @@ struct repo_settings {
 	int commit_graph_read_changed_paths;
 	int gc_write_commit_graph;
 	int fetch_write_commit_graph;
+	int command_requires_full_index;
+	int sparse_index;
 
 	int index_version;
 	enum untracked_cache_setting core_untracked_cache;
@@ -39,9 +41,6 @@ struct repo_settings {
 	enum fetch_negotiation_setting fetch_negotiation_algorithm;
 
 	int core_multi_pack_index;
-
-	unsigned command_requires_full_index:1,
-		 sparse_index:1;
 };
 
 struct repository {
-- 
2.33.0.1092.g44c994ea1be


  parent reply	other threads:[~2021-09-19  8:47 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-16 18:30 [PATCH v2 0/5] repo-settings.c: refactor for clarity, get rid of hacks etc Ævar Arnfjörð Bjarmason
2021-09-16 18:30 ` [PATCH v2 1/5] wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c Ævar Arnfjörð Bjarmason
2021-09-17 16:57   ` Junio C Hamano
2021-09-17 19:18     ` Ævar Arnfjörð Bjarmason
2021-09-16 18:30 ` [PATCH v2 2/5] environment.c: remove test-specific "ignore_untracked..." variable Ævar Arnfjörð Bjarmason
2021-09-17 17:19   ` Junio C Hamano
2021-09-16 18:30 ` [PATCH v2 3/5] read-cache & fetch-negotiator: check "enum" values in switch() Ævar Arnfjörð Bjarmason
2021-09-17 19:30   ` Junio C Hamano
2021-09-16 18:30 ` [PATCH v2 4/5] repo-settings.c: simplify the setup Ævar Arnfjörð Bjarmason
2021-09-16 18:30 ` [PATCH v2 5/5] repository.h: don't use a mix of int and bitfields Ævar Arnfjörð Bjarmason
2021-09-19  8:47 ` [PATCH v3 0/5] repo-settings.c: refactor for clarity, get rid of hacks etc Ævar Arnfjörð Bjarmason
2021-09-19  8:47   ` [PATCH v3 1/5] wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c Ævar Arnfjörð Bjarmason
2021-09-20 21:53     ` Taylor Blau
2021-09-20 23:17       ` Ævar Arnfjörð Bjarmason
2021-09-19  8:47   ` [PATCH v3 2/5] environment.c: remove test-specific "ignore_untracked..." variable Ævar Arnfjörð Bjarmason
2021-09-20 22:10     ` Taylor Blau
2021-09-20 23:27       ` Ævar Arnfjörð Bjarmason
2021-09-19  8:47   ` [PATCH v3 3/5] read-cache & fetch-negotiator: check "enum" values in switch() Ævar Arnfjörð Bjarmason
2021-09-20 22:14     ` Taylor Blau
2021-09-20 23:33       ` Ævar Arnfjörð Bjarmason
2021-09-19  8:47   ` [PATCH v3 4/5] repo-settings.c: simplify the setup Ævar Arnfjörð Bjarmason
2021-09-20 12:42     ` Derrick Stolee
2021-09-20 22:18       ` Taylor Blau
2021-09-19  8:47   ` Ævar Arnfjörð Bjarmason [this message]
2021-09-20 22:25     ` [PATCH v3 5/5] repository.h: don't use a mix of int and bitfields Taylor Blau
2021-09-21 13:12   ` [PATCH v4 0/5] repo-settings.c: refactor for clarity, get rid of hacks etc Ævar Arnfjörð Bjarmason
2021-09-21 13:12     ` [PATCH v4 1/5] wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c Ævar Arnfjörð Bjarmason
2021-09-21 13:13     ` [PATCH v4 2/5] environment.c: remove test-specific "ignore_untracked..." variable Ævar Arnfjörð Bjarmason
2021-09-21 13:13     ` [PATCH v4 3/5] read-cache & fetch-negotiator: check "enum" values in switch() Ævar Arnfjörð Bjarmason
2021-09-21 13:13     ` [PATCH v4 4/5] repo-settings.c: simplify the setup Ævar Arnfjörð Bjarmason
2021-09-21 13:13     ` [PATCH v4 5/5] repository.h: don't use a mix of int and bitfields Ævar Arnfjörð Bjarmason
2021-09-21 15:58     ` [PATCH v4 0/5] repo-settings.c: refactor for clarity, get rid of hacks etc Derrick Stolee
2021-09-21 20:46       ` Taylor Blau
2021-09-22 20:23         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v3-5.5-3cc033b8864-20210919T084703Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.