Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-13 Thread Alvaro Herrera
On 2022-Sep-09, Alvaro Herrera wrote: > Keeping 's' and removing the pstrdups better uses memory, because we > have a single palloc'ed chunk per option rather than two. Pushed. This is pretty much cosmetic, so no backpatch. -- Álvaro Herrera 48°01'N 7°57'E —

Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-09 Thread Alvaro Herrera
On 2022-Sep-01, Tom Lane wrote: > Junwang Zhao writes: > > result = lappend(result, makeDefElem(pstrdup(s), val, -1)); > > + pfree(s); > > I wonder why it's pstrdup'ing s in the first place. Yeah, I think both the pstrdups in that function are useless. The DefElems can just point to the

Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-01 Thread Junwang Zhao
got it, thanks. Tom Lane 于2022年9月2日 周五01:13写道: > Junwang Zhao writes: > > I'm a little confused when we should call *pfree* and when we should not. > > A few lines before there is a call *text_to_cstring* in which it invokes > > *pfree* to free the unpacked text [0]. I'm just thinking that

Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-01 Thread Tom Lane
Junwang Zhao writes: > I'm a little confused when we should call *pfree* and when we should not. > A few lines before there is a call *text_to_cstring* in which it invokes > *pfree* to free the unpacked text [0]. I'm just thinking that since *s* has > been duplicated, we should free it, that's

Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-01 Thread Junwang Zhao
On Thu, Sep 1, 2022 at 10:10 PM Tom Lane wrote: > > Junwang Zhao writes: > > result = lappend(result, makeDefElem(pstrdup(s), val, -1)); > > + pfree(s); > > I wonder why it's pstrdup'ing s in the first place. > Maybe it's pstrdup'ing s so that the caller should take care of the free? I'm a

Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-01 Thread Tom Lane
Junwang Zhao writes: > result = lappend(result, makeDefElem(pstrdup(s), val, -1)); > + pfree(s); I wonder why it's pstrdup'ing s in the first place. regards, tom lane

Re: [PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-01 Thread Daniel Gustafsson
> On 1 Sep 2022, at 10:36, Junwang Zhao wrote: > *TextDatumGetCString* calls palloc to alloc memory for the option > text datum, in some cases the the memory is allocated in > *TopTransactionContext*, this may cause memory leak for a long > running backend. Wouldn't that be a fairly

[PATCH v1] fix potential memory leak in untransformRelOptions

2022-09-01 Thread Junwang Zhao
*TextDatumGetCString* calls palloc to alloc memory for the option text datum, in some cases the the memory is allocated in *TopTransactionContext*, this may cause memory leak for a long running backend. --- src/backend/access/common/reloptions.c | 1 + 1 file changed, 1 insertion(+) diff --git