a couple of small cleanup patches for DSM-related code

Started by Nathan Bossart7 months ago4 messages
#1Nathan Bossart
nathandbossart@gmail.com
2 attachment(s)

0001 changes test_dsm_registry.c to use PG_GETARG_INT32 and
PG_RETURN_INT32. The installation script and the C code both used signed
integers, so I'm not sure why I used PG_GETARG/RETURN_UINT32 in commit
8b2bcf3. I'm planning to back-patch this one to v17, where the DSM
registry was first introduced.

0002 follows commit 5fe08c0's example and changes some calls to
dshash_attach() and dsa_create_in_place() to use NULL instead of 0 for
pointer arguments. I don't see any need to back-patch this one, but I also
don't see any need to wait for v19devel to commit it.

Any objections?

--
nathan

Attachments:

v1-0001-fix-sign-mismatches-in-test_dsm_registry.patchtext/plain; charset=us-asciiDownload
From 1fa264cbc3ee7775adaf4a682e2edd229b5b16db Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 4 Jun 2025 14:24:38 -0500
Subject: [PATCH v1 1/2] fix sign mismatches in test_dsm_registry

---
 src/test/modules/test_dsm_registry/test_dsm_registry.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c
index 462a80f8790..96a890be228 100644
--- a/src/test/modules/test_dsm_registry/test_dsm_registry.c
+++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c
@@ -54,7 +54,7 @@ set_val_in_shmem(PG_FUNCTION_ARGS)
 	tdr_attach_shmem();
 
 	LWLockAcquire(&tdr_state->lck, LW_EXCLUSIVE);
-	tdr_state->val = PG_GETARG_UINT32(0);
+	tdr_state->val = PG_GETARG_INT32(0);
 	LWLockRelease(&tdr_state->lck);
 
 	PG_RETURN_VOID();
@@ -72,5 +72,5 @@ get_val_in_shmem(PG_FUNCTION_ARGS)
 	ret = tdr_state->val;
 	LWLockRelease(&tdr_state->lck);
 
-	PG_RETURN_UINT32(ret);
+	PG_RETURN_INT32(ret);
 }
-- 
2.39.5 (Apple Git-154)

v1-0002-use-NULL-instead-of-0-for-pointer-arguments.patchtext/plain; charset=us-asciiDownload
From 6da53d0ec077ff793694f5930648635f7e3aa954 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 4 Jun 2025 14:30:19 -0500
Subject: [PATCH v1 2/2] use NULL instead of 0 for pointer arguments

---
 src/backend/replication/logical/launcher.c | 2 +-
 src/backend/utils/activity/pgstat_shmem.c  | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 10677da56b2..1c3c051403d 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -1016,7 +1016,7 @@ logicalrep_launcher_attach_dshmem(void)
 		last_start_times_dsa = dsa_attach(LogicalRepCtx->last_start_dsa);
 		dsa_pin_mapping(last_start_times_dsa);
 		last_start_times = dshash_attach(last_start_times_dsa, &dsh_params,
-										 LogicalRepCtx->last_start_dsh, 0);
+										 LogicalRepCtx->last_start_dsh, NULL);
 	}
 
 	MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 2e33293b000..53e7d534270 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -183,7 +183,7 @@ StatsShmemInit(void)
 		p += MAXALIGN(pgstat_dsa_init_size());
 		dsa = dsa_create_in_place(ctl->raw_dsa_area,
 								  pgstat_dsa_init_size(),
-								  LWTRANCHE_PGSTATS_DSA, 0);
+								  LWTRANCHE_PGSTATS_DSA, NULL);
 		dsa_pin(dsa);
 
 		/*
@@ -255,7 +255,8 @@ pgstat_attach_shmem(void)
 	dsa_pin_mapping(pgStatLocal.dsa);
 
 	pgStatLocal.shared_hash = dshash_attach(pgStatLocal.dsa, &dsh_params,
-											pgStatLocal.shmem->hash_handle, 0);
+											pgStatLocal.shmem->hash_handle,
+											NULL);
 
 	MemoryContextSwitchTo(oldcontext);
 }
-- 
2.39.5 (Apple Git-154)

#2Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Nathan Bossart (#1)
Re: a couple of small cleanup patches for DSM-related code

On Wed, Jun 4, 2025 at 12:48 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

0001 changes test_dsm_registry.c to use PG_GETARG_INT32 and
PG_RETURN_INT32. The installation script and the C code both used signed
integers, so I'm not sure why I used PG_GETARG/RETURN_UINT32 in commit
8b2bcf3. I'm planning to back-patch this one to v17, where the DSM
registry was first introduced.

+1

0002 follows commit 5fe08c0's example and changes some calls to
dshash_attach() and dsa_create_in_place() to use NULL instead of 0 for
pointer arguments. I don't see any need to back-patch this one, but I also
don't see any need to wait for v19devel to commit it.

It seems okay to me to commit it to HEAD as it's a cosmetic change and
improves the consistency between v18 and 19.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

#3Michael Paquier
michael@paquier.xyz
In reply to: Masahiko Sawada (#2)
Re: a couple of small cleanup patches for DSM-related code

On Wed, Jun 04, 2025 at 01:53:06PM -0700, Masahiko Sawada wrote:

It seems okay to me to commit it to HEAD as it's a cosmetic change and
improves the consistency between v18 and 19.

Right. It looks confusing to leave these at 0 rather than NULL as
they mean a pointer, for the same reasons as what you have documented
in 5fe08c006c82. Doing that now or waiting for v19 does not make much
difference. Anyway, there's always the less-noise-when-backpatching
argument, so if you apply that now on HEAD that's fine IMO.
--
Michael

#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Michael Paquier (#3)
Re: a couple of small cleanup patches for DSM-related code

Committed.

--
nathan