-
-
Notifications
You must be signed in to change notification settings - Fork 34.7k
bpo-20104: Expose posix_spawn in the os module
#5109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f138f0a
bd39bce
e8ca1d4
7ea83ed
90ed199
6ba804d
88831ec
cb440d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Expose posix_spawn as a low level API in the os module. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,6 +176,7 @@ corresponding Unix manual entries for more information on calls."); | |
| #else | ||
| /* Unix functions that the configure script doesn't check for */ | ||
| #define HAVE_EXECV 1 | ||
| #define HAVE_POSIX_SPAWN 1 | ||
| #define HAVE_FORK 1 | ||
| #if defined(__USLC__) && defined(__SCO_VERSION__) /* SCO UDK Compiler */ | ||
| #define HAVE_FORK1 1 | ||
|
|
@@ -246,6 +247,10 @@ extern int lstat(const char *, struct stat *); | |
|
|
||
| #endif /* !_MSC_VER */ | ||
|
|
||
| #ifdef HAVE_POSIX_SPAWN | ||
| #include <spawn.h> | ||
| #endif | ||
|
|
||
| #ifdef HAVE_UTIME_H | ||
| #include <utime.h> | ||
| #endif /* HAVE_UTIME_H */ | ||
|
|
@@ -5097,6 +5102,194 @@ os_execve_impl(PyObject *module, path_t *path, PyObject *argv, PyObject *env) | |
|
|
||
| #endif /* HAVE_EXECV */ | ||
|
|
||
| #ifdef HAVE_POSIX_SPAWN | ||
|
|
||
| enum posix_spawn_file_actions_identifier { | ||
| POSIX_SPAWN_OPEN, | ||
| POSIX_SPAWN_CLOSE, | ||
| POSIX_SPAWN_DUP2 | ||
| }; | ||
|
|
||
| /*[clinic input] | ||
|
|
||
| os.posix_spawn | ||
| path: path_t | ||
| Path of executable file. | ||
| argv: object | ||
| Tuple or list of strings. | ||
| env: object | ||
| Dictionary of strings mapping to strings. | ||
| file_actions: object = None | ||
| FileActions object. | ||
| / | ||
|
|
||
| Execute the program specified by path in a new process. | ||
| [clinic start generated code]*/ | ||
|
|
||
| static PyObject * | ||
| os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv, | ||
| PyObject *env, PyObject *file_actions) | ||
| /*[clinic end generated code: output=d023521f541c709c input=0ec9f1cfdc890be5]*/ | ||
| { | ||
| EXECV_CHAR **argvlist = NULL; | ||
| EXECV_CHAR **envlist; | ||
| Py_ssize_t argc, envc; | ||
|
|
||
| /* posix_spawn has three arguments: (path, argv, env), where | ||
| argv is a list or tuple of strings and env is a dictionary | ||
| like posix.environ. */ | ||
|
|
||
| if (!PySequence_Check(argv)){ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not |
||
| PyErr_SetString(PyExc_TypeError, | ||
| "posix_spawn: argv must be a tuple or list"); | ||
| goto fail; | ||
| } | ||
| argc = PySequence_Size(argv); | ||
| if (argc < 1) { | ||
| PyErr_SetString(PyExc_ValueError, "posix_spawn: argv must not be empty"); | ||
| return NULL; | ||
| } | ||
|
|
||
| if (!PyMapping_Check(env)) { | ||
| PyErr_SetString(PyExc_TypeError, | ||
| "posix_spawn: environment must be a mapping object"); | ||
| goto fail; | ||
| } | ||
|
|
||
| argvlist = parse_arglist(argv, &argc); | ||
| if (argvlist == NULL) { | ||
| goto fail; | ||
| } | ||
| if (!argvlist[0][0]) { | ||
| PyErr_SetString(PyExc_ValueError, | ||
| "posix_spawn: argv first element cannot be empty"); | ||
| goto fail; | ||
| } | ||
|
|
||
| envlist = parse_envlist(env, &envc); | ||
| if (envlist == NULL) | ||
| goto fail; | ||
|
|
||
| pid_t pid; | ||
| posix_spawn_file_actions_t *file_actionsp = NULL; | ||
| if (file_actions != NULL && file_actions != Py_None){ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| posix_spawn_file_actions_t _file_actions; | ||
| if(posix_spawn_file_actions_init(&_file_actions) != 0){ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't conform PEP 7. Needed spaces after "if" and before "{". |
||
| PyErr_SetString(PyExc_TypeError, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't |
||
| "Error initializing file actions"); | ||
| goto fail; | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too much empty lines. |
||
|
|
||
| file_actionsp = &_file_actions; | ||
|
|
||
|
|
||
| PyObject* seq = PySequence_Fast(file_actions, "file_actions must be a sequence"); | ||
| if(seq == NULL){ | ||
| goto fail; | ||
| } | ||
| PyObject* file_actions_obj; | ||
| PyObject* mode_obj; | ||
|
|
||
| for (int i = 0; i < PySequence_Fast_GET_SIZE(seq); ++i) { | ||
| file_actions_obj = PySequence_Fast_GET_ITEM(seq, i); | ||
|
|
||
| if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)){ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Use |
||
| PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence"); | ||
| goto fail; | ||
| } | ||
|
|
||
|
|
||
| mode_obj = PySequence_Fast_GET_ITEM(file_actions_obj, 0); | ||
| int mode = PyLong_AsLong(mode_obj); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be checked for error. |
||
|
|
||
| /* Populate the file_actions object */ | ||
|
|
||
| switch(mode) { | ||
|
|
||
| case POSIX_SPAWN_OPEN: | ||
| if(PySequence_Size(file_actions_obj) != 5){ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to check for errors in all of these cases: The PyXXX_AsYYY APIs can cause TypeError or OverflowError.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in cb440d5 |
||
| PyErr_SetString(PyExc_TypeError,"A open file_action object must have 5 elements"); | ||
| goto fail; | ||
| } | ||
|
|
||
| long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I would require
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if(PyErr_Occurred()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use idiomatic code: if (open_fd == -1 && PyErr_Occurred()) { |
||
| goto fail; | ||
| } | ||
| const char* open_path = PyUnicode_AsUTF8(PySequence_GetItem(file_actions_obj, 2)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use filesystem encoding. |
||
| if(open_path == NULL){ | ||
| goto fail; | ||
| } | ||
| long open_oflag = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 3)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
| if(PyErr_Occurred()) { | ||
| goto fail; | ||
| } | ||
| long open_mode = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 4)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check integer overflow when cast to |
||
| if(PyErr_Occurred()) { | ||
| goto fail; | ||
| } | ||
| posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode); | ||
| break; | ||
|
|
||
| case POSIX_SPAWN_CLOSE: | ||
| if(PySequence_Size(file_actions_obj) != 2){ | ||
| PyErr_SetString(PyExc_TypeError,"A close file_action object must have 2 elements"); | ||
| goto fail; | ||
| } | ||
|
|
||
| long close_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
| if(PyErr_Occurred()) { | ||
| goto fail; | ||
| } | ||
| posix_spawn_file_actions_addclose(file_actionsp, close_fd); | ||
| break; | ||
|
|
||
| case POSIX_SPAWN_DUP2: | ||
| if(PySequence_Size(file_actions_obj) != 3){ | ||
| PyErr_SetString(PyExc_TypeError,"A dup2 file_action object must have 3 elements"); | ||
| goto fail; | ||
| } | ||
|
|
||
| long fd1 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
| if(PyErr_Occurred()) { | ||
| goto fail; | ||
| } | ||
| long fd2 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 2)); | ||
| if(PyErr_Occurred()) { | ||
| goto fail; | ||
| } | ||
| posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2); | ||
| break; | ||
|
|
||
| default: | ||
| PyErr_SetString(PyExc_TypeError,"Unknown file_actions identifier"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't ValueError more appropriate exception type? |
||
| goto fail; | ||
| } | ||
| } | ||
| Py_DECREF(seq); | ||
| } | ||
|
|
||
| _Py_BEGIN_SUPPRESS_IPH | ||
| posix_spawn(&pid, path->narrow, file_actionsp, NULL, argvlist, envlist); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The result of the call is not checked. |
||
| return PyLong_FromPid(pid); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return before |
||
| _Py_END_SUPPRESS_IPH | ||
|
|
||
| path_error(path); | ||
|
|
||
| free_string_array(envlist, envc); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it leaked in error case? |
||
|
|
||
| fail: | ||
|
|
||
| if (argvlist) { | ||
| free_string_array(argvlist, argc); | ||
| } | ||
| return NULL; | ||
|
|
||
|
|
||
| } | ||
| #endif /* HAVE_POSIX_SPAWN */ | ||
|
|
||
|
|
||
| #if defined(HAVE_SPAWNV) || defined(HAVE_WSPAWNV) | ||
| /*[clinic input] | ||
|
|
@@ -5189,7 +5382,6 @@ os_spawnv_impl(PyObject *module, int mode, path_t *path, PyObject *argv) | |
| return Py_BuildValue(_Py_PARSE_INTPTR, spawnval); | ||
| } | ||
|
|
||
|
|
||
| /*[clinic input] | ||
| os.spawnve | ||
|
|
||
|
|
@@ -12610,6 +12802,7 @@ static PyMethodDef posix_methods[] = { | |
| OS_NICE_METHODDEF | ||
| OS_GETPRIORITY_METHODDEF | ||
| OS_SETPRIORITY_METHODDEF | ||
| OS_POSIX_SPAWN_METHODDEF | ||
| #ifdef HAVE_READLINK | ||
| {"readlink", (PyCFunction)posix_readlink, | ||
| METH_VARARGS | METH_KEYWORDS, | ||
|
|
@@ -13164,6 +13357,13 @@ all_ins(PyObject *m) | |
| if (PyModule_AddIntConstant(m, "RWF_NOWAIT", RWF_NOWAIT)) return -1; | ||
| #endif | ||
|
|
||
| /* constants for posix_spawn */ | ||
| #ifdef HAVE_POSIX_SPAWN | ||
| if (PyModule_AddIntConstant(m, "POSIX_SPAWN_OPEN", POSIX_SPAWN_OPEN)) return -1; | ||
| if (PyModule_AddIntConstant(m, "POSIX_SPAWN_CLOSE", POSIX_SPAWN_CLOSE)) return -1; | ||
| if (PyModule_AddIntConstant(m, "POSIX_SPAWN_DUP2", POSIX_SPAWN_DUP2)) return -1; | ||
| #endif | ||
|
|
||
| #ifdef HAVE_SPAWNV | ||
| if (PyModule_AddIntConstant(m, "P_WAIT", _P_WAIT)) return -1; | ||
| if (PyModule_AddIntConstant(m, "P_NOWAIT", _P_NOWAIT)) return -1; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad indentation.