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);