Skip to content

Commit b5a5827

Browse files
pablogsalpablogsal
committed
Fix refleaks, style and other issues for os.posix_spawn
1 parent 8534d53 commit b5a5827

3 files changed

Lines changed: 54 additions & 51 deletions

File tree

‎Lib/test/test_posix.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,16 +179,17 @@ def test_fexecve(self):
179179

180180
@unittest.skipUnless(hasattr(os, 'posix_spawn'), "test needs os.posix_spawn")
181181
def test_posix_spawn(self):
182-
pid = posix.posix_spawn(sys.executable, [sys.executable, "-c", "pass"], os.environ,[])
182+
pid = posix.posix_spawn(sys.executable, [sys.executable, "-c", "pass"], os.environ,tuple())
183183
self.assertEqual(os.waitpid(pid,0),(pid,0))
184184

185185

186186
@unittest.skipUnless(hasattr(os, 'posix_spawn'), "test needs os.posix_spawn")
187187
def test_posix_spawn_file_actions(self):
188-
file_actions = []
189-
file_actions.append((0,3,os.path.realpath(__file__),0,0))
190-
file_actions.append((os.POSIX_SPAWN_CLOSE,2))
191-
file_actions.append((os.POSIX_SPAWN_DUP2,1,4))
188+
file_actions = (
189+
(0,3,os.path.realpath(__file__),0,0),
190+
(os.POSIX_SPAWN_CLOSE,2),
191+
(os.POSIX_SPAWN_DUP2,1,4)
192+
)
192193
pid = posix.posix_spawn(sys.executable, [sys.executable, "-c", "pass"], os.environ, file_actions)
193194
self.assertEqual(os.waitpid(pid,0),(pid,0))
194195

‎Misc/NEWS.d/next/Core and Builtins/2018-04-01-16-20-05.bpo-33191.hVSbgx.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a refleak in os.posix_spawn.

‎Modules/posixmodule.c‎

Lines changed: 47 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -5142,15 +5142,15 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
51425142
Py_ssize_t argc, envc;
51435143
PyObject* result = NULL;
51445144
PyObject* seq = NULL;
5145-
5145+
PyObject* file_actions_obj = NULL;
51465146

51475147
/* posix_spawn has three arguments: (path, argv, env), where
5148-
argv is a list or tuple of strings and env is a dictionary
5149-
like posix.environ. */
5148+
argv is a list or tuple of strings and env is a dictionary
5149+
like posix.environ. */
51505150

5151-
if (!PySequence_Check(argv)) {
5151+
if (!PyList_Check(argv) && !PyTuple_Check(argv)) {
51525152
PyErr_SetString(PyExc_TypeError,
5153-
"posix_spawn: argv must be a tuple or list");
5153+
"posix_spawn: argv must be a tuple");
51545154
goto exit;
51555155
}
51565156
argc = PySequence_Size(argv);
@@ -5181,8 +5181,8 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
51815181
}
51825182

51835183
pid_t pid;
5184-
if (file_actions != NULL && file_actions != Py_None) {
5185-
if(posix_spawn_file_actions_init(&_file_actions) != 0){
5184+
if (file_actions != Py_None) {
5185+
if (posix_spawn_file_actions_init(&_file_actions) != 0) {
51865186
PyErr_SetString(PyExc_OSError,
51875187
"Error initializing file actions");
51885188
goto exit;
@@ -5191,104 +5191,104 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
51915191
file_actionsp = &_file_actions;
51925192

51935193
seq = PySequence_Fast(file_actions, "file_actions must be a sequence");
5194-
if(seq == NULL) {
5194+
if (seq == NULL) {
51955195
goto exit;
51965196
}
5197-
PyObject* file_actions_obj;
51985197
PyObject* mode_obj;
51995198

52005199
for (int i = 0; i < PySequence_Fast_GET_SIZE(seq); ++i) {
52015200
file_actions_obj = PySequence_Fast_GET_ITEM(seq, i);
5202-
5203-
if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)) {
5204-
PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence");
5201+
if (file_actions_obj == NULL) {
5202+
goto exit;
5203+
}
5204+
file_actions_obj = PySequence_Fast(file_actions_obj,
5205+
"Each file_action element must be"\
5206+
" a non-empty sequence");
5207+
if (file_actions_obj == NULL) {
52055208
goto exit;
52065209
}
5207-
52085210

52095211
mode_obj = PySequence_Fast_GET_ITEM(file_actions_obj, 0);
52105212
int mode = PyLong_AsLong(mode_obj);
5213+
if (mode == -1 && PyErr_Occurred()) {
5214+
goto exit;
5215+
}
52115216

52125217
/* Populate the file_actions object */
52135218

52145219
switch(mode) {
52155220

52165221
case POSIX_SPAWN_OPEN:
5217-
if(PySequence_Size(file_actions_obj) != 5) {
5222+
if (PySequence_Size(file_actions_obj) != 5) {
52185223
PyErr_SetString(PyExc_TypeError,"A open file_action object must have 5 elements");
52195224
goto exit;
52205225
}
5226+
int open_fd, open_oflag, open_mode;
5227+
char* open_path;
52215228

5222-
long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
5223-
if(PyErr_Occurred()) {
5224-
goto exit;
5225-
}
5226-
const char* open_path = PyUnicode_AsUTF8(PySequence_GetItem(file_actions_obj, 2));
5227-
if(open_path == NULL) {
5229+
if (!PyArg_ParseTuple(file_actions_obj, "iisii", &mode, &open_fd, &open_path,
5230+
&open_oflag, &open_mode)) {
52285231
goto exit;
52295232
}
5230-
long open_oflag = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 3));
5231-
if(PyErr_Occurred()) {
5232-
goto exit;
5233-
}
5234-
long open_mode = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 4));
5235-
if(PyErr_Occurred()) {
5236-
goto exit;
5237-
}
5238-
if(posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode)) {
5233+
5234+
if (posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode)) {
52395235
PyErr_SetString(PyExc_OSError,"Failed to add open file action");
52405236
goto exit;
52415237
}
5242-
5238+
52435239
break;
52445240

52455241
case POSIX_SPAWN_CLOSE:
5246-
if(PySequence_Size(file_actions_obj) != 2){
5242+
if (PySequence_Size(file_actions_obj) != 2) {
52475243
PyErr_SetString(PyExc_TypeError,"A close file_action object must have 2 elements");
52485244
goto exit;
52495245
}
52505246

5251-
long close_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
5252-
if(PyErr_Occurred()) {
5247+
int close_fd;
5248+
5249+
if (!PyArg_ParseTuple(file_actions_obj, "ii", &mode, &close_fd)) {
52535250
goto exit;
52545251
}
5255-
if(posix_spawn_file_actions_addclose(file_actionsp, close_fd)) {
5252+
5253+
if (posix_spawn_file_actions_addclose(file_actionsp, close_fd)) {
52565254
PyErr_SetString(PyExc_OSError,"Failed to add close file action");
52575255
goto exit;
52585256
}
5257+
52595258
break;
52605259

52615260
case POSIX_SPAWN_DUP2:
5262-
if(PySequence_Size(file_actions_obj) != 3){
5261+
if (PySequence_Size(file_actions_obj) != 3) {
52635262
PyErr_SetString(PyExc_TypeError,"A dup2 file_action object must have 3 elements");
52645263
goto exit;
52655264
}
52665265

5267-
long fd1 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
5268-
if(PyErr_Occurred()) {
5269-
goto exit;
5270-
}
5271-
long fd2 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 2));
5272-
if(PyErr_Occurred()) {
5266+
int fd1, fd2;
5267+
5268+
if (!PyArg_ParseTuple(file_actions_obj, "iii", &mode, &fd1, &fd2)) {
52735269
goto exit;
52745270
}
5275-
if(posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2)) {
5271+
5272+
if (posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2)) {
52765273
PyErr_SetString(PyExc_OSError,"Failed to add dup2 file action");
52775274
goto exit;
52785275
}
5276+
52795277
break;
52805278

52815279
default:
5282-
PyErr_SetString(PyExc_TypeError,"Unknown file_actions identifier");
5280+
PyErr_SetString(PyExc_ValueError,"Unknown file_actions identifier");
52835281
goto exit;
52845282
}
5283+
Py_DECREF(file_actions_obj);
52855284
}
5285+
file_actions_obj = NULL;
52865286
}
52875287

52885288
_Py_BEGIN_SUPPRESS_IPH
52895289
int err_code = posix_spawn(&pid, path->narrow, file_actionsp, NULL, argvlist, envlist);
52905290
_Py_END_SUPPRESS_IPH
5291-
if(err_code) {
5291+
if (err_code) {
52925292
PyErr_SetString(PyExc_OSError,"posix_spawn call failed");
52935293
goto exit;
52945294
}
@@ -5297,11 +5297,12 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
52975297
exit:
52985298

52995299
Py_XDECREF(seq);
5300+
Py_XDECREF(file_actions_obj);
53005301

5301-
if(file_actionsp) {
5302+
if (file_actionsp) {
53025303
posix_spawn_file_actions_destroy(file_actionsp);
53035304
}
5304-
5305+
53055306
if (envlist) {
53065307
free_string_array(envlist, envc);
53075308
}

0 commit comments

Comments
 (0)