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 <[email protected]>
Co-authored-by: Thomas Munro <[email protected]>
Reported-by: Tom Lane <[email protected]>
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