Re: free C string

Started by Tom Laneover 4 years ago3 messages
#1Tom Lane
tgl@sss.pgh.pa.us

Zhihong Yu <zyu@yugabyte.com> writes:

I was looking at fmgr_internal_validator().
It seems prosrc is only used internally.
The patch frees the C string prosrc points to, prior to returning.

There's really very little point in adding such code. Our memory
context mechanisms take care of minor leaks like this, with less
code and fewer cycles expended than explicit pfree calls require.
It's worth trying to clean up explicitly in code that might get
executed many times in a row, or might be allocating very big
temporary chunks; but fmgr_internal_validator hardly falls in
that category.

regards, tom lane

#2Zhihong Yu
zyu@yugabyte.com
In reply to: Tom Lane (#1)
1 attachment(s)

On Wed, Jul 14, 2021 at 10:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Zhihong Yu <zyu@yugabyte.com> writes:

I was looking at fmgr_internal_validator().
It seems prosrc is only used internally.
The patch frees the C string prosrc points to, prior to returning.

There's really very little point in adding such code. Our memory
context mechanisms take care of minor leaks like this, with less
code and fewer cycles expended than explicit pfree calls require.
It's worth trying to clean up explicitly in code that might get
executed many times in a row, or might be allocating very big
temporary chunks; but fmgr_internal_validator hardly falls in
that category.

regards, tom lane

Hi,
How about this occurrence which is in a loop ?

Thanks

Attachments:

c-str-free.patchapplication/octet-stream; name=c-str-free.patchDownload
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index dba32ceff3..e4a409e495 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1353,6 +1353,7 @@ untransformRelOptions(Datum options)
 			val = (Node *) makeString(pstrdup(p));
 		}
 		result = lappend(result, makeDefElem(pstrdup(s), val, -1));
+        pfree(s);
 	}
 
 	return result;
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zhihong Yu (#2)

Zhihong Yu <zyu@yugabyte.com> writes:

On Wed, Jul 14, 2021 at 10:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

There's really very little point in adding such code. Our memory
context mechanisms take care of minor leaks like this, with less
code and fewer cycles expended than explicit pfree calls require.
It's worth trying to clean up explicitly in code that might get
executed many times in a row, or might be allocating very big
temporary chunks; but fmgr_internal_validator hardly falls in
that category.

How about this occurrence which is in a loop ?

I'd say the burden is on you to prove that it's worth worrying
about, not vice versa. If we added pfree everywhere we possibly
could, the code would be larger and slower, not faster.

regards, tom lane