v1-0005-Clean-up-readlink-return-type.patch

text/plain

Filename: v1-0005-Clean-up-readlink-return-type.patch
Type: text/plain
Part: 4
Message: clean up size_t/ssize_t use with POSIX file system APIs

Patch

Format: format-patch
Series: patch v1-0005
Subject: Clean up readlink() return type
File+
src/backend/access/transam/xlog.c 1 1
src/backend/backup/basebackup.c 1 1
src/backend/catalog/pg_tablespace.c 1 1
src/bin/initdb/findtimezone.c 1 1
src/bin/pg_combinebackup/pg_combinebackup.c 1 1
src/bin/pg_rewind/file_ops.c 1 1
src/include/port.h 1 1
src/include/port/win32_port.h 1 1
src/port/dirmod.c 12 2
src/port/win32stat.c 6 1
From e1ab328489c50f94174694beed4782bca87cbba7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 29 Jun 2026 13:37:54 +0200
Subject: [PATCH v1 05/15] Clean up readlink() return type

The return type of readlink() per POSIX is ssize_t, but most existing
callers use int, so fix that.  Also fix the return type of the Windows
implementation to match.

In _pglstat64(), we neglected to handle the case where the output
buffer is not large enough and the result would be truncated.  This
case actually can't happen, because the Windows pgreadlink()
implementation doesn't ever return that case, but adding this seems
good for consistency with _pgstat64(), which already had this check,
and in case pgreadlink() ever changes in this regard.

Some callers of readlink(), in particular _pglstat64(), assume that
errno == EINVAL means that the file was not a symlink.  But Windows
pgreadlink() also sets EINVAL in other cases, in particular if the
buffer was too small.  This could result in incorrect behavior, so
pick a different errno.  (There might be other cases where EINVAL is
set inappropriately, but they are outside the theme of this patch.)
---
 src/backend/access/transam/xlog.c           |  2 +-
 src/backend/backup/basebackup.c             |  2 +-
 src/backend/catalog/pg_tablespace.c         |  2 +-
 src/bin/initdb/findtimezone.c               |  2 +-
 src/bin/pg_combinebackup/pg_combinebackup.c |  2 +-
 src/bin/pg_rewind/file_ops.c                |  2 +-
 src/include/port.h                          |  2 +-
 src/include/port/win32_port.h               |  2 +-
 src/port/dirmod.c                           | 14 ++++++++++++--
 src/port/win32stat.c                        |  7 ++++++-
 10 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a81912b7441..ddd0f4b7642 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9668,7 +9668,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			if (de_type == PGFILETYPE_LNK)
 			{
 				StringInfoData escapedpath;
-				int			rllen;
+				ssize_t		rllen;
 
 				rllen = readlink(fullpath, linkpath, sizeof(linkpath));
 				if (rllen < 0)
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 9c79dadaacc..74b2e2ec9b1 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -1412,7 +1412,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 		if (strcmp(path, "./pg_tblspc") == 0 && S_ISLNK(statbuf.st_mode))
 		{
 			char		linkpath[MAXPGPATH];
-			int			rllen;
+			ssize_t		rllen;
 
 			rllen = readlink(pathbuf, linkpath, sizeof(linkpath));
 			if (rllen < 0)
diff --git a/src/backend/catalog/pg_tablespace.c b/src/backend/catalog/pg_tablespace.c
index 99978bfbdbb..46aaf4c0de4 100644
--- a/src/backend/catalog/pg_tablespace.c
+++ b/src/backend/catalog/pg_tablespace.c
@@ -31,7 +31,7 @@ get_tablespace_location(Oid tablespaceOid)
 {
 	char		sourcepath[MAXPGPATH];
 	char		targetpath[MAXPGPATH];
-	int			rllen;
+	ssize_t		rllen;
 	struct stat st;
 
 	/*
diff --git a/src/bin/initdb/findtimezone.c b/src/bin/initdb/findtimezone.c
index 910b97a570b..50fe4fcb0af 100644
--- a/src/bin/initdb/findtimezone.c
+++ b/src/bin/initdb/findtimezone.c
@@ -546,7 +546,7 @@ check_system_link_file(const char *linkname, struct tztry *tt,
 {
 #ifdef HAVE_READLINK
 	char		link_target[MAXPGPATH];
-	int			len;
+	ssize_t		len;
 	const char *cur_name;
 
 	/*
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index d13bf63eb1e..c8731c474b0 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -1227,7 +1227,6 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 		Oid			oid;
 		char		tblspcdir[MAXPGPATH];
 		char		link_target[MAXPGPATH];
-		int			link_length;
 		cb_tablespace *ts;
 		cb_tablespace *otherts;
 		PGFileType	type;
@@ -1268,6 +1267,7 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 		 */
 		if (type == PGFILETYPE_LNK)
 		{
+			ssize_t		link_length;
 			cb_tablespace_mapping *tsmap;
 
 			/* Read the link target. */
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index acd8fa758fd..6a3562d8bde 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -457,7 +457,7 @@ recurse_dir(const char *datadir, const char *parentpath,
 		else if (S_ISLNK(fst.st_mode))
 		{
 			char		link_target[MAXPGPATH];
-			int			len;
+			ssize_t		len;
 
 			len = readlink(fullpath, link_target, sizeof(link_target));
 			if (len < 0)
diff --git a/src/include/port.h b/src/include/port.h
index 074a8c7e5f1..172acf7d02f 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -319,7 +319,7 @@ extern int	pgunlink(const char *path);
  */
 #if defined(WIN32) && !defined(__CYGWIN__)
 extern int	pgsymlink(const char *oldpath, const char *newpath);
-extern int	pgreadlink(const char *path, char *buf, size_t size);
+extern ssize_t pgreadlink(const char *path, char *buf, size_t size);
 
 #define symlink(oldpath, newpath)	pgsymlink(oldpath, newpath)
 #define readlink(path, buf, size)	pgreadlink(path, buf, size)
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index 2bad3688e91..aff6eaa28f6 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -237,7 +237,7 @@ typedef unsigned short mode_t;
  *		Note: Some CYGWIN includes might #define WIN32.
  */
 extern int	pgsymlink(const char *oldpath, const char *newpath);
-extern int	pgreadlink(const char *path, char *buf, size_t size);
+extern ssize_t pgreadlink(const char *path, char *buf, size_t size);
 
 #define symlink(oldpath, newpath)	pgsymlink(oldpath, newpath)
 #define readlink(path, buf, size)	pgreadlink(path, buf, size)
diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index f1f33023a3b..30725ab6aee 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -305,7 +305,7 @@ pgsymlink(const char *oldpath, const char *newpath)
 /*
  *	pgreadlink - uses Win32 junction points
  */
-int
+ssize_t
 pgreadlink(const char *path, char *buf, size_t size)
 {
 	DWORD		attr;
@@ -389,7 +389,17 @@ pgreadlink(const char *path, char *buf, size_t size)
 
 	if (r <= 0)
 	{
-		errno = EINVAL;
+		/*
+		 * If the buffer is not sufficient, we must return a different errno
+		 * than EINVAL, because callers will take EINVAL to mean it was not a
+		 * symlink.  The correct POSIX behavior would be to fill the buffer up
+		 * to the size, but we can't easily do that here.
+		 */
+		if (GetLastError() == ERROR_INSUFFICIENT_BUFFER)
+			errno = ENAMETOOLONG;
+		else
+			errno = EINVAL;
+
 		return -1;
 	}
 
diff --git a/src/port/win32stat.c b/src/port/win32stat.c
index 563696a5e1d..da2aa735101 100644
--- a/src/port/win32stat.c
+++ b/src/port/win32stat.c
@@ -176,6 +176,11 @@ _pglstat64(const char *name, struct stat *buf)
 				ret = -1;
 			}
 		}
+		else if (size >= sizeof(next))
+		{
+			errno = ENAMETOOLONG;
+			ret = -1;
+		}
 		else
 		{
 			/* It's a junction point, so report it as a symlink. */
@@ -234,7 +239,7 @@ _pgstat64(const char *name, struct stat *buf)
 			}
 			return -1;
 		}
-		if (size >= sizeof(next))
+		else if (size >= sizeof(next))
 		{
 			errno = ENAMETOOLONG;
 			return -1;
-- 
2.54.0