Re: [PATCH v2 3/7] trace2: remove use of xstrdup()

> On Tue, Aug 25, 2026 at 11:58 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
> [...]
>> 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.
>>
> [...]
>> --- 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_cb
> 
> and 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.