From 47a7e7406c0896e985435360013d99f0553dd607 Mon Sep 17 00:00:00 2001 From: Bryan Green Date: Fri, 12 Dec 2025 22:31:15 -0600 Subject: [PATCH 3/3] Clean up sloppy code in test_cloexec.c. The command line construction code in run_parent_tests() had several issues: 1. A useless snprintf() call that buitlt a command line using GetCommandLine(), which was immediately overwritten by a second snprintf() that correctly used GetModuleFileName() instead. 2. An unused variable 'space_pos' that was declared but never used. 3. An unnecessary scope block around the GetModuleFileName() call. 4. Inconsistent buffer sizing: exe_path used MAX_PATH while cmdline used 1024. This appears to be code that was refactored when it was realized GetCommandLine() wouldn't work correctly (it returns the full command line with arguments, not just the executable path), but the old approach was never fully removed. Clean this up by: - Removing the redundant first snprintf() call - Removing the unused space_pos variable - Removing the unnecessary scope block - Using consistent 1024-byte buffer size for both exe_path and cmdline - Adding a clearer comment explaining the purpose of the code - Removed some simplistic, unneeded comments Per code review from Tom Lane. Backpatch-through: 16, like commit c507ba55 Author: Bryan Green Reviewed-by: Tom Lane Reviewed-by: Thomas Munro Discussion: https://postgr.es/m/1086088.1765593851%40sss.pgh.pa.us --- src/test/modules/test_cloexec/test_cloexec.c | 40 ++++++-------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c index 7ff6585d710..621a3cc02f5 100644 --- a/src/test/modules/test_cloexec/test_cloexec.c +++ b/src/test/modules/test_cloexec/test_cloexec.c @@ -80,16 +80,15 @@ run_parent_tests(const char *testfile1, const char *testfile2) 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) { @@ -97,9 +96,7 @@ run_parent_tests(const char *testfile1, const char *testfile2) 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) { @@ -123,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); @@ -182,7 +162,9 @@ 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"); -- 2.51.2