Hi Junio, On Tue, 30 Aug 2016, Junio C Hamano wrote:
> Johannes Schindelin <[email protected]> writes: > > > @@ -811,13 +813,18 @@ static int populate_opts_cb(const char *key, const > > char *value, void *data) > > opts->allow_ff = git_config_bool_or_int(key, value, > > &error_flag); > > else if (!strcmp(key, "options.mainline")) > > opts->mainline = git_config_int(key, value); > > - else if (!strcmp(key, "options.strategy")) > > + else if (!strcmp(key, "options.strategy")) { > > git_config_string(&opts->strategy, key, value); > > - else if (!strcmp(key, "options.gpg-sign")) > > + sequencer_entrust(opts, (char *) opts->strategy); > > + } > > + else if (!strcmp(key, "options.gpg-sign")) { > > git_config_string(&opts->gpg_sign, key, value); > > + sequencer_entrust(opts, (char *) opts->gpg_sign); > > + } > > else if (!strcmp(key, "options.strategy-option")) { > > ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc); > > - opts->xopts[opts->xopts_nr++] = xstrdup(value); > > + opts->xopts[opts->xopts_nr++] = > > + sequencer_entrust(opts, xstrdup(value)); > > } else > > return error(_("Invalid key: %s"), key); > > Hmm. > > I would have expected a call to sequencer_opts_clear(&opts) once the > machinery is done with the options structure, It is part of sequencer_remove_state(). > and among these places where an old value in opts->field is overwritten > by a new one would get > > free(opt->field); opt->field = ... new value ...; That is *exactly* what we *cannot* do. You see, the replay_opts struct is *not necessarily* populated by read_populate_opts(). So we really cannot tell whether we are free to free(opt->field) or whether some other code still wants to reference that memory in the end (or for that matter, whether it was malloc()ed). That is the *precise* reason for the existence of sequencer_entrust(). Ciao, Dscho
