From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 03 Mar 2026 13:42:28 GMT
Subject: Re: [PATCH 3/3] path: remove redundant function calls
Message-ID: <aablRKlhQZUTct56@pks.im>
In-Reply-To: <20260302142138.712273-4-jayatheerthkulkarni2005@gmail.com>

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 <jayatheerthkulkarni2005@gmail.com>
> ---
>  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