From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 29 May 2024 20:23:04 GMT
Subject: Re: [PATCH 13/19] revision: always store allocated strings in output encoding
Message-ID: <xmqqzfs8v047.fsf@gitster.g>
In-Reply-To: <68a7d24e4a715eaf60414373636860215d27e643.1716983704.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The `git_log_output_encoding` variable can be set via the `--encoding=`
> option. When doing so, we conditionally either assign it to the passed
> value, or if the value is "none" we assign it the empty string.
> Depending on which of the both code paths we pick though, the variable
> may end up being assigned either an allocated string or a string
> constant.
>
> This is somewhat risky and may easily lead to bugs when a different code
> path may want to reassign a new value to it, freeing the previous value.
> We already to this when parsing the "i18n.logoutputencoding" config in
> `git_default_i18n_config()`. But because the config is typically parsed
> before we parse command line options this has been fine so far.
>
> Regardless of that, safeguard the code such that the variable always
> contains an allocated string. While at it, also free the old value in
> case there was any to plug a potential memory leak.

Nice.  Now the thing always has to be freed once we are done.
Consistency is good.