From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: how to correctly react on exception in pfree function? |
Date: | 2022-10-13 03:49:44 |
Message-ID: | Y0eK2FlkuOUXJeSI@jrouhaud |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 12, 2022 at 11:34:32PM -0400, Tom Lane wrote:
> Julien Rouhaud <rjuju123(at)gmail(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | kuroda.hayato@fujitsu.com | 2022-10-13 04:25:50 | Add mssing test to test_decoding/meson.build |
Previous Message | Amit Kapila | 2022-10-13 03:49:26 | Re: Perform streaming logical transactions by background workers and parallel apply |