From e1ab328489c50f94174694beed4782bca87cbba7 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut 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