Re: [PATCH 3/3] builtin/mailinfo: don't rely on "the_repository"

From: shejialuo <shejialuo@gmail.com>
Date: Oct 15, 2024, 16:14 UTC
Message-ID: <Zw6U3NLlxCFoszMq@ArchLinux>
In-Reply-To: <20241015144935.4059-4-five231003@gmail.com>

On Tue, Oct 15, 2024 at 08:01:24PM +0530, Kousik Sanagavarapu wrote:

Show 37 quoted lines

> Change builtin/mailinfo.c so that it doesn't have to rely on
> "the_repository" anymore - hence also allowing us the remove the
> USE_THE_REPOSITORY_VARIABLE guard.
> 
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---
>  builtin/mailinfo.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 828b2b5845..9463a8780a 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -2,7 +2,6 @@
>   * Another stupid program, this one parsing the headers of an
>   * email to figure out authorship and subject
>   */
> -#define USE_THE_REPOSITORY_VARIABLE
>  #include "builtin.h"
>  #include "abspath.h"
>  #include "environment.h"
> @@ -52,7 +51,7 @@ static int parse_opt_quoted_cr(const struct option *opt, const char *arg, int un
>  int cmd_mailinfo(int argc,
>  		 const char **argv,
>  		 const char *prefix,
> -		 struct repository *repo UNUSED)
> +		 struct repository *repo)
>  {
>  	struct metainfo_charset meta_charset;
>  	struct mailinfo mi;
> @@ -93,8 +92,7 @@ int cmd_mailinfo(int argc,
>  
>  	switch (meta_charset.policy) {
>  	case CHARSET_DEFAULT:
> -		mi.metainfo_charset =
> -			repo_get_commit_output_encoding(the_repository);
> +		mi.metainfo_charset = repo_get_commit_output_encoding(repo);

This is wrong. We cannot simply pass the "repo" here. As [1] shows, we will pass "NULL" for "repo" when running "git-mailinfo(1)" outside of the repo. There is no harm when using "repo_get_commit_output_encoding". Because this function will check whether "repo" is NULL and it will call "repo_get_commit_output_encoding" and then return the default value "UTF-8".

But what if the user has set up the "i18n.logoutputencoding" in the config file? We will ignore. There are no tests in the current codebase for running "git-mailinfo(1)" outside of the repo. But we should care about this.

[1] <d59b85b529865793c652d983d71a9fbb7e16b3e3.1728594828.git.gitgitgadget@gmail.com>

Show 7 quoted lines

>  		break;
>  	case CHARSET_NO_REENCODE:
>  		mi.metainfo_charset = NULL;
> -- 
> 2.47.0.73.g7a80afd5fd.dirty
> 
>