Re: [PATCH v7 3/4] environment: move trust_executable_bit into repo_config_values

[Junio C Hamano <gitster@pobox.com>]

Tian Yuchen <cat@malon.dev> writes:

> diff --git a/environment.c b/environment.c
> index fc3ed8bb1c..75069a884d 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -41,7 +41,6 @@
>  static int pack_compression_seen;
>  static int zlib_compression_seen;
>  
> -int trust_executable_bit = 1;
>  int trust_ctime = 1;
>  int check_stat = 1;
>  int has_symlinks = 1;
> @@ -142,6 +141,13 @@ int is_bare_repository(void)
>  	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>  }
>  
> +int repo_trust_executable_bit(struct repository *repo)
> +{
> +	return repo->gitdir?
> +		repo_config_values(repo)->trust_executable_bit :
> +		1;
> +}
> +
>  int have_git_dir(void)
>  {
>  	return startup_info->have_repository

Two comments.

 * Missing SP before '?'.  It may be easier to read if it is written
   like this:

	return repo->gitdir
	       ? repo_config_values(repo)->trust_executable_bit
	       : 1;

   which more clearly highlights the ternary structure.  If you tilt
   your head 90 degrees to the left, you can almost see the parse
   tree of the expression.
  
 * Does it make sense to protect against a NULL 'repo' case, as
   repo_protect_ntfs() and repo_protect_hfs() helpers do?  Or is it
   better to crash loudly with a segfault to let the developer know
   they have a bug to fix?  I lean toward the latter myself, and if
   we go that route, we should probably stop using 'repo &&
   repo->gitdir' elsewhere, rather than sweeping the problem under
   the rug with defensive checks.