Hi Jeff,
On Thu, 17 Feb 2022, Jeff Hostetler wrote:
> On 2/17/22 11:27 AM, Johannes Schindelin wrote:
>
> > On Fri, 11 Feb 2022, Jeff Hostetler via GitGitGadget wrote:
> >
> > > diff --git a/fsmonitor.h b/fsmonitor.h
> > > index f20d72631d7..f9201411aa7 100644
> > > --- a/fsmonitor.h
> > > +++ b/fsmonitor.h
> > > @@ -3,6 +3,7 @@
> > >
> > > #include "cache.h"
> > > #include "dir.h"
> > > +#include "fsmonitor-settings.h"
> > >
> > > extern struct trace_key trace_fsmonitor;
> > >
> > > @@ -57,7 +58,11 @@ int fsmonitor_is_trivial_response(const struct strbuf
> > > *query_result);
> > > */
> > > static inline int is_fsmonitor_refreshed(const struct index_state
> > > *istate)
> > > {
> > > - return !core_fsmonitor || istate->fsmonitor_has_run_once;
> > > + struct repository *r = istate->repo ? istate->repo : the_repository;
> >
> > I see this repeated a few times. Would it maybe make sense to change
> > the signature of the `fsm_settings__*()` functions to accept an index
> > instead of a repository?
>
> I think is just me being paranoid -- testing istate->repo for null
> and assuming the_repository if necessary.
Indeed, this might be a bit too careful here. There is a pretty strong
assumption built into the Git index that there is a repository, after all.
> I'm wondering if it is always safe to just do
>
> fsm_mode = fsm_settings__get_mode(istate->repo);
>
> (or maybe put the null check inside the fsm_settings__*() functions.
I would like to try. The test suite should give us enough confidence that
we have dashed all Ts and dotted all Is on the question whether
`istate->repo` is always non-NULL.
> changing the signature of those fsm_* functions seems wrong since they
> are associated with a repo and not an index.
True.
Thank you for the sanity check,
Dscho
