v1-0001-Review-comments-io_direct-GUC.txt

From b3eed3d6fc849b9e16fbace1f37d401424f81ab0 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 25 Jan 2023 07:18:11 +0000
Subject: [PATCH v1] Review comments io_direct GUC

---
 src/backend/storage/file/fd.c       | 57 ++++++++++++-----------------
 src/backend/utils/misc/guc_tables.c |  4 +-
 src/include/utils/guc_hooks.h       |  1 -
 3 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index eb83de4fb9..329acc2ffd 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -3749,7 +3749,7 @@ data_sync_elevel(int elevel)
 bool
 check_io_direct(char **newval, void **extra, GucSource source)
 {
-	int		   *flags = guc_malloc(ERROR, sizeof(*flags));
+	int		flags;
 
 #if PG_O_DIRECT == 0
 	if (strcmp(*newval, "") != 0)
@@ -3757,38 +3757,51 @@ check_io_direct(char **newval, void **extra, GucSource source)
 		GUC_check_errdetail("io_direct is not supported on this platform.");
 		return false;
 	}
-	*flags = 0;
+	flags = 0;
 #else
-	List	   *list;
+	List	   *elemlist;
 	ListCell   *l;
+	char	   *rawstring;
 
-	if (!SplitGUCList(*newval, ',', &list))
+	/* Need a modifiable copy of string */
+	rawstring = pstrdup(*newval);
+
+	if (!SplitGUCList(rawstring, ',', &elemlist))
 	{
 		GUC_check_errdetail("invalid list syntax in parameter \"%s\"",
 							"io_direct");
+		pfree(rawstring);
+		list_free(elemlist);
 		return false;
 	}
 
-	*flags = 0;
-	foreach (l, list)
+	flags = 0;
+	foreach (l, elemlist)
 	{
 		char	   *item = (char *) lfirst(l);
 
 		if (pg_strcasecmp(item, "data") == 0)
-			*flags |= IO_DIRECT_DATA;
+			flags |= IO_DIRECT_DATA;
 		else if (pg_strcasecmp(item, "wal") == 0)
-			*flags |= IO_DIRECT_WAL;
+			flags |= IO_DIRECT_WAL;
 		else if (pg_strcasecmp(item, "wal_init") == 0)
-			*flags |= IO_DIRECT_WAL_INIT;
+			flags |= IO_DIRECT_WAL_INIT;
 		else
 		{
 			GUC_check_errdetail("invalid option \"%s\"", item);
+			pfree(rawstring);
+			list_free(elemlist);
 			return false;
 		}
 	}
+
+	pfree(rawstring);
+	list_free(elemlist);
 #endif
 
-	*extra = flags;
+	/* Save the flags in *extra, for use by assign_io_direct */
+	*extra = guc_malloc(ERROR, sizeof(int));
+	*((int *) *extra) = flags;
 
 	return true;
 }
@@ -3800,27 +3813,3 @@ assign_io_direct(const char *newval, void *extra)
 
 	io_direct_flags = *flags;
 }
-
-extern const char *
-show_io_direct(void)
-{
-	static char result[80];
-
-	result[0] = 0;
-	if (io_direct_flags & IO_DIRECT_DATA)
-		strcat(result, "data");
-	if (io_direct_flags & IO_DIRECT_WAL)
-	{
-		if (result[0])
-			strcat(result, ", ");
-		strcat(result, "wal");
-	}
-	if (io_direct_flags & IO_DIRECT_WAL_INIT)
-	{
-		if (result[0])
-			strcat(result, ", ");
-		strcat(result, "wal_init");
-	}
-
-	return result;
-}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 9410493ae7..25b7e87abb 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4529,11 +4529,11 @@ struct config_string ConfigureNamesString[] =
 		{"io_direct", PGC_POSTMASTER, DEVELOPER_OPTIONS,
 			gettext_noop("Use direct I/O for file access."),
 			NULL,
-			GUC_NOT_IN_SAMPLE
+			GUC_LIST_INPUT | GUC_NOT_IN_SAMPLE
 		},
 		&io_direct_string,
 		"",
-		check_io_direct, assign_io_direct, show_io_direct
+		check_io_direct, assign_io_direct, NULL
 	},
 
 	/* End-of-list marker */
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index e656a16a40..b3b5148185 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -156,6 +156,5 @@ extern void assign_wal_consistency_checking(const char *newval, void *extra);
 extern void assign_xlog_sync_method(int new_sync_method, void *extra);
 extern bool check_io_direct(char **newval, void **extra, GucSource source);
 extern void assign_io_direct(const char *newval, void *extra);
-extern const char *show_io_direct(void);
 
 #endif							/* GUC_HOOKS_H */
-- 
2.34.1