Improve error reporting from validate_exec().

validate_exec() didn't guarantee to set errno to something appropriate
after a failure, leading to callers not being able to print an on-point
message.  Improve that.

Noted by Kyotaro Horiguchi, though this isn't exactly his proposal.

Discussion: https://postgr.es/m/20220615.131403.1791191615590453058.horikyota.ntt@gmail.com

diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 64276223d5086e3bef2b74a92129af10de15c751..aa3d2f88ed840bf4052d37c30800dddc3651ad21 100644 (file)
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -423,18 +423,11 @@ check_exec(const char *dir, const char *program, bool check_version)
        char            line[MAXPGPATH];
        char            cmd[MAXPGPATH];
        char            versionstr[128];
-       int                     ret;
 
        snprintf(path, sizeof(path), "%s/%s", dir, program);
 
-       ret = validate_exec(path);
-
-       if (ret == -1)
-               pg_fatal("check for \"%s\" failed: does not exist or cannot be executed",
-                                path);
-       else if (ret == -2)
-               pg_fatal("check for \"%s\" failed: cannot read (permission denied)",
-                                path);
+       if (validate_exec(path) != 0)
+               pg_fatal("check for \"%s\" failed: %m", path);
 
        snprintf(cmd, sizeof(cmd), "\"%s\" -V", path);
 

diff --git a/src/common/exec.c b/src/common/exec.c
index 9da588daf9104f0e0f495e7591e4c16c0136b193..f7d44b09569caeb7b2c71088312ea97a117dd8ae 100644 (file)
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -74,6 +74,7 @@ static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser);
  * returns 0 if the file is found and no error is encountered.
  *               -1 if the regular file "path" does not exist or cannot be executed.
  *               -2 if the file is otherwise valid but cannot be read.
+ * in the failure cases, errno is set appropriately
  */
 int
 validate_exec(const char *path)
@@ -105,7 +106,16 @@ validate_exec(const char *path)
                return -1;
 
        if (!S_ISREG(buf.st_mode))
+       {
+               /*
+                * POSIX offers no errno code that's simply "not a regular file".  If
+                * it's a directory we can use EISDIR.  Otherwise, it's most likely a
+                * device special file, and EPERM (Operation not permitted) isn't too
+                * horribly off base.
+                */
+               errno = S_ISDIR(buf.st_mode) ? EISDIR : EPERM;
                return -1;
+       }
 
        /*
         * Ensure that the file is both executable and readable (required for
@@ -114,9 +124,11 @@ validate_exec(const char *path)
 #ifndef WIN32
        is_r = (access(path, R_OK) == 0);
        is_x = (access(path, X_OK) == 0);
+       /* access() will set errno if it returns -1 */
 #else
        is_r = buf.st_mode & S_IRUSR;
        is_x = buf.st_mode & S_IXUSR;
+       errno = EACCES;                         /* appropriate thing if we return nonzero */
 #endif
        return is_x ? (is_r ? 0 : -2) : -1;
 }
@@ -165,7 +177,7 @@ find_my_exec(const char *argv0, char *retpath)
                        return resolve_symlinks(retpath);
 
                log_error(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                 _("invalid binary \"%s\""), retpath);
+                                 _("invalid binary \"%s\": %m"), retpath);
                return -1;
        }
 
@@ -215,7 +227,7 @@ find_my_exec(const char *argv0, char *retpath)
                                        break;
                                case -2:                /* found but disqualified */
                                        log_error(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                                         _("could not read binary \"%s\""),
+                                                         _("could not read binary \"%s\": %m"),
                                                          retpath);
                                        break;
                        }