On 2022-Sep-01, Tom Lane wrote:
> Junwang Zhao <zhjw...@gmail.com> 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 correct portion of the (already pstrdup'd
by TextDatumGetCString) copy of optiondatums[i]. We modify that copy to
install \0 in the place where the = is, and that copy is not freed
anywhere.
diff --git a/src/backend/access/common/reloptions.c
b/src/backend/access/common/reloptions.c
index 609329bb21..0aa4b334ab 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1357,9 +1357,9 @@ untransformRelOptions(Datum options)
if (p)
{
*p++ = '\0';
- val = (Node *) makeString(pstrdup(p));
+ val = (Node *) makeString(p);
}
- result = lappend(result, makeDefElem(pstrdup(s), val, -1));
+ result = lappend(result, makeDefElem(s, val, -1));
}
return result;
I think these pstrdups were already not necessary when the function was
added in 265f904d8f25, because textout() was already known to return a
palloc'ed copy of its input; but later 220db7ccd8c8 made this contract
even more explicit.
Keeping 's' and removing the pstrdups better uses memory, because we
have a single palloc'ed chunk per option rather than two.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
