From: shejialuo Date: Tue, 15 Oct 2024 16:14:20 GMT Subject: Re: [PATCH 3/3] builtin/mailinfo: don't rely on "the_repository" Message-ID: In-Reply-To: <20241015144935.4059-4-five231003@gmail.com> On Tue, Oct 15, 2024 at 08:01:24PM +0530, Kousik Sanagavarapu wrote: > 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 > --- > 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] > break; > case CHARSET_NO_REENCODE: > mi.metainfo_charset = NULL; > -- > 2.47.0.73.g7a80afd5fd.dirty > >