Clean up test_cloexec.c and Makefile.

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 c507ba55
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
index 70d38575e2653c8b76f1f4078248d168429d5656..cd16a59add5a80a90487448a628a210b557e09e2 100644 (file)
--- 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
index 9f64545168440bd5b886869b4baa72a73d9be542..aec8af9937d0ff19dc173fd4273e644b46c30d15 100644 (file)
--- 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,26 +69,26 @@ 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)
        {
@@ -95,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)
        {
@@ -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