On Wed, Oct 12, 2022 at 11:34:32PM -0400, Tom Lane wrote:
> Julien Rouhaud <rjuju...@gmail.com> writes:
> > On Wed, Oct 12, 2022 at 11:08:25PM -0400, Tom Lane wrote:
> >> It may be worth looking at the GUC code, which has been dealing
> >> with the same sorts of issues pretty successfully for many years.
> 
> > The GUC code relies on malloc/free,
> 
> Not for much longer [1].  And no, I don't believe that that patch
> makes any noticeable difference in the code's robustness.

Ok, so the new code still assumes that guc_free can't/shouldn't fail:

static void
set_string_field(struct config_string *conf, char **field, char *newval)
{
        char       *oldval = *field;

        /* Do the assignment */
        *field = newval;

        /* Free old value if it's not NULL and isn't referenced anymore */
        if (oldval && !string_field_used(conf, oldval))
                guc_free(oldval);
}


[...]

                                                set_string_field(conf, 
&conf->reset_val, newval);
                                                set_extra_field(&conf->gen, 
&conf->reset_extra,
                                                                                
newextra);
                                                conf->gen.reset_source = source;
                                                conf->gen.reset_scontext = 
context;
                                                conf->gen.reset_srole = srole;

Any error in guc_free will leave the struct in some inconsistent state and
possibly leak some data.  We can use the same approach for session variables.


Reply via email to