From 2a662b72c6acd469174250aaca6a7d60da856037 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Date: Tue, 24 Mar 2026 15:43:41 +0900
Subject: [PATCH v18 2/2] Address comments from Chao, Amit and Kuroda

---
 src/bin/pg_basebackup/pg_createsubscriber.c | 72 ++++++++++++---------
 1 file changed, 42 insertions(+), 30 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 63ffd337613..1f6247a8cd8 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -160,8 +160,9 @@ static void report_createsub_log_v(enum pg_log_level level, enum pg_log_part par
 pg_noreturn static void report_createsub_fatal(const char *pg_restrict fmt,...)
 			pg_attribute_printf(1, 2);
 static void internal_log_file_write(enum pg_log_level level,
+									enum pg_log_part part,
 									const char *pg_restrict fmt, va_list args)
-			pg_attribute_printf(2, 0);
+			pg_attribute_printf(3, 0);
 
 #define	WAIT_INTERVAL	1		/* 1 second */
 
@@ -209,7 +210,7 @@ report_createsub_log_v(enum pg_log_level level, enum pg_log_part part,
 		va_list		arg_cpy;
 
 		va_copy(arg_cpy, args);
-		internal_log_file_write(level, fmt, arg_cpy);
+		internal_log_file_write(level, part, fmt, arg_cpy);
 		va_end(arg_cpy);
 		/* Restore errno in case internal_log_file_write changed it */
 		errno = save_errno;
@@ -348,13 +349,6 @@ cleanup_objects_atexit(void)
 
 	if (standby_running)
 		stop_standby_server(subscriber_dir);
-
-	if (internal_log_file_fp != NULL)
-	{
-		if (fclose(internal_log_file_fp) != 0)
-			report_createsub_fatal("could not close %s/%s.log: %m", logdir, INTERNAL_LOG_FILE_NAME);
-		internal_log_file_fp = NULL;
-	}
 }
 
 static void
@@ -1081,8 +1075,8 @@ server_is_in_recovery(PGconn *conn)
 }
 
 static void
-internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
-						va_list args)
+internal_log_file_write(enum pg_log_level level, enum pg_log_part part,
+						const char *pg_restrict fmt, va_list args)
 {
 	Assert(internal_log_file_fp);
 
@@ -1090,6 +1084,30 @@ internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
 	if (level < __pg_log_level)
 		return;
 
+	/* Append prefix based on the log part and log level */
+	switch (part)
+	{
+		case PG_LOG_PRIMARY:
+			switch (level)
+			{
+				case PG_LOG_ERROR:
+					fprintf(internal_log_file_fp, _("error: "));
+					break;
+				case PG_LOG_WARNING:
+					fprintf(internal_log_file_fp, _("warning: "));
+					break;
+				default:
+					break;
+			}
+			break;
+		case PG_LOG_DETAIL:
+			fprintf(internal_log_file_fp, _("detail: "));
+			break;
+		case PG_LOG_HINT:
+			fprintf(internal_log_file_fp, _("hint: "));
+			break;
+	}
+
 	vfprintf(internal_log_file_fp, _(fmt), args);
 
 	fprintf(internal_log_file_fp, "\n");
@@ -1098,28 +1116,15 @@ internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
 
 /*
  * Open a new logfile with proper permissions.
- * From src/backend/postmaster/syslogger.c
  */
 static FILE *
 logfile_open(const char *filename, const char *mode)
 {
 	FILE	   *fh;
-	mode_t		oumask;
 
-	oumask = umask((mode_t) ((~(S_IRUSR | S_IWUSR)) & (S_IRWXU | S_IRWXG | S_IRWXO)));
 	fh = fopen(filename, mode);
-	umask(oumask);
 
-	if (fh)
-	{
-		setvbuf(fh, NULL, PG_IOLBF, 0);
-
-#ifdef WIN32
-		/* use CRLF line endings on Windows */
-		_setmode(_fileno(fh), _O_TEXT);
-#endif
-	}
-	else
+	if (!fh)
 		report_createsub_fatal("could not open log file \"%s\": %m",
 							   filename);
 
@@ -1151,8 +1156,8 @@ make_output_dirs(const char *log_basedir)
 	len = snprintf(logdir, MAXPGPATH, "%s/%s", log_basedir, timestamp);
 
 	if (len >= MAXPGPATH)
-		report_createsub_fatal("directory path for log files, %s/%s, is too long",
-							   logdir, timestamp);
+		report_createsub_fatal("directory path for log files \"%s/%s\" is too long",
+							   log_basedir, timestamp);
 
 	/* Create base directory (ignore if exists) */
 	if (mkdir(log_basedir, pg_dir_create_mode) < 0 && errno != EEXIST)
@@ -2756,10 +2761,20 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
+	/* Rudimentary check for a data directory */
+	check_data_directory(subscriber_dir);
+
 	if (opt.log_dir != NULL)
 	{
 		char	   *internal_log_file;
 
+		/* Set mask based on the PGDATA permissions */
+		if (!GetDataDirectoryCreatePerm(subscriber_dir))
+			report_createsub_fatal("could not read permissions of directory \"%s\": %m",
+								   subscriber_dir);
+
+		umask(pg_mode_mask);
+
 		make_output_dirs(opt.log_dir);
 		internal_log_file = psprintf("%s/%s.log", logdir,
 									 INTERNAL_LOG_FILE_NAME);
@@ -2875,9 +2890,6 @@ main(int argc, char **argv)
 	pg_ctl_path = get_exec_path(argv[0], "pg_ctl");
 	pg_resetwal_path = get_exec_path(argv[0], "pg_resetwal");
 
-	/* Rudimentary check for a data directory */
-	check_data_directory(subscriber_dir);
-
 	dbinfos.two_phase = opt.two_phase;
 
 	/*
-- 
2.47.3