Clean up test_cloexec.c and Makefile.

macdice · Thomas Munro · macdice · commit 0451859131e5 · 2025-12-21T17:35:46.000+13:00
An unused variable caused a compiler warning on BF animal fairywren, an snprintf() call was redundant, and some buffer sizes were inconsistent. Per code review from Tom Lane. The Makefile's test ifeq ($(PORTNAME), win32) never succeeded due to a circularity, so only Meson builds were actually compiling the new test code, partially explaining why CI didn't tell us about the warning sooner (the other problem being that CompilerWarnings only makes world-bin, a problem for another commit). Simplify. Backpatch-through: 16, like commit c507ba5 Author: Bryan Green <dbryan.green@gmail.com> Co-authored-by: Thomas Munro <tmunro@gmail.com> Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1086088.1765593851%40sss.pgh.pa.us
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
@@ -1,23 +1,14 @@
 # src/test/modules/test_cloexec/Makefile
 
-# This module is for Windows only
-ifeq ($(PORTNAME),win32)
-
-MODULE_big = test_cloexec
-OBJS = \
-	$(WIN32RES) \
-	test_cloexec.o
-
 PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+PGAPPICON = win32
 
-# Build as a standalone program, not a shared library
-PROGRAM = test_cloexec
-override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+PROGRAM += test_cloexec
+OBJS += $(WIN32RES) test_cloexec.o
 
+NO_INSTALLCHECK = 1
 TAP_TESTS = 1
 
-endif
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
@@ -24,21 +24,22 @@
 #include "common/file_utils.h"
 #include "port.h"
 
+#ifdef WIN32
 static void run_parent_tests(const char *testfile1, const char *testfile2);
 static void run_child_tests(const char *handle1_str, const char *handle2_str);
 static bool try_write_to_handle(HANDLE h, const char *label);
+#endif
 
 int
 main(int argc, char *argv[])
 {
-	char		testfile1[MAXPGPATH];
-	char		testfile2[MAXPGPATH];
-
 	/* Windows-only test */
 #ifndef WIN32
 	fprintf(stderr, "This test only runs on Windows\n");
 	return 0;
-#endif
+#else
+	char		testfile1[MAXPGPATH];
+	char		testfile2[MAXPGPATH];
 
 	if (argc == 3)
 	{
@@ -68,36 +69,34 @@ main(int argc, char *argv[])
 		fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
 		return 1;
 	}
+#endif
 }
 
+#ifdef WIN32
 static void
 run_parent_tests(const char *testfile1, const char *testfile2)
 {
-#ifdef WIN32
 	int			fd1,
 				fd2;
 	HANDLE		h1,
 				h2;
-	char		cmdline[1024];
-	STARTUPINFO si;
-	PROCESS_INFORMATION pi;
+	char		exe_path[MAXPGPATH];
+	char		cmdline[MAXPGPATH + 100];
+	STARTUPINFO si = {.cb = sizeof(si)};
+	PROCESS_INFORMATION pi = {0};
 	DWORD		exit_code;
 
 	printf("Parent: Opening test files...\n");
 
-	/*
-	 * Open first file WITH O_CLOEXEC - should NOT be inherited
-	 */
+	/* Open first file WITH O_CLOEXEC - should NOT be inherited */
 	fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
 	if (fd1 < 0)
 	{
 		fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
 		exit(1);
 	}
 
-	/*
-	 * Open second file WITHOUT O_CLOEXEC - should be inherited
-	 */
+	/* Open second file WITHOUT O_CLOEXEC - should be inherited */
 	fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
 	if (fd2 < 0)
 	{
@@ -121,29 +120,12 @@ run_parent_tests(const char *testfile1, const char *testfile2)
 	printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
 	printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
 
-	/*
-	 * Spawn child process with bInheritHandles=TRUE, passing handle values as
-	 * hex strings
-	 */
-	snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
-			 GetCommandLine(), h1, h2);
-
 	/*
 	 * Find the actual executable path by removing any arguments from
-	 * GetCommandLine().
+	 * GetCommandLine(), and add our new arguments.
 	 */
-	{
-		char		exe_path[MAX_PATH];
-		char	   *space_pos;
-
-		GetModuleFileName(NULL, exe_path, sizeof(exe_path));
-		snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
-				 exe_path, h1, h2);
-	}
-
-	memset(&si, 0, sizeof(si));
-	si.cb = sizeof(si);
-	memset(&pi, 0, sizeof(pi));
+	GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+	snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p", exe_path, h1, h2);
 
 	printf("Parent: Spawning child process...\n");
 	printf("Parent: Command line: %s\n", cmdline);
@@ -180,19 +162,19 @@ run_parent_tests(const char *testfile1, const char *testfile2)
 	printf("Parent: Child exit code: %lu\n", exit_code);
 
 	if (exit_code == 0)
+	{
 		printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+	}
 	else
 	{
 		printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
 		exit(1);
 	}
-#endif
 }
 
 static void
 run_child_tests(const char *handle1_str, const char *handle2_str)
 {
-#ifdef WIN32
 	HANDLE		h1,
 				h2;
 	bool		h1_worked,
@@ -232,13 +214,11 @@ run_child_tests(const char *handle1_str, const char *handle2_str)
 		printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
 		exit(1);
 	}
-#endif
 }
 
 static bool
 try_write_to_handle(HANDLE h, const char *label)
 {
-#ifdef WIN32
 	const char *test_data = "test\n";
 	DWORD		bytes_written;
 	BOOL		result;
@@ -256,7 +236,5 @@ try_write_to_handle(HANDLE h, const char *label)
 			   label, GetLastError());
 		return false;
 	}
-#else
-	return false;
-#endif
 }
+#endif
