Subject: Re: [PATCH 3/3] path: remove redundant function calls

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 3 Mar 2026 14:42:28 +0100

On Mon, Mar 02, 2026 at 07:51:38PM +0530, K Jayatheerth wrote:
> We fetch the exact same setting up to four times.
> We fix this by evaluating it once, storing it in a local variable,

Micronit: we typically write this as if instructing the code itself to
change. So this would rather be something like "Fix this by storing it
in a local variable.".

> and referencing that variable.
> 
> Signed-off-by: K Jayatheerth <[email-masked]>
> ---
>  path.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/path.c b/path.c
> index 56be5e1726..5cd38b2a16 100644
> --- a/path.c
> +++ b/path.c
> @@ -741,18 +741,18 @@ int calc_shared_perm(struct repository *repo,
>  		     int mode)
>  {
>  	int tweak;
> -
> -	if (repo_settings_get_shared_repository(repo) < 0)
> -		tweak = -repo_settings_get_shared_repository(repo);
> +	int shared_repo = repo_settings_get_shared_repository(repo);
> +	if (shared_repo < 0)
> +		tweak = -shared_repo;
>  	else
> -		tweak = repo_settings_get_shared_repository(repo);
> +		tweak = shared_repo;
>  
>  	if (!(mode & S_IWUSR))
>  		tweak &= ~0222;
>  	if (mode & S_IXUSR)
>  		/* Copy read bits to execute bits */
>  		tweak |= (tweak & 0444) >> 2;
> -	if (repo_settings_get_shared_repository(repo) < 0)
> +	if (shared_repo < 0)
>  		mode = (mode & ~0777) | tweak;
>  	else
>  		mode |= tweak;

I agree with the fix itself though. Probably doesn't matter much as we
simply retrieve a value from the repo settings, but this also removes
some mental overhead in my mind.

Thanks!

Patrick