bpo-38234: read_pth_file() now returns PyStatus (GH-16338)

Refactor path configuration code:

* read_pth_file() now returns PyStatus to report errors, rather than
  calling Py_FatalError().
* Move argv0_path and zip_path buffers out of PyCalculatePath
  structures.
* On Windows, _PyPathConfig.home is now preferred over PyConfig.home.

1 files changed, 131 insertions, 76 deletions

diff --git a/PC/getpathp.c b/PC/getpathp.c
index 5670fa2..c4c0636 100644
--- a/PC/getpathp.c
+++ b/PC/getpathp.c
@@ -115,20 +115,21 @@
  */
 
 #ifndef LANDMARK
-#define LANDMARK L"lib\\os.py"
+#  define LANDMARK L"lib\\os.py"
 #endif
 
+#define INIT_ERR_BUFFER_OVERFLOW() _PyStatus_ERR("buffer overflow")
+
+
 typedef struct {
     const wchar_t *path_env;           /* PATH environment variable */
     const wchar_t *home;               /* PYTHONHOME environment variable */
 
-    /* Registry key "Software\Python\PythonCore\PythonPath" */
+    /* Registry key "Software\Python\PythonCore\X.Y\PythonPath"
+       where X.Y is the Python version (major.minor) */
     wchar_t *machine_path;   /* from HKEY_LOCAL_MACHINE */
     wchar_t *user_path;      /* from HKEY_CURRENT_USER */
 
-    wchar_t argv0_path[MAXPATHLEN+1];
-    wchar_t zip_path[MAXPATHLEN+1];
-
     wchar_t *dll_path;
 
     const wchar_t *pythonpath_env;
@@ -276,7 +277,10 @@ typedef HRESULT(__stdcall *PPathCchCanonicalizeEx) (PWSTR pszPathOut, size_t cch
     PCWSTR pszPathIn, unsigned long dwFlags);
 static PPathCchCanonicalizeEx _PathCchCanonicalizeEx;
 
-static PyStatus canonicalize(wchar_t *buffer, const wchar_t *path)
+/* Call PathCchCanonicalizeEx(path): remove navigation elements such as "."
+   and ".." to produce a direct, well-formed path. */
+static PyStatus
+canonicalize(wchar_t *buffer, const wchar_t *path)
 {
     if (buffer == NULL) {
         return _PyStatus_NO_MEMORY();
@@ -295,12 +299,12 @@ static PyStatus canonicalize(wchar_t *buffer, const wchar_t *path)
 
     if (_PathCchCanonicalizeEx) {
         if (FAILED(_PathCchCanonicalizeEx(buffer, MAXPATHLEN + 1, path, 0))) {
-            return _PyStatus_ERR("buffer overflow in getpathp.c's canonicalize()");
+            return INIT_ERR_BUFFER_OVERFLOW();
         }
     }
     else {
         if (!PathCanonicalizeW(buffer, path)) {
-            return _PyStatus_ERR("buffer overflow in getpathp.c's canonicalize()");
+            return INIT_ERR_BUFFER_OVERFLOW();
         }
     }
     return _PyStatus_OK();
@@ -588,12 +592,18 @@ get_program_full_path(_PyPathConfig *pathconfig)
 }
 
 
-static int
-read_pth_file(_PyPathConfig *pathconfig, wchar_t *prefix, const wchar_t *path)
+static PyStatus
+read_pth_file(_PyPathConfig *pathconfig, wchar_t *prefix, const wchar_t *path,
+              int *found)
 {
-    FILE *sp_file = _Py_wfopen(path, L"r");
+    PyStatus status;
+    wchar_t *buf = NULL;
+    wchar_t *wline = NULL;
+    FILE *sp_file;
+
+    sp_file = _Py_wfopen(path, L"r");
     if (sp_file == NULL) {
-        return 0;
+        return _PyStatus_OK();
     }
 
     wcscpy_s(prefix, MAXPATHLEN+1, path);
@@ -604,15 +614,16 @@ read_pth_file(_PyPathConfig *pathconfig, wchar_t *prefix, const wchar_t *path)
     size_t bufsiz = MAXPATHLEN;
     size_t prefixlen = wcslen(prefix);
 
-    wchar_t *buf = (wchar_t*)PyMem_RawMalloc(bufsiz * sizeof(wchar_t));
+    buf = (wchar_t*)PyMem_RawMalloc(bufsiz * sizeof(wchar_t));
     if (buf == NULL) {
-        goto error;
+        status = _PyStatus_NO_MEMORY();
+        goto done;
     }
     buf[0] = '\0';
 
     while (!feof(sp_file)) {
         char line[MAXPATHLEN + 1];
-        char *p = fgets(line, MAXPATHLEN + 1, sp_file);
+        char *p = fgets(line, Py_ARRAY_LENGTH(line), sp_file);
         if (!p) {
             break;
         }
@@ -631,13 +642,16 @@ read_pth_file(_PyPathConfig *pathconfig, wchar_t *prefix, const wchar_t *path)
             continue;
         }
         else if (strncmp(line, "import ", 7) == 0) {
-            Py_FatalError("only 'import site' is supported in ._pth file");
+            status = _PyStatus_ERR("only 'import site' is supported "
+                                   "in ._pth file");
+            goto done;
         }
 
         DWORD wn = MultiByteToWideChar(CP_UTF8, 0, line, -1, NULL, 0);
         wchar_t *wline = (wchar_t*)PyMem_RawMalloc((wn + 1) * sizeof(wchar_t));
         if (wline == NULL) {
-            goto error;
+            status = _PyStatus_NO_MEMORY();
+            goto done;
         }
         wn = MultiByteToWideChar(CP_UTF8, 0, line, -1, wline, wn + 1);
         wline[wn] = '\0';
@@ -648,8 +662,8 @@ read_pth_file(_PyPathConfig *pathconfig, wchar_t *prefix, const wchar_t *path)
             wchar_t *tmp = (wchar_t*)PyMem_RawRealloc(buf, (bufsiz + 1) *
                                                             sizeof(wchar_t));
             if (tmp == NULL) {
-                PyMem_RawFree(wline);
-                goto error;
+                status = _PyStatus_NO_MEMORY();
+                goto done;
             }
             buf = tmp;
         }
@@ -663,48 +677,39 @@ read_pth_file(_PyPathConfig *pathconfig, wchar_t *prefix, const wchar_t *path)
         _Py_BEGIN_SUPPRESS_IPH
         result = wcscat_s(buf, bufsiz, prefix);
         _Py_END_SUPPRESS_IPH
+
         if (result == EINVAL) {
-            Py_FatalError("invalid argument during ._pth processing");
+            status = _PyStatus_ERR("invalid argument during ._pth processing");
+            goto done;
         } else if (result == ERANGE) {
-            Py_FatalError("buffer overflow during ._pth processing");
+            status = _PyStatus_ERR("buffer overflow during ._pth processing");
+            goto done;
         }
+
         wchar_t *b = &buf[usedsiz];
         join(b, wline);
 
         PyMem_RawFree(wline);
+        wline = NULL;
     }
 
-    fclose(sp_file);
     if (pathconfig->module_search_path == NULL) {
         pathconfig->module_search_path = _PyMem_RawWcsdup(buf);
         if (pathconfig->module_search_path == NULL) {
-            Py_FatalError("out of memory");
+            status = _PyStatus_NO_MEMORY();
+            goto done;
         }
     }
-    PyMem_RawFree(buf);
-    return 1;
 
-error:
+    *found = 1;
+    status = _PyStatus_OK();
+    goto done;
+
+done:
     PyMem_RawFree(buf);
+    PyMem_RawFree(wline);
     fclose(sp_file);
-    return 0;
-}
-
-
-static PyStatus
-calculate_init(PyCalculatePath *calculate, const PyConfig *config)
-{
-    calculate->home = config->home;
-    calculate->path_env = _wgetenv(L"PATH");
-
-    calculate->dll_path = _Py_GetDLLPath();
-    if (calculate->dll_path == NULL) {
-        return _PyStatus_NO_MEMORY();
-    }
-
-    calculate->pythonpath_env = config->pythonpath_env;
-
-    return _PyStatus_OK();
+    return status;
 }
 
 
@@ -730,17 +735,17 @@ get_pth_filename(PyCalculatePath *calculate, wchar_t *filename,
 }
 
 
-static int
+static PyStatus
 calculate_pth_file(PyCalculatePath *calculate, _PyPathConfig *pathconfig,
-                   wchar_t *prefix)
+                   wchar_t *prefix, int *found)
 {
     wchar_t filename[MAXPATHLEN+1];
 
     if (!get_pth_filename(calculate, filename, pathconfig)) {
-        return 0;
+        return _PyStatus_OK();
     }
 
-    return read_pth_file(pathconfig, prefix, filename);
+    return read_pth_file(pathconfig, prefix, filename, found);
 }
 
 
@@ -749,12 +754,13 @@ calculate_pth_file(PyCalculatePath *calculate, _PyPathConfig *pathconfig,
    If found, open it for use when searching for prefixes.
 */
 static void
-calculate_pyvenv_file(PyCalculatePath *calculate)
+calculate_pyvenv_file(PyCalculatePath *calculate,
+                      wchar_t *argv0_path, size_t argv0_path_len)
 {
     wchar_t envbuffer[MAXPATHLEN+1];
     const wchar_t *env_cfg = L"pyvenv.cfg";
 
-    wcscpy_s(envbuffer, MAXPATHLEN+1, calculate->argv0_path);
+    wcscpy_s(envbuffer, MAXPATHLEN+1, argv0_path);
     join(envbuffer, env_cfg);
 
     FILE *env_file = _Py_wfopen(envbuffer, L"r");
@@ -778,25 +784,25 @@ calculate_pyvenv_file(PyCalculatePath *calculate)
     /* Look for a 'home' variable and set argv0_path to it, if found */
     wchar_t tmpbuffer[MAXPATHLEN+1];
     if (_Py_FindEnvConfigValue(env_file, L"home", tmpbuffer, MAXPATHLEN)) {
-        wcscpy_s(calculate->argv0_path, MAXPATHLEN+1, tmpbuffer);
+        wcscpy_s(argv0_path, argv0_path_len, tmpbuffer);
     }
     fclose(env_file);
 }
 
 
-#define INIT_ERR_BUFFER_OVERFLOW() _PyStatus_ERR("buffer overflow")
-
-
 static void
-calculate_home_prefix(PyCalculatePath *calculate, wchar_t *prefix)
+calculate_home_prefix(PyCalculatePath *calculate,
+                      const wchar_t *argv0_path,
+                      const wchar_t *zip_path,
+                      wchar_t *prefix)
 {
     if (calculate->home == NULL || *calculate->home == '\0') {
-        if (calculate->zip_path[0] && exists(calculate->zip_path)) {
-            wcscpy_s(prefix, MAXPATHLEN+1, calculate->zip_path);
+        if (zip_path[0] && exists(zip_path)) {
+            wcscpy_s(prefix, MAXPATHLEN+1, zip_path);
             reduce(prefix);
             calculate->home = prefix;
         }
-        else if (search_for_prefix(prefix, calculate->argv0_path, LANDMARK)) {
+        else if (search_for_prefix(prefix, argv0_path, LANDMARK)) {
             calculate->home = prefix;
         }
         else {
@@ -812,7 +818,9 @@ calculate_home_prefix(PyCalculatePath *calculate, wchar_t *prefix)
 static PyStatus
 calculate_module_search_path(PyCalculatePath *calculate,
                              _PyPathConfig *pathconfig,
-                             wchar_t *prefix)
+                             const wchar_t *argv0_path,
+                             wchar_t *prefix,
+                             const wchar_t *zip_path)
 {
     int skiphome = calculate->home==NULL ? 0 : 1;
 #ifdef Py_ENABLE_SHARED
@@ -852,14 +860,14 @@ calculate_module_search_path(PyCalculatePath *calculate,
         bufsz *= wcslen(calculate->home);
     }
     bufsz += wcslen(PYTHONPATH) + 1;
-    bufsz += wcslen(calculate->argv0_path) + 1;
+    bufsz += wcslen(argv0_path) + 1;
     if (calculate->user_path) {
         bufsz += wcslen(calculate->user_path) + 1;
     }
     if (calculate->machine_path) {
         bufsz += wcslen(calculate->machine_path) + 1;
     }
-    bufsz += wcslen(calculate->zip_path) + 1;
+    bufsz += wcslen(zip_path) + 1;
     if (calculate->pythonpath_env != NULL) {
         bufsz += wcslen(calculate->pythonpath_env) + 1;
     }
@@ -867,7 +875,7 @@ calculate_module_search_path(PyCalculatePath *calculate,
     wchar_t *buf, *start_buf;
     buf = PyMem_RawMalloc(bufsz * sizeof(wchar_t));
     if (buf == NULL) {
-        Py_FatalError("Can't malloc dynamic PYTHONPATH");
+        return _PyStatus_NO_MEMORY();
     }
     start_buf = buf;
 
@@ -879,8 +887,8 @@ calculate_module_search_path(PyCalculatePath *calculate,
         buf = wcschr(buf, L'\0');
         *buf++ = DELIM;
     }
-    if (calculate->zip_path[0]) {
-        if (wcscpy_s(buf, bufsz - (buf - start_buf), calculate->zip_path)) {
+    if (zip_path[0]) {
+        if (wcscpy_s(buf, bufsz - (buf - start_buf), zip_path)) {
             return INIT_ERR_BUFFER_OVERFLOW();
         }
         buf = wcschr(buf, L'\0');
@@ -937,8 +945,8 @@ calculate_module_search_path(PyCalculatePath *calculate,
             p = q+1;
         }
     }
-    if (calculate->argv0_path) {
-        wcscpy(buf, calculate->argv0_path);
+    if (argv0_path) {
+        wcscpy(buf, argv0_path);
         buf = wcschr(buf, L'\0');
         *buf++ = DELIM;
     }
@@ -996,28 +1004,40 @@ calculate_path(PyCalculatePath *calculate, _PyPathConfig *pathconfig)
     }
 
     /* program_full_path guaranteed \0 terminated in MAXPATH+1 bytes. */
-    wcscpy_s(calculate->argv0_path, MAXPATHLEN+1, pathconfig->program_full_path);
-    reduce(calculate->argv0_path);
+    wchar_t argv0_path[MAXPATHLEN+1];
+    memset(argv0_path, 0, sizeof(argv0_path));
+
+    wcscpy_s(argv0_path, MAXPATHLEN+1, pathconfig->program_full_path);
+    reduce(argv0_path);
 
     wchar_t prefix[MAXPATHLEN+1];
     memset(prefix, 0, sizeof(prefix));
 
     /* Search for a sys.path file */
-    if (calculate_pth_file(calculate, pathconfig, prefix)) {
+    int pth_found = 0;
+    status = calculate_pth_file(calculate, pathconfig, prefix, &pth_found);
+    if (_PyStatus_EXCEPTION(status)) {
+        return status;
+    }
+    if (pth_found) {
         goto done;
     }
 
-    calculate_pyvenv_file(calculate);
+    calculate_pyvenv_file(calculate, argv0_path, Py_ARRAY_LENGTH(argv0_path));
 
     /* Calculate zip archive path from DLL or exe path */
-    change_ext(calculate->zip_path,
+    wchar_t zip_path[MAXPATHLEN+1];
+    memset(zip_path, 0, sizeof(zip_path));
+
+    change_ext(zip_path,
                calculate->dll_path[0] ? calculate->dll_path : pathconfig->program_full_path,
                L".zip");
 
-    calculate_home_prefix(calculate, prefix);
+    calculate_home_prefix(calculate, argv0_path, zip_path, prefix);
 
     if (pathconfig->module_search_path == NULL) {
-        status = calculate_module_search_path(calculate, pathconfig, prefix);
+        status = calculate_module_search_path(calculate, pathconfig,
+                                              argv0_path, prefix, zip_path);
         if (_PyStatus_EXCEPTION(status)) {
             return status;
         }
@@ -1041,6 +1061,24 @@ done:
 }
 
 
+static PyStatus
+calculate_init(PyCalculatePath *calculate, _PyPathConfig *pathconfig,
+               const PyConfig *config)
+{
+    calculate->home = pathconfig->home;
+    calculate->path_env = _wgetenv(L"PATH");
+
+    calculate->dll_path = _Py_GetDLLPath();
+    if (calculate->dll_path == NULL) {
+        return _PyStatus_NO_MEMORY();
+    }
+
+    calculate->pythonpath_env = config->pythonpath_env;
+
+    return _PyStatus_OK();
+}
+
+
 static void
 calculate_free(PyCalculatePath *calculate)
 {
@@ -1050,7 +1088,24 @@ calculate_free(PyCalculatePath *calculate)
 }
 
 
-/* Calculate 'pathconfig' attributes:
+/* Calculate the Python path configuration.
+
+   Inputs:
+
+   - PyConfig.pythonpath_env: PYTHONPATH environment variable
+   - _PyPathConfig.home: Py_SetPythonHome() or PYTHONHOME environment variable
+   - DLL path: _Py_GetDLLPath()
+   - PATH environment variable
+   - __PYVENV_LAUNCHER__ environment variable
+   - GetModuleFileNameW(NULL): fully qualified path of the executable file of
+     the current process
+   - .pth configuration file
+   - pyvenv.cfg configuration file
+   - Registry key "Software\Python\PythonCore\X.Y\PythonPath"
+     of HKEY_LOCAL_MACHINE and HKEY_CURRENT_USER where X.Y is the Python
+     version (major.minor).
+
+   Outputs, 'pathconfig' fields:
 
    - base_executable
    - program_full_path
@@ -1060,7 +1115,7 @@ calculate_free(PyCalculatePath *calculate)
    - isolated
    - site_import
 
-   If an attribute is already set (non NULL), it is left unchanged. */
+   If a field is already set (non NULL), it is left unchanged. */
 PyStatus
 _PyPathConfig_Calculate(_PyPathConfig *pathconfig, const PyConfig *config)
 {
@@ -1068,7 +1123,7 @@ _PyPathConfig_Calculate(_PyPathConfig *pathconfig, const PyConfig *config)
     PyCalculatePath calculate;
     memset(&calculate, 0, sizeof(calculate));
 
-    status = calculate_init(&calculate, config);
+    status = calculate_init(&calculate, pathconfig, config);
     if (_PyStatus_EXCEPTION(status)) {
         goto done;
     }