Re: [PATCH v2 3/7] trace2: remove use of xstrdup()
- From
Elijah Newren <newren@gmail.com>
- Date
- Aug 25, 2026, 22:14 UTC
- Message-ID
- <CABPp-BH1TeDTeqddZw+cvzou+3PRgw+HNpYF2JnhMTSBp9qfbQ@mail.gmail.com>
- In-Reply-To
- <ec447a6a778a5c49344346df54b434a96c792082.1787684181.git.gitgitgadget@gmail.com>
On Tue, Aug 25, 2026 at 11:58 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
>
[...]
Show 6 quoted lines
> For full defense in depth, we remove the xstrdup() calls from > trace2/tr2_sysenv.c. > > First, in tr2_sysenv_cb(), we need to handle a failed assignment of the > value with a negative return to halt the config parsing loop. >
[...]
Show 10 quoted lines
> --- a/trace2/tr2_sysenv.c > +++ b/trace2/tr2_sysenv.c > @@ -74,7 +74,9 @@ static int tr2_sysenv_cb(const char *key, const char *value, > if (!value) > return config_error_nonbool(key); > free(tr2_sysenv_settings[k].value); > - tr2_sysenv_settings[k].value = xstrdup(value); > + tr2_sysenv_settings[k].value = strdup(value); > + if (!tr2_sysenv_settings[k].value) > + return -1;
I'm not sure if this matters, but I think the call sequence from config.c to this function is:
read_very_early_config ->
config_with_options ->
git_config_from_file_with_options ->
do_config_from_file ->
do_config_from ->
git_parse_source ->
get_value ->
git_config_include ->
tr2_sysenv_cband the -1 unwinds back to git_parse_source, which breaks, formats an error message, and calls die:
error_msg = xstrfmt(_("bad config line %d in file %s")...)
die("%s", error_msg)Am I reading this right? If so, the -1 actually triggers a die as well -- unless the allocation in xstrfmt manages to kill it first. This isn't a regression (the old xstrdup() also died) and the die isn't inside the trace functions, but the commit message might read as promising more than it delivers.