fsmonitor: add assertion that fsmonitor is valid to check_removed

Validate that fsmonitor is valid to futureproof against bugs where check_removed might be called from places that haven't refreshed. Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nipunn Koorapati committed Mar 17, 2021 at 21:22 UTC 0ec9949f78e348250f55ba0343344ec8c606be11
2 files changed +22 -7
diff-lib.c
+11 -7
@@ -28,8 +28,9 @@
28 * exists for ce that is a submodule -- it is a submodule that is not
29 * checked out). Return negative for an error.
30 */
31 -static int check_removed(const struct cache_entry *ce, struct stat *st)
31 +static int check_removed(const struct index_state *istate, const struct cache_entry *ce, struct stat *st)
32 {
33 + assert(is_fsmonitor_refreshed(istate));
34 if (!(ce->ce_flags & CE_FSMONITOR_VALID) && lstat(ce->name, st) < 0) {
35 if (!is_missing_file_error(errno))
36 return -1;
@@ -136,7 +137,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
137 memset(&(dpath->parent[0]), 0,
138 sizeof(struct combine_diff_parent)*5);
139
139 - changed = check_removed(ce, &st);
140 + changed = check_removed(istate, ce, &st);
141 if (!changed)
142 wt_mode = ce_mode_from_stat(ce, st.st_mode);
143 else {
@@ -216,7 +217,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
217 } else {
218 struct stat st;
219
219 - changed = check_removed(ce, &st);
220 + changed = check_removed(istate, ce, &st);
221 if (changed) {
222 if (changed < 0) {
223 perror(ce->name);
@@ -278,7 +279,8 @@ static void diff_index_show_file(struct rev_info *revs,
279 oid, oid_valid, ce->name, dirty_submodule);
280 }
281
281 -static int get_stat_data(const struct cache_entry *ce,
282 +static int get_stat_data(const struct index_state *istate,
283 + const struct cache_entry *ce,
284 const struct object_id **oidp,
285 unsigned int *modep,
286 int cached, int match_missing,
@@ -290,7 +292,7 @@ static int get_stat_data(const struct cache_entry *ce,
292 if (!cached && !ce_uptodate(ce)) {
293 int changed;
294 struct stat st;
293 - changed = check_removed(ce, &st);
295 + changed = check_removed(istate, ce, &st);
296 if (changed < 0)
297 return -1;
298 else if (changed) {
@@ -321,12 +323,13 @@ static void show_new_file(struct rev_info *revs,
323 const struct object_id *oid;
324 unsigned int mode;
325 unsigned dirty_submodule = 0;
326 + struct index_state *istate = revs->diffopt.repo->index;
327
328 /*
329 * New file in the index: it might actually be different in
330 * the working tree.
331 */
329 - if (get_stat_data(new_file, &oid, &mode, cached, match_missing,
332 + if (get_stat_data(istate, new_file, &oid, &mode, cached, match_missing,
333 &dirty_submodule, &revs->diffopt) < 0)
334 return;
335
@@ -342,8 +345,9 @@ static int show_modified(struct rev_info *revs,
345 unsigned int mode, oldmode;
346 const struct object_id *oid;
347 unsigned dirty_submodule = 0;
348 + struct index_state *istate = revs->diffopt.repo->index;
349
346 - if (get_stat_data(new_entry, &oid, &mode, cached, match_missing,
350 + if (get_stat_data(istate, new_entry, &oid, &mode, cached, match_missing,
351 &dirty_submodule, &revs->diffopt) < 0) {
352 if (report_missing)
353 diff_index_show_file(revs, "-", old_entry,
fsmonitor.h
+11
@@ -49,6 +49,17 @@ void refresh_fsmonitor(struct index_state *istate);
49 */
50 int fsmonitor_is_trivial_response(const struct strbuf *query_result);
51
52 +/*
53 + * Check if refresh_fsmonitor has been called at least once.
54 + * refresh_fsmonitor is idempotent. Returns true if fsmonitor is
55 + * not enabled (since the state will be "fresh" w/ CE_FSMONITOR_VALID unset)
56 + * This version is useful for assertions
57 + */
58 +static inline int is_fsmonitor_refreshed(const struct index_state *istate)
59 +{
60 + return !core_fsmonitor || istate->fsmonitor_has_run_once;
61 +}
62 +
63 /*
64 * Set the given cache entries CE_FSMONITOR_VALID bit. This should be
65 * called any time the cache entry has been updated to reflect the