Re: [PATCH 09/22] builtin/rebase: fix leaking `commit.gpgsign` value

> In `get_replay_opts()`, we unconditionally override the `gpg_sign` field
> that already got populated by `sequencer_init_config()` in case the user
> has "commit.gpgsign" set in their config. It is kind of dubious whether
> this is the correct thing to do or a bug. What is clear though is that
> this creates a memory leak.
>
> Let's mark this assignment with a TODO comment to figure out whether
> this needs to be fixed or not. Meanwhile though, let's plug the memory
> leak.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/rebase.c              | 8 ++++++++
>  sequencer.c                   | 1 +
>  t/t3404-rebase-interactive.sh | 1 +
>  t/t3435-rebase-gpg-sign.sh    | 1 +
>  t/t7030-verify-tag.sh         | 1 +
>  5 files changed, 12 insertions(+)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index e3a8e74cfc..f65316a023 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -186,7 +186,15 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
>  	replay.committer_date_is_author_date =
>  					opts->committer_date_is_author_date;
>  	replay.ignore_date = opts->ignore_date;
> +
> +	/*
> +	 * TODO: Is it really intentional that we unconditionally override
> +	 * `replay.gpg_sign` even if it has already been initialized via the
> +	 * configuration?
> +	 */
> +	free(replay.gpg_sign);
>  	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
> +
>  	replay.reflog_action = xstrdup(opts->reflog_action);
>  	if (opts->strategy)
>  		replay.strategy = xstrdup_or_null(opts->strategy);
> diff --git a/sequencer.c b/sequencer.c
> index 0291920f0b..cade9b0ca8 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -303,6 +303,7 @@ static int git_sequencer_config(const char *k, const char *v,
>  	}
>  
>  	if (!strcmp(k, "commit.gpgsign")) {
> +		free(opts->gpg_sign);
>  		opts->gpg_sign = git_config_bool(k, v) ? xstrdup("") : NULL;
>  		return 0;
>  	}