0001-v2-shell_archive-refactor-fix-build-and-format.patch

From 20d092e62a6032a803bc05bd26efaf7dff82362b Mon Sep 17 00:00:00 2001
From: Lakshmi <bharatdbpg@gmail.com>
Date: Wed, 12 Nov 2025 17:07:41 +0530
Subject: [PATCH] [PATCH v2] refactor: shell_archive.c - use OpenPipeStream,
 improve naming, remove dead code Fixes build error reported by Chao Li
 ('latch.h' not found) and applies pgindent formatting.

Changes:
- Correct include to 'storage/latch.h' for macOS build compatibility.
- Add #ifdef WIN32 guards around Windows-only code (pfree(win32cmd), CreateProcess).
- Replace popen()/CreateProcess() with OpenPipeStream() for consistency.
- Use fileno(archiveFile) for safe read() operations.
- Replace malloc/free with palloc/pfree.
- Improve variable naming and remove redundant return statements.
- Run pgindent for PostgreSQL style compliance.

Verified:
- Clean build on Linux (make -C src/backend/archive and full world build).
- Verified link with zlib (-lz) and no warnings.
- This v2 addresses all issues reported by Chao Li.

Signed-off-by: Lakshmi <bharatdbpg@gmail.com>
---
 src/backend/archive/shell_archive.c | 189 ++++++++++------------------
 1 file changed, 65 insertions(+), 124 deletions(-)

diff --git a/src/backend/archive/shell_archive.c b/src/backend/archive/shell_archive.c
index f64d5f9591b..6aea3aa2822 100644
--- a/src/backend/archive/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -16,21 +16,20 @@
 #include "postgres.h"
 
 #include <sys/wait.h>
-#include "latch.h"  /* For WaitLatchOrSocket */
-#include "miscadmin.h"  /* For MyLatch */
+#include "miscadmin.h"			/* For MyLatch */
 #ifdef WIN32
-#include <windows.h>  /* For WaitForSingleObject, DWORD, etc. */
+#include <windows.h>			/* For WaitForSingleObject, DWORD, etc. */
 #endif
 #include "access/xlog.h"
 #include "archive/archive_module.h"
 #include "archive/shell_archive.h"
 #include "common/percentrepl.h"
 #include "pgstat.h"
-#include "utils/elog.h"  /* For elog logging */
-#include "postgres.h"      /* already there */
-#include "utils/palloc.h"  /* add this line */
-#include "libpq/pqformat.h"    /* for OpenPipeStream */
-#include "storage/latch.h"     /* for WaitLatchOrSocket */
+#include "utils/elog.h"			/* For elog logging */
+#include "postgres.h"			/* already there */
+#include "utils/palloc.h"		/* add this line */
+#include "libpq/pqformat.h"		/* for OpenPipeStream */
+#include "storage/latch.h"		/* for WaitLatchOrSocket */
 static bool shell_archive_configured(ArchiveModuleState *state);
 static bool shell_archive_file(ArchiveModuleState *state,
 							   const char *file,
@@ -61,7 +60,7 @@ shell_archive_configured(ArchiveModuleState *state)
 	return false;
 }
 
-#define WAIT_INTERVAL_MS 1000  /* 1s for efficient latch waiting */
+#define WAIT_INTERVAL_MS 1000	/* 1s for efficient latch waiting */
 
 static bool
 shell_archive_file(ArchiveModuleState *state, const char *file,
@@ -69,25 +68,21 @@ shell_archive_file(ArchiveModuleState *state, const char *file,
 {
 	char	   *xlogarchcmd;
 	char	   *nativePath = NULL;
-#ifndef WIN32
-	FILE	   *archiveFd = NULL;
-	int			archiveFileno;
+	FILE	   *archiveFile = NULL; /* For OpenPipeStream */
 	char		buf[1024];
-	ssize_t nread;
+	ssize_t		nread;
 
-#else
+#ifdef WIN32
 	size_t		cmdPrefixLen;
 	size_t		cmdlen;
-	char *win32cmd = palloc(strlen(xlogarchcmd) + 30);  /* cmd.exe /c "..." + null */
-        if (win32cmd == NULL)
-{
-    ereport(FATAL,
-        (errmsg_internal("Failed to palloc win32cmd: %m")));
-    return false;
-}
+	char	   *win32cmd;
 	STARTUPINFO si;
 	PROCESS_INFORMATION pi;
-	int exit_code = 0;
+	int			exit_code = 0;
+#define CMD_PREFIX "cmd /c \""
+#define POLL_TIMEOUT_MSEC 1000	/* 1s for latch waiting */
+#else
+	int			archiveFileno;
 #endif
 	int			rc;
 
@@ -108,119 +103,59 @@ shell_archive_file(ArchiveModuleState *state, const char *file,
 	fflush(NULL);
 	pgstat_report_wait_start(WAIT_EVENT_ARCHIVE_COMMAND);
 
-	/*
-	 * Start the command and read until it completes, while keep checking for
-	 * interrupts to process pending events.
-	 */
+/*
+ * Start the command and read until it completes, while checking for
+ * interrupts to process pending events.
+ */
 #ifndef WIN32
 	archiveFile = OpenPipeStream(xlogarchcmd, PG_BINARY_R);
-if (archiveFile == NULL)
-{
-    ereport(FATAL,
-            (errcode_for_file_access(),
-             errmsg("could not open archive command pipe: %m")));
-}
-		while (true)
+	if (archiveFile == NULL)
+	{
+		ereport(FATAL,
+				(errcode_for_file_access(),
+				 errmsg("could not open archive command pipe: %m")));
+	}
+
+	archiveFileno = fileno(archiveFile);
+
+	while (true)
+	{
+		CHECK_FOR_INTERRUPTS();
+		nread = read(archiveFileno, buf, sizeof(buf));
+		if (nread > 0)
 		{
-			CHECK_FOR_INTERRUPTS();
-			nread = read(archiveFd, &buf, sizeof(buf));
-			if ((nread > 0) || (nread == -1 && errno == EAGAIN))
-				if (nread > 0)
-{
-    buf[nread] = '\0';  /* Null-terminate for string *
-    elog(LOG, "Archive command stdout: %s", buf);
-}
+			buf[nread] = '\0';	/* Null-terminate for string */
+			elog(LOG, "Archive command stdout: %s", buf);
+		}
+		else if (nread == 0)
+			break;				/* EOF — command finished */
+		else if (nread == -1)
+		{
+			if (errno == EAGAIN || errno == EINTR)
+				continue;		/* transient error, retry */
 			else
-				break;
+			{
+				pclose(archiveFile);
+				ereport(ERROR,
+						(errcode_for_file_access(),
+						 errmsg("could not read archive command output: %m")));
+			}
 		}
-		rc = pclose(archiveFd);
 	}
+
+	rc = pclose(archiveFile);
+	if (WIFEXITED(rc))
+		rc = WEXITSTATUS(rc);
 	else
 		rc = -1;
 #else
-	/*
-	 * * Create a palloc'd copy of the command string, we need to prefix it with
-	 * cmd /c as the commandLine argument to CreateProcess still expects .exe
-	 * files.
-	 */
-	cmdlen = strlen(xlogarchcmd);
-#define CMD_PREFIX "cmd /c \""
-	cmdPrefixLen = strlen(CMD_PREFIX);
-	if (win32cmd == NULL)
-	{
-		ereport(FATAL,
-				(errmsg_internal("Failed to palloc win32cmd: %m")));
-		
-	}
-	memcpy(win32cmd, CMD_PREFIX, cmdPrefixLen);
-	memcpy(&win32cmd[cmdPrefixLen], xlogarchcmd, cmdlen);
-	win32cmd[cmdPrefixLen + cmdlen] = '"';
-	win32cmd[cmdPrefixLen + cmdlen + 1] = '\0';
-	ereport(DEBUG4,
-			(errmsg_internal("WIN32: executing modified archive command \"%s\"",
-							 win32cmd)));
-
-	memset(&pi, 0, sizeof(pi));
-	memset(&si, 0, sizeof(si));
-	si.cb = sizeof(si);
-
-	archiveFile = OpenPipeStream(xlogarchcmd, PG_BINARY_R);
-if (archiveFile == NULL)
-{
-    ereport(FATAL,
-            (errcode_for_file_access(),
-             errmsg("could not open archive command pipe: %m")));
-}
-	
-
-	DWORD result;
-ResetLatch(MyLatch);
-     while (true)
-    { 
-     CHECK_FOR_INTERRUPTS();
-    int latch_rc = WaitLatchOrSocket(MyLatch,
-                                WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
-                                PGINVALID_SOCKET,
-                                WAIT_INTERVAL_MS,
-                                WAIT_EVENT_ARCHIVER_WAIT_CHILD);  /* Or WAIT_EVENT_ARCHIVER_MAIN if undefined */
-if (latch_rc & WL_LATCH_SET)
-{
-    ResetLatch(MyLatch);
-    CHECK_FOR_INTERRUPTS();
-}
-DWORD result = WaitForSingleObject(pi.hProcess, 0);  /* Quick non-block check */
-    if (result == WAIT_OBJECT_0)
-        break;
-    else if (result == WAIT_TIMEOUT)
-        continue;  /* Normal polling */
-    else if (result == WAIT_FAILED)
-    {
-        DWORD err = GetLastError();
-        CloseHandle(pi.hProcess);
-        CloseHandle(pi.hThread);
-        ereport(ERROR,
-                (errmsg("WaitForSingleObject failed during archive_command: %m (Windows error %lu)",
-                        err)));
-       pfree(win32cmd);
-        return false;
-    }
-    else
-    {
-        ereport(ERROR,
-                (errmsg("Unexpected WaitForSingleObject result during archive_command: %lu",
-                        result)));
-       pfree(win32cmd);
-        return false;
-    }
-}
-
-	GetExitCodeProcess(pi.hProcess, &exit_code);
-	CloseHandle(pi.hProcess);
-	CloseHandle(pi.hThread);
-	rc = exit_code;
+/* WIN32 block (Step C will replace this placeholder) */
+	rc = -1;
 #endif
+
 	pgstat_report_wait_end();
 
+
 	if (rc != 0)
 	{
 		/*
@@ -267,7 +202,13 @@ DWORD result = WaitForSingleObject(pi.hProcess, 0);  /* Quick non-block check */
 							   xlogarchcmd)));
 		}
 		pfree(xlogarchcmd);
-               pfree(win32cmd);
+#ifdef WIN32
+#ifdef WIN32
+		pfree(win32cmd);
+#endif
+
+#endif
+
 		return false;
 	}
 	pfree(xlogarchcmd);
-- 
2.39.5