v15-0002-Some-simplifications-from-me.patch

From 402d42283b872b1abd1038a9e640a67f8a3248b9 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 2 Nov 2022 15:45:41 +0900
Subject: [PATCH v15 2/2] Some simplifications from me

---
 src/include/utils/conffiles.h       |  23 ++++
 src/include/utils/guc.h             |   2 -
 src/backend/libpq/hba.c             |  66 ++++++------
 src/backend/utils/misc/Makefile     |   1 +
 src/backend/utils/misc/conffiles.c  | 161 ++++++++++++++++++++++++++++
 src/backend/utils/misc/guc-file.l   | 138 +-----------------------
 src/backend/utils/misc/meson.build  |   1 +
 src/test/authentication/meson.build |   1 +
 8 files changed, 221 insertions(+), 172 deletions(-)
 create mode 100644 src/include/utils/conffiles.h
 create mode 100644 src/backend/utils/misc/conffiles.c

diff --git a/src/include/utils/conffiles.h b/src/include/utils/conffiles.h
new file mode 100644
index 0000000000..3f23a2a011
--- /dev/null
+++ b/src/include/utils/conffiles.h
@@ -0,0 +1,23 @@
+/*--------------------------------------------------------------------
+ * conffiles.h
+ *
+ * Utilities related to configuration files.
+ *
+ * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/utils/conffiles.h
+ *
+ *--------------------------------------------------------------------
+ */
+#ifndef CONFFILES_H
+#define CONFFILES_H
+
+extern char *AbsoluteConfigLocation(const char *location,
+									const char *calling_file);
+extern char **GetConfFilesInDir(const char *includedir,
+								const char *calling_file,
+								int elevel, int *num_filenames,
+								char **err_msg);
+
+#endif							/* CONFFILES_H */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 59ca39d908..b3aaff9665 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -144,8 +144,6 @@ typedef struct ConfigVariable
 	struct ConfigVariable *next;
 } ConfigVariable;
 
-extern char **GetDirConfFiles(const char *includedir, const char *calling_file,
-							  int elevel, int *num_filenames, char **err_msg);
 extern bool ParseConfigFile(const char *config_file, bool strict,
 							const char *calling_file, int calling_lineno,
 							int depth, int elevel,
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 1034ff8220..7ebbd5d7b1 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -22,7 +22,6 @@
 #include <sys/param.h>
 #include <sys/socket.h>
 #include <netdb.h>
-#include <sys/stat.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <unistd.h>
@@ -42,6 +41,7 @@
 #include "storage/fd.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
+#include "utils/conffiles.h"
 #include "utils/guc.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -122,10 +122,9 @@ static const char *const UserAuthName[] =
 };
 
 
-static void tokenize_file_with_context(MemoryContext linecxt,
-									   const char *filename, FILE *file,
-									   List **tok_lines, int depth,
-									   int elevel);
+static void tokenize_auth_file_internal(const char *filename, FILE *file,
+								   List **tok_lines, int depth,
+								   int elevel);
 static List *tokenize_inc_file(List *tokens, const char *outer_filename,
 							   const char *inc_filename, int depth, int elevel,
 							   char **err_msg);
@@ -138,9 +137,9 @@ static int	regexec_auth_token(const char *match, AuthToken *token,
 static FILE *open_inc_file(HbaIncludeKind kind, const char *inc_filename,
 						   bool strict, const char *outer_filename, int elevel,
 						   char **err_msg, char **inc_fullname);
-static char *process_included_authfile(const char *inc_filename, bool strict,
+static char *process_included_auth_file(const char *inc_filename, bool strict,
 									   const char *outer_filename, int depth,
-									   int elevel, MemoryContext linecxt,
+									   int elevel,
 									   List **tok_lines);
 
 
@@ -508,8 +507,8 @@ tokenize_inc_file(List *tokens,
 		return tokens;
 
 	/* There is possible recursion here if the file contains @ */
-	linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, depth + 1,
-								 elevel);
+	linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines,
+								depth + 1, elevel);
 
 	FreeFile(inc_file);
 	pfree(inc_fullname);
@@ -548,8 +547,9 @@ tokenize_inc_file(List *tokens,
 /*
  * tokenize_auth_file
  *
- * Wrapper around tokenize_file_with_context, creating a dedicated memory
- * context.
+ * Wrapper around tokenize_auth_file_internal, creating a dedicated memory
+ * context where all the magic happens, working as an entry point for
+ * the tokenization of the HBA and ident files.
  *
  * Return value is this memory context which contains all memory allocated by
  * this function (it's a child of caller's context).
@@ -559,14 +559,19 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines,
 				   int depth, int elevel)
 {
 	MemoryContext linecxt;
+	MemoryContext oldcxt;
+
 	linecxt = AllocSetContextCreate(CurrentMemoryContext,
 									"tokenize_auth_file",
 									ALLOCSET_SMALL_SIZES);
+	oldcxt = MemoryContextSwitchTo(linecxt);
 
 	*tok_lines = NIL;
 
-	tokenize_file_with_context(linecxt, filename, file, tok_lines, depth,
-							   elevel);
+	/* can recurse into itself */
+	tokenize_auth_file_internal(filename, file, tok_lines, depth, elevel);
+
+	MemoryContextSwitchTo(oldcxt);
 
 	return linecxt;
 }
@@ -577,8 +582,6 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines,
  * The output is a list of TokenizedAuthLine structs; see the struct definition
  * in libpq/hba.h.
  *
- * linecxt: memory context which must contain all memory allocated by the
- * function
  * filename: the absolute path to the target file
  * file: the already-opened target file
  * tok_lines: receives output list
@@ -589,14 +592,11 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines,
  * output list.
  */
 static void
-tokenize_file_with_context(MemoryContext linecxt, const char *filename,
-						   FILE *file, List **tok_lines, int depth, int elevel)
+tokenize_auth_file_internal(const char *filename,
+					   FILE *file, List **tok_lines, int depth, int elevel)
 {
 	StringInfoData buf;
 	int			line_number = 1;
-	MemoryContext oldcxt;
-
-	oldcxt = MemoryContextSwitchTo(linecxt);
 
 	initStringInfo(&buf);
 
@@ -683,8 +683,8 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename,
 
 				inc_filename = second->string;
 
-				err_msg = process_included_authfile(inc_filename, true,
-										  filename, depth + 1, elevel, linecxt,
+				err_msg = process_included_auth_file(inc_filename, true,
+										  filename, depth + 1, elevel,
 										  tok_lines);
 
 				if (!err_msg)
@@ -703,7 +703,7 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename,
 				int			num_filenames;
 				StringInfoData err_buf;
 
-				filenames = GetDirConfFiles(dir_name, filename, elevel,
+				filenames = GetConfFilesInDir(dir_name, filename, elevel,
 						&num_filenames, &err_msg);
 
 				if (!filenames)
@@ -720,9 +720,9 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename,
 					 * overwritten at the end of the loop with the
 					 * cumulated errors, if any.
 					 */
-					err_msg = process_included_authfile(filenames[i], true,
+					err_msg = process_included_auth_file(filenames[i], true,
 												filename, depth + 1, elevel,
-												linecxt, tok_lines);
+												tok_lines);
 
 					/* Cumulate errors if any. */
 					if (err_msg)
@@ -749,8 +749,8 @@ tokenize_file_with_context(MemoryContext linecxt, const char *filename,
 
 				inc_filename = second->string;
 
-				err_msg = process_included_authfile(inc_filename, false,
-										  filename, depth + 1, elevel, linecxt,
+				err_msg = process_included_auth_file(inc_filename, false,
+										  filename, depth + 1, elevel,
 										  tok_lines);
 
 				if (!err_msg)
@@ -780,8 +780,6 @@ process_line:
 next_line:
 		line_number += continuations + 1;
 	}
-
-	MemoryContextSwitchTo(oldcxt);
 }
 
 /*
@@ -2650,9 +2648,9 @@ open_inc_file(HbaIncludeKind kind, const char *inc_filename, bool strict,
  * Returns NULL if no error happens during tokenization, otherwise the error.
  */
 static char *
-process_included_authfile(const char *inc_filename, bool strict,
-						  const char *outer_filename, int depth, int elevel,
-						  MemoryContext linecxt, List **tok_lines)
+process_included_auth_file(const char *inc_filename, bool strict,
+						   const char *outer_filename, int depth, int elevel,
+						   List **tok_lines)
 {
 	char	   *inc_fullname;
 	FILE	   *inc_file;
@@ -2693,8 +2691,8 @@ process_included_authfile(const char *inc_filename, bool strict,
 		Assert(err_msg == NULL);
 	}
 
-	tokenize_file_with_context(linecxt, inc_fullname, inc_file,
-							   tok_lines, depth, elevel);
+	tokenize_auth_file_internal(inc_fullname, inc_file,
+								tok_lines, depth, elevel);
 
 	FreeFile(inc_file);
 	pfree(inc_fullname);
diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile
index 6097309033..b9ee4eb48a 100644
--- a/src/backend/utils/misc/Makefile
+++ b/src/backend/utils/misc/Makefile
@@ -15,6 +15,7 @@ include $(top_builddir)/src/Makefile.global
 override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
 
 OBJS = \
+	conffiles.o \
 	guc.o \
 	guc-file.o \
 	guc_funcs.o \
diff --git a/src/backend/utils/misc/conffiles.c b/src/backend/utils/misc/conffiles.c
new file mode 100644
index 0000000000..c9659a7c70
--- /dev/null
+++ b/src/backend/utils/misc/conffiles.c
@@ -0,0 +1,161 @@
+/*--------------------------------------------------------------------
+ * conffiles.c
+ *
+ * Utilities and tools related to the handling of configuration files.
+ *
+ * This file contains the generic tools to work on configuration files
+ * used by PostgreSQL, be they related to GUC, HBA or ident files.
+ *
+ *
+ * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/misc/conffiles.c
+ *
+ *--------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <dirent.h>
+
+#include "common/file_utils.h"
+#include "miscadmin.h"
+#include "storage/fd.h"
+#include "utils/conffiles.h"
+
+/*
+ * Given a configuration file or directory location that may be a relative
+ * path, return an absolute one.  We consider the location to be relative to
+ * the directory holding the calling file, or to DataDir if no calling file.
+ */
+char *
+AbsoluteConfigLocation(const char *location, const char *calling_file)
+{
+	char		abs_path[MAXPGPATH];
+
+	if (is_absolute_path(location))
+		return pstrdup(location);
+	else
+	{
+		if (calling_file != NULL)
+		{
+			strlcpy(abs_path, calling_file, sizeof(abs_path));
+			get_parent_directory(abs_path);
+			join_path_components(abs_path, abs_path, location);
+			canonicalize_path(abs_path);
+		}
+		else
+		{
+			Assert(DataDir);
+			join_path_components(abs_path, DataDir, location);
+			canonicalize_path(abs_path);
+		}
+		return pstrdup(abs_path);
+	}
+}
+
+
+/*
+ * Returns the list of config files located in a directory, in alphabetical
+ * order.
+ *
+ * On error, returns NULL with details stored in "err_msg".
+ */
+char **
+GetConfFilesInDir(const char *includedir, const char *calling_file,
+				  int elevel, int *num_filenames, char **err_msg)
+{
+	char	   *directory;
+	DIR		   *d;
+	struct dirent *de;
+	char	  **filenames = NULL;
+	int			size_filenames;
+
+	/*
+	 * Reject directory name that is all-blank (including empty), as that
+	 * leads to confusion --- we'd read the containing directory, typically
+	 * resulting in recursive inclusion of the same file(s).
+	 */
+	if (strspn(includedir, " \t\r\n") == strlen(includedir))
+	{
+		ereport(elevel,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("empty configuration directory name: \"%s\"",
+						includedir)));
+		*err_msg = "empty configuration directory name";
+		return NULL;
+	}
+
+	directory = AbsoluteConfigLocation(includedir, calling_file);
+	d = AllocateDir(directory);
+	if (d == NULL)
+	{
+		ereport(elevel,
+				(errcode_for_file_access(),
+				 errmsg("could not open configuration directory \"%s\": %m",
+						directory)));
+		*err_msg = psprintf("could not open directory \"%s\"", directory);
+		goto cleanup;
+	}
+
+	/*
+	 * Read the directory and put the filenames in an array, so we can sort
+	 * them prior to caller processing the contents.
+	 */
+	size_filenames = 32;
+	filenames = (char **) palloc(size_filenames * sizeof(char *));
+	*num_filenames = 0;
+
+	while ((de = ReadDir(d, directory)) != NULL)
+	{
+		PGFileType	de_type;
+		char		filename[MAXPGPATH];
+
+		/*
+		 * Only parse files with names ending in ".conf".  Explicitly reject
+		 * files starting with ".".  This excludes things like "." and "..",
+		 * as well as typical hidden files, backup files, and editor debris.
+		 */
+		if (strlen(de->d_name) < 6)
+			continue;
+		if (de->d_name[0] == '.')
+			continue;
+		if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0)
+			continue;
+
+		join_path_components(filename, directory, de->d_name);
+		canonicalize_path(filename);
+		de_type = get_dirent_type(filename, de, true, elevel);
+		if (de_type == PGFILETYPE_ERROR)
+		{
+			*err_msg = psprintf("could not stat file \"%s\"", filename);
+			pfree(filenames);
+			filenames = NULL;
+			goto cleanup;
+		}
+		else if (de_type != PGFILETYPE_DIR)
+		{
+			/* Add file to array, increasing its size in blocks of 32 */
+			if (*num_filenames >= size_filenames)
+			{
+				size_filenames += 32;
+				filenames = (char **) repalloc(filenames,
+										size_filenames * sizeof(char *));
+			}
+			filenames[*num_filenames] = pstrdup(filename);
+			(*num_filenames)++;
+		}
+	}
+
+	/* Sort the files by name before leaving */
+	if (*num_filenames > 0)
+		qsort(filenames, *num_filenames, sizeof(char *), pg_qsort_strcmp);
+
+cleanup:
+	if (d)
+		FreeDir(d);
+	pfree(directory);
+	return filenames;
+}
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index f76a56398b..937dc5fbf8 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -18,6 +18,7 @@
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include <sys/stat.h>
+#include "utils/conffiles.h"
 #include "utils/memutils.h"
 }
 
@@ -156,37 +157,6 @@ ProcessConfigFile(GucContext context)
 	MemoryContextDelete(config_cxt);
 }
 
-/*
- * Given a configuration file or directory location that may be a relative
- * path, return an absolute one.  We consider the location to be relative to
- * the directory holding the calling file, or to DataDir if no calling file.
- */
-static char *
-AbsoluteConfigLocation(const char *location, const char *calling_file)
-{
-	char		abs_path[MAXPGPATH];
-
-	if (is_absolute_path(location))
-		return pstrdup(location);
-	else
-	{
-		if (calling_file != NULL)
-		{
-			strlcpy(abs_path, calling_file, sizeof(abs_path));
-			get_parent_directory(abs_path);
-			join_path_components(abs_path, abs_path, location);
-			canonicalize_path(abs_path);
-		}
-		else
-		{
-			Assert(DataDir);
-			join_path_components(abs_path, DataDir, location);
-			canonicalize_path(abs_path);
-		}
-		return pstrdup(abs_path);
-	}
-}
-
 /*
  * Read and parse a single configuration file.  This function recurses
  * to handle "include" directives.
@@ -345,110 +315,6 @@ GUC_flex_fatal(const char *msg)
 	return 0;					/* keep compiler quiet */
 }
 
-/*
- * Returns the list of config files located in a directory, in alphabetical
- * order.
- *
- * We don't check for recursion or too-deep nesting depth here, its up to the
- * caller to take care of that.
- */
-char **
-GetDirConfFiles(const char *includedir, const char *calling_file, int elevel,
-				int *num_filenames, char **err_msg)
-{
-	char	   *directory;
-	DIR		   *d;
-	struct dirent *de;
-	char	  **filenames;
-	int			size_filenames;
-
-	/*
-	 * Reject directory name that is all-blank (including empty), as that
-	 * leads to confusion --- we'd read the containing directory, typically
-	 * resulting in recursive inclusion of the same file(s).
-	 */
-	if (strspn(includedir, " \t\r\n") == strlen(includedir))
-	{
-		ereport(elevel,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("empty configuration directory name: \"%s\"",
-						includedir)));
-		*err_msg = "empty configuration directory name";
-		return NULL;
-	}
-
-	directory = AbsoluteConfigLocation(includedir, calling_file);
-	d = AllocateDir(directory);
-	if (d == NULL)
-	{
-		ereport(elevel,
-				(errcode_for_file_access(),
-				 errmsg("could not open configuration directory \"%s\": %m",
-						directory)));
-		*err_msg = psprintf("could not open directory \"%s\"", directory);
-		filenames = NULL;
-		goto cleanup;
-	}
-
-	/*
-	 * Read the directory and put the filenames in an array, so we can sort
-	 * them prior to caller processing the contents.
-	 */
-	size_filenames = 32;
-	filenames = (char **) palloc(size_filenames * sizeof(char *));
-	*num_filenames = 0;
-
-	while ((de = ReadDir(d, directory)) != NULL)
-	{
-		PGFileType	de_type;
-		char		filename[MAXPGPATH];
-
-		/*
-		 * Only parse files with names ending in ".conf".  Explicitly reject
-		 * files starting with ".".  This excludes things like "." and "..",
-		 * as well as typical hidden files, backup files, and editor debris.
-		 */
-		if (strlen(de->d_name) < 6)
-			continue;
-		if (de->d_name[0] == '.')
-			continue;
-		if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0)
-			continue;
-
-		join_path_components(filename, directory, de->d_name);
-		canonicalize_path(filename);
-		de_type = get_dirent_type(filename, de, true, elevel);
-		if (de_type == PGFILETYPE_ERROR)
-		{
-			*err_msg = psprintf("could not stat file \"%s\"", filename);
-			pfree(filenames);
-			filenames = NULL;
-			goto cleanup;
-		}
-		else if (de_type != PGFILETYPE_DIR)
-		{
-			/* Add file to array, increasing its size in blocks of 32 */
-			if (*num_filenames >= size_filenames)
-			{
-				size_filenames += 32;
-				filenames = (char **) repalloc(filenames,
-										size_filenames * sizeof(char *));
-			}
-			filenames[*num_filenames] = pstrdup(filename);
-			(*num_filenames)++;
-		}
-	}
-
-	if (*num_filenames > 0)
-		qsort(filenames, *num_filenames, sizeof(char *), pg_qsort_strcmp);
-
-cleanup:
-	if (d)
-		FreeDir(d);
-	pfree(directory);
-	return filenames;
-}
-
 /*
  * Read and parse a single configuration file.  This function recurses
  * to handle "include" directives.
@@ -714,7 +580,7 @@ ParseConfigDirectory(const char *includedir,
 	char	  **filenames;
 	int			num_filenames;
 
-	filenames = GetDirConfFiles(includedir, calling_file, elevel,
+	filenames = GetConfFilesInDir(includedir, calling_file, elevel,
 							   &num_filenames, &err_msg);
 
 	if (!filenames)
diff --git a/src/backend/utils/misc/meson.build b/src/backend/utils/misc/meson.build
index db4de225e1..e7a9730229 100644
--- a/src/backend/utils/misc/meson.build
+++ b/src/backend/utils/misc/meson.build
@@ -1,4 +1,5 @@
 backend_sources += files(
+  'conffiles.c',
   'guc.c',
   'guc_funcs.c',
   'guc_tables.c',
diff --git a/src/test/authentication/meson.build b/src/test/authentication/meson.build
index c2b48c43c9..cfc23fa213 100644
--- a/src/test/authentication/meson.build
+++ b/src/test/authentication/meson.build
@@ -7,6 +7,7 @@ tests += {
       't/001_password.pl',
       't/002_saslprep.pl',
       't/003_peer.pl',
+      't/004_file_inclusion.pl',
     ],
   },
 }
-- 
2.38.1