Subject: Re: [PATCH 05/12] fsmonitor: refactor refresh callback for non-directory events

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 15 Feb 2024 10:32:19 +0100

On Tue, Feb 13, 2024 at 08:52:14PM +0000, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <[email-masked]>
> 
> Signed-off-by: Jeff Hostetler <[email-masked]>
> ---
>  fsmonitor.c | 66 ++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/fsmonitor.c b/fsmonitor.c
> index b1ef01bf3cd..614270fa5e8 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -183,6 +183,42 @@ static int query_fsmonitor_hook(struct repository *r,
>  	return result;
>  }
>  
> +static void fsmonitor_refresh_callback_unqualified(
> +	struct index_state *istate, const char *name, int len, int pos)
> +{
> +	int i;

Same remarks here regarding the integer types. But again, not a fault of
your patch.

Patrick

> +
> +	if (pos >= 0) {
> +		/*
> +		 * We have an exact match for this path and can just
> +		 * invalidate it.
> +		 */
> +		istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
> +	} else {
> +		/*
> +		 * The path is not a tracked file -or- it is a
> +		 * directory event on a platform that cannot
> +		 * distinguish between file and directory events in
> +		 * the event handler, such as Windows.
> +		 *
> +		 * Scan as if it is a directory and invalidate the
> +		 * cone under it.  (But remember to ignore items
> +		 * between "name" and "name/", such as "name-" and
> +		 * "name.".
> +		 */
> +		pos = -pos - 1;
> +
> +		for (i = pos; i < istate->cache_nr; i++) {
> +			if (!starts_with(istate->cache[i]->name, name))
> +				break;
> +			if ((unsigned char)istate->cache[i]->name[len] > '/')
> +				break;
> +			if (istate->cache[i]->name[len] == '/')
> +				istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> +		}
> +	}
> +}
> +
>  static void fsmonitor_refresh_callback_slash(
>  	struct index_state *istate, const char *name, int len, int pos)
>  {
> @@ -214,7 +250,7 @@ static void fsmonitor_refresh_callback_slash(
>  
>  static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
>  {
> -	int i, len = strlen(name);
> +	int len = strlen(name);
>  	int pos = index_name_pos(istate, name, len);
>  
>  	trace_printf_key(&trace_fsmonitor,
> @@ -229,34 +265,8 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
>  		 * for the untracked cache.
>  		 */
>  		name[len - 1] = '\0';
> -	} else if (pos >= 0) {
> -		/*
> -		 * We have an exact match for this path and can just
> -		 * invalidate it.
> -		 */
> -		istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
>  	} else {
> -		/*
> -		 * The path is not a tracked file -or- it is a
> -		 * directory event on a platform that cannot
> -		 * distinguish between file and directory events in
> -		 * the event handler, such as Windows.
> -		 *
> -		 * Scan as if it is a directory and invalidate the
> -		 * cone under it.  (But remember to ignore items
> -		 * between "name" and "name/", such as "name-" and
> -		 * "name.".
> -		 */
> -		pos = -pos - 1;
> -
> -		for (i = pos; i < istate->cache_nr; i++) {
> -			if (!starts_with(istate->cache[i]->name, name))
> -				break;
> -			if ((unsigned char)istate->cache[i]->name[len] > '/')
> -				break;
> -			if (istate->cache[i]->name[len] == '/')
> -				istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> -		}
> +		fsmonitor_refresh_callback_unqualified(istate, name, len, pos);
>  	}
>  
>  	/*
> -- 
> gitgitgadget
> 
>