Avoid use scoped block variable

Started by Ranier Vilelaabout 1 month ago4 messages
#1Ranier Vilela
ranier.vf@gmail.com
3 attachment(s)

Hi.

I noticed a possible violation of C rules.
Some functions rely on local block variables,
but this are a mistake.
Once that block exits, the memory of the variable is released.

Fix by moving the declaration variables.

best regards,
Ranier Vilela

Attachments:

avoid-use-scoped-block-variable-lockfuncs.patchapplication/octet-stream; name=avoid-use-scoped-block-variable-lockfuncs.patchDownload
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index df938812dd..4b78cc8d8d 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -78,11 +78,8 @@ VXIDGetDatum(ProcNumber procNumber, LocalTransactionId lxid)
 	 * decimal respectively.  Note that elog.c also knows how to format a
 	 * vxid.
 	 */
-	char		vxidstr[32];
 
-	snprintf(vxidstr, sizeof(vxidstr), "%d/%u", procNumber, lxid);
-
-	return CStringGetTextDatum(vxidstr);
+	return CStringGetTextDatum(psprintf("%d/%u", procNumber, lxid));
 }
 
 
avoid-use-scoped-block-variable-rawpage.patchapplication/octet-stream; name=avoid-use-scoped-block-variable-rawpage.patchDownload
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index aef442b5db..26c23c28f5 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -260,6 +260,7 @@ page_header(PG_FUNCTION_ARGS)
 	Page		page;
 	PageHeader	pageheader;
 	XLogRecPtr	lsn;
+	char		lsnchar[64];
 
 	if (!superuser())
 		ereport(ERROR,
@@ -280,8 +281,6 @@ page_header(PG_FUNCTION_ARGS)
 	/* pageinspect >= 1.2 uses pg_lsn instead of text for the LSN field. */
 	if (TupleDescAttr(tupdesc, 0)->atttypid == TEXTOID)
 	{
-		char		lsnchar[64];
-
 		snprintf(lsnchar, sizeof(lsnchar), "%X/%08X", LSN_FORMAT_ARGS(lsn));
 		values[0] = CStringGetTextDatum(lsnchar);
 	}
avoid-use-scoped-block-variable-walsender.patchapplication/octet-stream; name=avoid-use-scoped-block-variable-walsender.patchDownload
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 0564c99550..74a5f9622f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -488,6 +488,7 @@ ReadReplicationSlot(ReadReplicationSlotCmd *cmd)
 	TupleDesc	tupdesc;
 	Datum		values[READ_REPLICATION_SLOT_COLS] = {0};
 	bool		nulls[READ_REPLICATION_SLOT_COLS];
+	char		xloc[64];
 
 	tupdesc = CreateTemplateTupleDesc(READ_REPLICATION_SLOT_COLS);
 	TupleDescInitBuiltinEntry(tupdesc, (AttrNumber) 1, "slot_type",
@@ -531,8 +532,6 @@ ReadReplicationSlot(ReadReplicationSlotCmd *cmd)
 		/* start LSN */
 		if (XLogRecPtrIsValid(slot_contents.data.restart_lsn))
 		{
-			char		xloc[64];
-
 			snprintf(xloc, sizeof(xloc), "%X/%08X",
 					 LSN_FORMAT_ARGS(slot_contents.data.restart_lsn));
 			values[i] = CStringGetTextDatum(xloc);
#2Andres Freund
andres@anarazel.de
In reply to: Ranier Vilela (#1)
Re: Avoid use scoped block variable

Hi,

On 2025-12-09 13:06:35 -0300, Ranier Vilela wrote:

I noticed a possible violation of C rules.
Some functions rely on local block variables,
but this are a mistake.
Once that block exits, the memory of the variable is released.

CStringGetTextDatum() copies its input to a fresh allocation. So there's no
longer-lived references to the local memory, unless I miss something?

Greetings,

Andres Freund

#3Tomas Vondra
tomas@vondra.me
In reply to: Ranier Vilela (#1)
Re: Avoid use scoped block variable

On 12/9/25 17:06, Ranier Vilela wrote:

Hi.

I noticed a possible violation of C rules.
Some functions rely on local block variables, 
but this are a mistake.
Once that block exits, the memory of the variable is released.

Fix by moving the declaration variables.

When you say "possible violation", did you check the issue is real?

All these places call CStringGetTextDatum, which calls cstring_to_text,
which allocates a new varlena copy of the string. So why is this an
issue, exactly?

regards

--
Tomas Vondra

#4Ranier Vilela
ranier.vf@gmail.com
In reply to: Andres Freund (#2)
Re: Avoid use scoped block variable

Em ter., 9 de dez. de 2025 às 13:19, Andres Freund <andres@anarazel.de>
escreveu:

Hi,

On 2025-12-09 13:06:35 -0300, Ranier Vilela wrote:

I noticed a possible violation of C rules.
Some functions rely on local block variables,
but this are a mistake.
Once that block exits, the memory of the variable is released.

CStringGetTextDatum() copies its input to a fresh allocation. So there's no
longer-lived references to the local memory, unless I miss something?

Yeah. My bad.
cstring_to_text use palloc.

Sorry for the noise.

best regards,
Ranier Vilela