Mailing List Archive

https://github.com/python/cpython/commit/23950beff84c39d50f48011e930f4c6ebf32fc73
commit: 23950beff84c39d50f48011e930f4c6ebf32fc73
branch: main
author: Eric Snow <ericsnowcurrently@gmail.com>
committer: ericsnowcurrently <ericsnowcurrently@gmail.com>
date: 2024-04-23T08:25:50-06:00
summary:

gh-117953: Small Cleanup of Extensions-Related Machinery Code (gh-118167)

This is a collection of very basic cleanups I've pulled out of gh-118116. It is mostly renaming variables and moving a couple bits of code in functionally equivalent ways.

files:
M Include/internal/pycore_import.h
M Python/import.c
M Python/importdl.c

diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h
index eb8a9a0db46c22..08af53258cde97 100644
--- a/Include/internal/pycore_import.h
+++ b/Include/internal/pycore_import.h
@@ -22,6 +22,7 @@ extern int _PyImport_SetModuleString(const char *name, PyObject* module);
extern void _PyImport_AcquireLock(PyInterpreterState *interp);
extern int _PyImport_ReleaseLock(PyInterpreterState *interp);

+// This is used exclusively for the sys and builtins modules:
extern int _PyImport_FixupBuiltin(
PyObject *mod,
const char *name, /* UTF-8 encoded string */
diff --git a/Python/import.c b/Python/import.c
index b040c7d5c0f7f5..8cdc04f03dd201 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -1125,10 +1125,10 @@ _PyImport_CheckSubinterpIncompatibleExtensionAllowed(const char *name)

static PyObject *
get_core_module_dict(PyInterpreterState *interp,
- PyObject *name, PyObject *filename)
+ PyObject *name, PyObject *path)
{
/* Only builtin modules are core. */
- if (filename == name) {
+ if (path == name) {
assert(!PyErr_Occurred());
if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) {
return interp->sysdict_copy;
@@ -1143,11 +1143,11 @@ get_core_module_dict(PyInterpreterState *interp,
}

static inline int
-is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename)
+is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *path)
{
/* This might be called before the core dict copies are in place,
so we can't rely on get_core_module_dict() here. */
- if (filename == name) {
+ if (path == name) {
if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) {
return 1;
}
@@ -1159,7 +1159,7 @@ is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename)
}

static int
-fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename)
+fix_up_extension(PyObject *mod, PyObject *name, PyObject *path)
{
if (mod == NULL || !PyModule_Check(mod)) {
PyErr_BadInternalCall();
@@ -1180,7 +1180,7 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename)
// bpo-44050: Extensions and def->m_base.m_copy can be updated
// when the extension module doesn't support sub-interpreters.
if (def->m_size == -1) {
- if (!is_core_module(tstate->interp, name, filename)) {
+ if (!is_core_module(tstate->interp, name, path)) {
assert(PyUnicode_CompareWithASCIIString(name, "sys") != 0);
assert(PyUnicode_CompareWithASCIIString(name, "builtins") != 0);
if (def->m_base.m_copy) {
@@ -1202,7 +1202,7 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename)

// XXX Why special-case the main interpreter?
if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) {
- if (_extensions_cache_set(filename, name, def) < 0) {
+ if (_extensions_cache_set(path, name, def) < 0) {
return -1;
}
}
@@ -1227,10 +1227,10 @@ _PyImport_FixupExtensionObject(PyObject *mod, PyObject *name,

static PyObject *
import_find_extension(PyThreadState *tstate, PyObject *name,
- PyObject *filename)
+ PyObject *path)
{
/* Only single-phase init modules will be in the cache. */
- PyModuleDef *def = _extensions_cache_get(filename, name);
+ PyModuleDef *def = _extensions_cache_get(path, name);
if (def == NULL) {
return NULL;
}
@@ -1253,7 +1253,7 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
if (m_copy == NULL) {
/* It might be a core module (e.g. sys & builtins),
for which we don't set m_copy. */
- m_copy = get_core_module_dict(tstate->interp, name, filename);
+ m_copy = get_core_module_dict(tstate->interp, name, path);
if (m_copy == NULL) {
return NULL;
}
@@ -1292,16 +1292,16 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
int verbose = _PyInterpreterState_GetConfig(tstate->interp)->verbose;
if (verbose) {
PySys_FormatStderr("import %U # previously loaded (%R)\n",
- name, filename);
+ name, path);
}
return mod;
}

static int
clear_singlephase_extension(PyInterpreterState *interp,
- PyObject *name, PyObject *filename)
+ PyObject *name, PyObject *path)
{
- PyModuleDef *def = _extensions_cache_get(filename, name);
+ PyModuleDef *def = _extensions_cache_get(path, name);
if (def == NULL) {
if (PyErr_Occurred()) {
return -1;
@@ -1322,7 +1322,7 @@ clear_singlephase_extension(PyInterpreterState *interp,
}

/* Clear the cached module def. */
- _extensions_cache_delete(filename, name);
+ _extensions_cache_delete(path, name);

return 0;
}
@@ -1336,11 +1336,20 @@ int
_PyImport_FixupBuiltin(PyObject *mod, const char *name, PyObject *modules)
{
int res = -1;
+ assert(mod != NULL && PyModule_Check(mod));
+
PyObject *nameobj;
nameobj = PyUnicode_InternFromString(name);
if (nameobj == NULL) {
return -1;
}
+
+ PyModuleDef *def = PyModule_GetDef(mod);
+ if (def == NULL) {
+ PyErr_BadInternalCall();
+ goto finally;
+ }
+
if (PyObject_SetItem(modules, nameobj, mod) < 0) {
goto finally;
}
@@ -1348,6 +1357,7 @@ _PyImport_FixupBuiltin(PyObject *mod, const char *name, PyObject *modules)
PyMapping_DelItem(modules, nameobj);
goto finally;
}
+
res = 0;

finally:
@@ -1382,39 +1392,45 @@ create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec)
}

PyObject *modules = MODULES(tstate->interp);
+ struct _inittab *found = NULL;
for (struct _inittab *p = INITTAB; p->name != NULL; p++) {
if (_PyUnicode_EqualToASCIIString(name, p->name)) {
- if (p->initfunc == NULL) {
- /* Cannot re-init internal module ("sys" or "builtins") */
- return import_add_module(tstate, name);
- }
- mod = (*p->initfunc)();
- if (mod == NULL) {
- return NULL;
- }
+ found = p;
+ }
+ }
+ if (found == NULL) {
+ // not found
+ Py_RETURN_NONE;
+ }

- if (PyObject_TypeCheck(mod, &PyModuleDef_Type)) {
- return PyModule_FromDefAndSpec((PyModuleDef*)mod, spec);
- }
- else {
- /* Remember pointer to module init function. */
- PyModuleDef *def = PyModule_GetDef(mod);
- if (def == NULL) {
- return NULL;
- }
+ PyModInitFunction p0 = (PyModInitFunction)found->initfunc;
+ if (p0 == NULL) {
+ /* Cannot re-init internal module ("sys" or "builtins") */
+ assert(is_core_module(tstate->interp, name, name));
+ return import_add_module(tstate, name);
+ }

- def->m_base.m_init = p->initfunc;
- if (_PyImport_FixupExtensionObject(mod, name, name,
- modules) < 0) {
- return NULL;
- }
- return mod;
- }
- }
+ mod = p0();
+ if (mod == NULL) {
+ return NULL;
}

- // not found
- Py_RETURN_NONE;
+ if (PyObject_TypeCheck(mod, &PyModuleDef_Type)) {
+ return PyModule_FromDefAndSpec((PyModuleDef*)mod, spec);
+ }
+ else {
+ /* Remember pointer to module init function. */
+ PyModuleDef *def = PyModule_GetDef(mod);
+ if (def == NULL) {
+ return NULL;
+ }
+
+ def->m_base.m_init = p0;
+ if (_PyImport_FixupExtensionObject(mod, name, name, modules) < 0) {
+ return NULL;
+ }
+ return mod;
+ }
}


@@ -3724,7 +3740,7 @@ static PyObject *
_imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
/*[clinic end generated code: output=83249b827a4fde77 input=c31b954f4cf4e09d]*/
{
- PyObject *mod, *name, *path;
+ PyObject *mod, *name, *filename;
FILE *fp;

name = PyObject_GetAttrString(spec, "name");
@@ -3732,36 +3748,56 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
return NULL;
}

- path = PyObject_GetAttrString(spec, "origin");
- if (path == NULL) {
+ filename = PyObject_GetAttrString(spec, "origin");
+ if (filename == NULL) {
Py_DECREF(name);
return NULL;
}

PyThreadState *tstate = _PyThreadState_GET();
- mod = import_find_extension(tstate, name, path);
+ mod = import_find_extension(tstate, name, filename);
if (mod != NULL || _PyErr_Occurred(tstate)) {
assert(mod == NULL || !_PyErr_Occurred(tstate));
goto finally;
}

+ if (PySys_Audit("import", "OOOOO", name, filename,
+ Py_None, Py_None, Py_None) < 0)
+ {
+ goto finally;
+ }
+
+ /* Is multi-phase init or this is the first time being loaded. */
+
+ /* We would move this (and the fclose() below) into
+ * _PyImport_GetModInitFunc(), but it isn't clear if the intervening
+ * code relies on fp still being open. */
if (file != NULL) {
- fp = _Py_fopen_obj(path, "r");
+ fp = _Py_fopen_obj(filename, "r");
if (fp == NULL) {
goto finally;
}
}
- else
+ else {
fp = NULL;
+ }

mod = _PyImport_LoadDynamicModuleWithSpec(spec, fp);
+ if (mod != NULL) {
+ /* Remember the filename as the __file__ attribute */
+ if (PyModule_AddObjectRef(mod, "__file__", filename) < 0) {
+ PyErr_Clear(); /* Not important enough to report */
+ }
+ }

- if (fp)
+ // XXX Shouldn't this happen in the error cases too.
+ if (fp) {
fclose(fp);
+ }

finally:
Py_DECREF(name);
- Py_DECREF(path);
+ Py_DECREF(filename);
return mod;
}

diff --git a/Python/importdl.c b/Python/importdl.c
index 7dfd301d77efb4..7cf30bea3a861a 100644
--- a/Python/importdl.c
+++ b/Python/importdl.c
@@ -97,9 +97,10 @@ PyObject *
_PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
{
#ifndef MS_WINDOWS
- PyObject *pathbytes = NULL;
+ PyObject *filename_bytes = NULL;
+ const char *filename_buf;
#endif
- PyObject *name_unicode = NULL, *name = NULL, *path = NULL, *m = NULL;
+ PyObject *name_unicode = NULL, *name = NULL, *filename = NULL, *m = NULL;
const char *name_buf, *hook_prefix;
const char *oldcontext, *newcontext;
dl_funcptr exportfunc;
@@ -126,26 +127,23 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
}
name_buf = PyBytes_AS_STRING(name);

- path = PyObject_GetAttrString(spec, "origin");
- if (path == NULL)
- goto error;
-
- if (PySys_Audit("import", "OOOOO", name_unicode, path,
- Py_None, Py_None, Py_None) < 0) {
+ filename = PyObject_GetAttrString(spec, "origin");
+ if (filename == NULL) {
goto error;
}

#ifdef MS_WINDOWS
- exportfunc = _PyImport_FindSharedFuncptrWindows(hook_prefix, name_buf,
- path, fp);
+ exportfunc = _PyImport_FindSharedFuncptrWindows(
+ hook_prefix, name_buf, filename, fp);
#else
- pathbytes = PyUnicode_EncodeFSDefault(path);
- if (pathbytes == NULL)
+ filename_bytes = PyUnicode_EncodeFSDefault(filename);
+ if (filename_bytes == NULL) {
goto error;
- exportfunc = _PyImport_FindSharedFuncptr(hook_prefix, name_buf,
- PyBytes_AS_STRING(pathbytes),
- fp);
- Py_DECREF(pathbytes);
+ }
+ filename_buf = PyBytes_AS_STRING(filename_bytes);
+ exportfunc = _PyImport_FindSharedFuncptr(
+ hook_prefix, name_buf, filename_buf, fp);
+ Py_DECREF(filename_bytes);
#endif

if (exportfunc == NULL) {
@@ -157,7 +155,7 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
hook_prefix, name_buf);
if (msg == NULL)
goto error;
- PyErr_SetImportError(msg, name_unicode, path);
+ PyErr_SetImportError(msg, name_unicode, filename);
Py_DECREF(msg);
}
goto error;
@@ -199,7 +197,7 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
if (PyObject_TypeCheck(m, &PyModuleDef_Type)) {
Py_DECREF(name_unicode);
Py_DECREF(name);
- Py_DECREF(path);
+ Py_DECREF(filename);
return PyModule_FromDefAndSpec((PyModuleDef*)m, spec);
}

@@ -228,25 +226,20 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp)
}
def->m_base.m_init = p0;

- /* Remember the filename as the __file__ attribute */
- if (PyModule_AddObjectRef(m, "__file__", path) < 0) {
- PyErr_Clear(); /* Not important enough to report */
- }
-
PyObject *modules = PyImport_GetModuleDict();
- if (_PyImport_FixupExtensionObject(m, name_unicode, path, modules) < 0)
+ if (_PyImport_FixupExtensionObject(m, name_unicode, filename, modules) < 0)
goto error;

Py_DECREF(name_unicode);
Py_DECREF(name);
- Py_DECREF(path);
+ Py_DECREF(filename);

return m;

error:
Py_DECREF(name_unicode);
Py_XDECREF(name);
- Py_XDECREF(path);
+ Py_XDECREF(filename);
Py_XDECREF(m);
return NULL;
}

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-leave@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: list-python-checkins@lists.gossamer-threads.com