Re: [PATCH] findprog-in: Set errno to indicate why NULL was returned.

[Bruno Haible <bruno@clisp.org>]

Hi Paul,

> Without this change it's impossible for a program to show the correct
> error message when the program is found on the path, but is not
> executable.

Good point!

I've committed a slightly different patch, based on yours. While your
patch was fully correct, I have different preferences:

  - You don't need a dependency to the 'errno' module, because
    (see doc/posix-headers/errno.texi) it is only needed for more
    "advanced" errno values. For the basic ones like ENOENT, EINVAL,
    EACCES, etc. no portability problems are known.

  - Since the errno value is a saved one in some cases, but not in
    all cases, I prefer a variable name 'failure_errno' to 'saved_errno'.

  - In the specification comment in findprog.h: I avoid explaining the
    algorithm that a certain function employs. Only: what are the inputs?
    what are the outputs? and only the minimal necessary information about
    the inner workings of the function.
    It must be possible to describe a function in an abstract way. If I
    was led to document the algorithm that a function uses, this would
    be a sign that the function is not well designed.


2019-09-15  Paul Smith  <psmith@gnu.org>
            Bruno Haible  <bruno@clisp.org>

	findprog-in: Set errno when the search fails.
	* lib/findprog-in.c: Include <errno.h>.
	(find_in_given_path): Set errno before returning NULL.
	* lib/findprog.h (find_in_given_path): Update comment accordingly.
	Define the term "slash".

diff --git a/lib/findprog-in.c b/lib/findprog-in.c
index d601e06..5e90680 100644
--- a/lib/findprog-in.c
+++ b/lib/findprog-in.c
@@ -21,6 +21,7 @@
 /* Specification.  */
 #include "findprog.h"
 
+#include <errno.h>
 #include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
@@ -98,6 +99,7 @@ find_in_given_path (const char *progname, const char *path,
           {
             /* Try the various suffixes and see whether one of the files
                with such a suffix is actually executable.  */
+            int failure_errno;
             size_t i;
             #if defined _WIN32 && !defined __CYGWIN__ /* Native Windows */
             const char *progbasename;
@@ -113,6 +115,7 @@ find_in_given_path (const char *progname, const char *path,
             #endif
 
             /* Try all platform-dependent suffixes.  */
+            failure_errno = ENOENT;
             for (i = 0; i < sizeof (suffixes) / sizeof (suffixes[0]); i++)
               {
                 const char *suffix = suffixes[i];
@@ -143,10 +146,14 @@ find_in_given_path (const char *progname, const char *path,
                           return progpathname;
                       }
 
+                    if (errno != ENOENT)
+                      failure_errno = errno;
+
                     free (progpathname);
                   }
               }
 
+            errno = failure_errno;
             return NULL;
           }
       }
@@ -158,11 +165,13 @@ find_in_given_path (const char *progname, const char *path,
     path = "";
 
   {
+    int failure_errno;
     /* Make a copy, to prepare for destructive modifications.  */
     char *path_copy = xstrdup (path);
     char *path_rest;
     char *cp;
 
+    failure_errno = ENOENT;
     for (path_rest = path_copy; ; path_rest = cp + 1)
       {
         const char *dir;
@@ -222,6 +231,9 @@ find_in_given_path (const char *progname, const char *path,
                     return progpathname;
                   }
 
+                if (errno != ENOENT)
+                  failure_errno = errno;
+
                 free (progpathname);
               }
           }
@@ -232,7 +244,8 @@ find_in_given_path (const char *progname, const char *path,
 
     /* Not found in PATH.  */
     free (path_copy);
-  }
 
-  return NULL;
+    errno = failure_errno;
+    return NULL;
+  }
 }
diff --git a/lib/findprog.h b/lib/findprog.h
index f7b4407..804f02a 100644
--- a/lib/findprog.h
+++ b/lib/findprog.h
@@ -36,19 +36,29 @@ extern "C" {
 extern const char *find_in_path (const char *progname);
 
 /* Looks up a program in the given PATH-like string.
+
    The PATH argument consists of a list of directories, separated by ':' or
    (on native Windows) by ';'.  An empty PATH element designates the current
    directory.  A null PATH is equivalent to an empty PATH, that is, to the
    singleton list that contains only the current directory.
+
    Determines the pathname that would be called by execlp/execvp of PROGNAME.
    - If successful, it returns a pathname containing a slash (either absolute
      or relative to the current directory).  The returned string can be used
      with either execl/execv or execlp/execvp.  It is freshly malloc()ed if it
      is != PROGNAME.
-   - Otherwise, it returns NULL.
+   - Otherwise, it sets errno and returns NULL.
+     Specific errno values include:
+       - ENOENT: means that the program's file was not found.
+       - EACCESS: means that the program's file was found but lacks the
+         execute permissions.
    If OPTIMIZE_FOR_EXEC is true, the function saves some work, under the
    assumption that the resulting pathname will not be accessed directly,
-   only through execl/execv or execlp/execvp.  */
+   only through execl/execv or execlp/execvp.
+
+   Here, a "slash" means:
+     - On POSIX systems excluding Cygwin: a '/',
+     - On Windows, OS/2, DOS platforms: a '/' or '\'. */
 extern const char *find_in_given_path (const char *progname, const char *path,
                                        bool optimize_for_exec);