> -----Original Message----- > From: Ben Peart [mailto:benpeart@xxxxxxxxxxxxx] > Sent: Friday, September 15, 2017 3:21 PM > To: benpeart@xxxxxxxxxxxxx > Cc: David Turner <David.Turner@xxxxxxxxxxxx>; avarab@xxxxxxxxx; > christian.couder@xxxxxxxxx; git@xxxxxxxxxxxxxxx; gitster@xxxxxxxxx; > johannes.schindelin@xxxxxx; pclouds@xxxxxxxxx; peff@xxxxxxxx > Subject: [PATCH v6 04/12] fsmonitor: teach git to optionally utilize a file system > monitor to speed up detecting new or changed files. > +int git_config_get_fsmonitor(void) > +{ > + if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor)) > + core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); > + > + if (core_fsmonitor && !*core_fsmonitor) > + core_fsmonitor = NULL; > + > + if (core_fsmonitor) > + return 1; > + > + return 0; > +} This functions return values are backwards relative to the rest of the git_config_* functions. [snip] +> /* +> * With fsmonitor, we can trust the untracked cache's valid field. +> */ [snip] > +int read_fsmonitor_extension(struct index_state *istate, const void *data, > + unsigned long sz) > +{ If git_config_get_fsmonitor returns 0, fsmonitor_dirty will leak. [snip] > + /* a fsmonitor process can return '*' to indicate all entries are invalid */ That's not documented in your documentation. Also, I'm not sure I like it: what if I have a file whose name starts with '*'? Yeah, that would be silly, but this indicates the need for the protocol to have some sort of signaling mechanism that's out-of-band Maybe have some key\0value\0 pairs and then \0\0 and then the list of files? Or, if you want to keep it really simple, allow an entry of '/' (which is an invalid filename) to mean 'all'. > +void add_fsmonitor(struct index_state *istate) { > + int i; > + > + if (!istate->fsmonitor_last_update) { [snip] > + /* reset the untracked cache */ Is this really necessary? Shouldn't the untracked cache be in a correct state already? > +/* > + * Clear the given cache entries CE_FSMONITOR_VALID bit and invalidate Nit: "s/entries/entry's/".
