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 <hlinnaka@iki.fi>
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 b431a921e4bb7897392832054ada7c8fc5991639..a9d0ab312c8919d86aee04856c6877535371adac 100644 (file)
--- 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 5214e7b99c7ebb8e183c51b8cf9fc6934da1139f..fe5ce23aaba89482393bba35240d0a3384d7436f 100644 (file)
--- 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 99978bfbdbb6e98b2d89e4a0812f49a61b81ae11..46aaf4c0de4f0b92db9d8c6044b8bff6cd05f198 100644 (file)
--- 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 f7050df48412172c1b0722abc323b767cab584cb..c7e8ff6d40ba75304fa351cdf8362f75e1327499 100644 (file)
--- 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 a0a58c4a1a83c451ff6ff8c915319a139ec0a78e..2374493307386363c8619b08c9a4a4f1a1d282ef 100644 (file)
--- 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 acd8fa758fda2bf1ec815b0a37dd036118575e8f..6a3562d8bde724ad89831d7e7a814dab9419cdfe 100644 (file)
--- 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 074a8c7e5f17864ad1e91676d8bd3da7fb10004b..172acf7d02f04deb5be3199cc3d5a39c32343c72 100644 (file)
--- 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 2bad3688e913745d221238c19eff499afe3c8ed9..aff6eaa28f6c653ba7e67520bc73a1c59e81aa25 100644 (file)
--- 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 f1f33023a3be0589f7e6538071ecfd0b62536e5d..30725ab6aeee642451c1b592e34e6c676ed764c5 100644 (file)
--- 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 563696a5e1d90e1e3a098297b91098a64b31e87f..da2aa735101096cb5384da56e7623f8d75df2cea 100644 (file)
--- 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;