From: Peter Eisentraut Date: Mon, 13 Jul 2026 09:01:36 +0000 (+0200) Subject: Clean up readlink() return type X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=302222eddc3967d11c11708a59bdafd6d51da556;p=thirdparty%2Fpostgresql.git 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.) Reviewed-by: Heikki Linnakangas Discussion: https://www.postgresql.org/message-id/flat/f9aab072-0078-49e4-ab93-3b08086a4406@eisentraut.org --- diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index b431a921e4b..a9d0ab312c8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -9667,7 +9667,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 5214e7b99c7..fe5ce23aaba 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -1410,7 +1410,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 f7050df4841..c7e8ff6d40b 100644 --- a/src/bin/initdb/findtimezone.c +++ b/src/bin/initdb/findtimezone.c @@ -533,7 +533,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 a0a58c4a1a8..23744933073 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -1229,7 +1229,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; @@ -1270,6 +1269,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;