Thread (75 messages) flat view 75 messages, 8 authors, 2021-12-13

Re: [PATCH v3 04/11] unpack-trees: introduce preserve_ignored to unpack_trees_options

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-10-01 10:17:35
Subsystem: the rest · Maintainer: Linus Torvalds

On Fri, Oct 01 2021, Ævar Arnfjörð Bjarmason wrote:
Aside about safety: One thing I'll sometimes do when I'm unsure about
those sorts of fixes is to have my new INIT set a new "sentinel" field
to "12345" or whatever, then just BUG() out in an entry point in the API
that you can't avoid calling if it's not set like that, e.g. dir_clear()
or whatever the setup/work function is.
For reference: Something like the below, which passes with my WIP
patches. Showing that no non-static entry point can reach the code in
unpack-trees.c without "foo" being 12345, which can only be the case if
callers have used the macro (and the code internal to unpack-trees.c is
easy enough to audit).

 unpack-trees.c | 25 +++++++++++++++++++++++++
 unpack-trees.h |  2 ++
 2 files changed, 27 insertions(+)
diff --git a/unpack-trees.c b/unpack-trees.c
index d40af221e1c..f2365ecf215 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -199,6 +199,8 @@ void clear_unpack_trees_porcelain(struct unpack_trees_options *opts)
 {
 	strvec_clear(&opts->msgs_to_free);
 	dir_clear(&opts->dir);
+	if (opts->foo != 12345)
+		BUG("noes");
 	memset(opts->msgs, 0, sizeof(opts->msgs));
 }
 
@@ -1702,6 +1704,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	struct pattern_list pl;
 	int free_pattern_list = 0;
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	if (len > MAX_UNPACK_TREES)
 		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
 
@@ -1903,6 +1908,9 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
 	unsigned old_show_all_errors;
 	int free_pattern_list = 0;
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	old_show_all_errors = o->show_all_errors;
 	o->show_all_errors = 1;
 
@@ -2033,6 +2041,8 @@ static int verify_uptodate_1(const struct cache_entry *ce,
 int verify_uptodate(const struct cache_entry *ce,
 		    struct unpack_trees_options *o)
 {
+	if (o->foo != 12345)
+		BUG("noes");
 	if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))
 		return 0;
 	return verify_uptodate_1(ce, o, ERROR_NOT_UPTODATE_FILE);
@@ -2417,6 +2427,9 @@ int threeway_merge(const struct cache_entry * const *stages,
 	int no_anc_exists = 1;
 	int i;
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	for (i = 1; i < o->head_idx; i++) {
 		if (!stages[i] || stages[i] == o->df_conflict_entry)
 			any_anc_missing = 1;
@@ -2580,6 +2593,9 @@ int twoway_merge(const struct cache_entry * const *src,
 	const struct cache_entry *oldtree = src[1];
 	const struct cache_entry *newtree = src[2];
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	if (o->merge_size != 2)
 		return error("Cannot do a twoway merge of %d trees",
 			     o->merge_size);
@@ -2654,6 +2670,9 @@ int bind_merge(const struct cache_entry * const *src,
 	const struct cache_entry *old = src[0];
 	const struct cache_entry *a = src[1];
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	if (o->merge_size != 1)
 		return error("Cannot do a bind merge of %d trees",
 			     o->merge_size);
@@ -2680,6 +2699,9 @@ int oneway_merge(const struct cache_entry * const *src,
 	const struct cache_entry *old = src[0];
 	const struct cache_entry *a = src[1];
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	if (o->merge_size != 1)
 		return error("Cannot do a oneway merge of %d trees",
 			     o->merge_size);
@@ -2717,6 +2739,9 @@ int stash_worktree_untracked_merge(const struct cache_entry * const *src,
 	const struct cache_entry *worktree = src[1];
 	const struct cache_entry *untracked = src[2];
 
+	if (o->foo != 12345)
+		BUG("noes");
+
 	if (o->merge_size != 2)
 		BUG("invalid merge_size: %d", o->merge_size);
 
diff --git a/unpack-trees.h b/unpack-trees.h
index 75b67f90ccd..8dae0938ad1 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -90,10 +90,12 @@ struct unpack_trees_options {
 
 	struct pattern_list *pl; /* for internal use */
 	struct checkout_metadata meta;
+	int foo;
 };
 #define UNPACK_TREES_OPTIONS_INIT { \
 	.msgs_to_free = STRVEC_INIT, \
 	.dir = DIR_INIT, \
+	.foo = 12345, \
 }
 
 void unpack_trees_init(struct unpack_trees_options *options);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help