Re: [PATCH v7 2/4] read-cache: pass 'repo' to 'ce_mode_from_stat()'

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Jul 17, 2026 at 02:35:57PM +0800, Tian Yuchen wrote:
> The ce_mode_from_stat() function is a performance-critical static
> inline helper in 'read-cache.h'. As we migrate configuration
> variables into the repository struct, this helper needs access
> to the repository context.
> 
> Update the signature of ce_mode_from_stat() to take a 'struct
> repository *' parameter, and update all callers to pass the
> appropriate repository instance.
> 
> To prepare for the overhead of replacing cheap global variable
> accesses with getter functions, the boolean expressions are
> reordered to evaluate 'S_ISREG(mode)' first.
> 
> While at it, add a comment for ce_mode_from_stat().
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---

> diff --git a/read-cache.h b/read-cache.h
> index 043da1f1aa..94b8d3e547 100644
> --- a/read-cache.h
> +++ b/read-cache.h
> @@ -4,15 +4,24 @@
>  #include "read-cache-ll.h"
>  #include "object.h"
>  #include "pathspec.h"
> +#include "environment.h"
>  
> -static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
> +/*
> + * Determine the appropriate index mode for a file based on its stat()
> + * information and the existing cache entry (if any).
> + *
> + * This function handles degradation for filesystems that lack
> + * symlink support or reliable executable bits.
> + */
> +static inline unsigned int ce_mode_from_stat(struct repository *repo,

This new parameter is not yet used in this function, which causes
compilation errors in all source files which include "read-cache.h"
when trying to build this commit using DEVELOPER=1, e.g.:

      CC pathspec.o
  In file included from pathspec.c:11:
  read-cache.h: In function ‘ce_mode_from_stat’:
  read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
     16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
        |                                              ~~~~~~~~~~~~~~~~~~~^~~~
  cc1: all warnings being treated as errors
  make: *** [Makefile:2921: pathspec.o] Error 1
      CC preload-index.o
  In file included from preload-index.c:16:
  read-cache.h: In function ‘ce_mode_from_stat’:
  read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
     16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
        |                                              ~~~~~~~~~~~~~~~~~~~^~~~
  cc1: all warnings being treated as errors
  make: *** [Makefile:2921: preload-index.o] Error 1
      CC read-cache.o
  In file included from read-cache.c:34:
  read-cache.h: In function ‘ce_mode_from_stat’:
  read-cache.h:16:65: error: unused parameter ‘repo’ [-Werror=unused-parameter]
     16 | static inline unsigned int ce_mode_from_stat(struct repository *repo,
        |                                              ~~~~~~~~~~~~~~~~~~~^~~~
  cc1: all warnings being treated as errors
  make: *** [Makefile:2921: read-cache.o] Error 1

I think the new parameter should be marked as UNUSED in this patch,
and then the UNUSED should be dropped in the next, where you start
using the parameter.

> +					     const struct cache_entry *ce,
>  					     unsigned int mode)
>  {
>  	extern int trust_executable_bit, has_symlinks;
> -	if (!has_symlinks && S_ISREG(mode) &&
> +	if (S_ISREG(mode) && !has_symlinks &&
>  	    ce && S_ISLNK(ce->ce_mode))
>  		return ce->ce_mode;
> -	if (!trust_executable_bit && S_ISREG(mode)) {
> +	if (S_ISREG(mode) && !trust_executable_bit) {
>  		if (ce && S_ISREG(ce->ce_mode))
>  			return ce->ce_mode;
>  		return create_ce_mode(0666);
> -- 
> 2.43.0
>