Skip to content

return the new file descriptor from os.dup2 (closes bpo-32441)#5041

Merged
benjaminp merged 2 commits intomasterfrom
benjamin-dup2-return
Dec 29, 2017
Merged

return the new file descriptor from os.dup2 (closes bpo-32441)#5041
benjaminp merged 2 commits intomasterfrom
benjamin-dup2-return

Conversation

@benjaminp
Copy link
Copy Markdown
Contributor

@benjaminp benjaminp commented Dec 29, 2017

@benjaminp benjaminp force-pushed the benjamin-dup2-return branch from 4f56378 to 251d3a2 Compare December 29, 2017 07:24
Comment thread Modules/posixmodule.c Outdated
#endif

Py_RETURN_NONE;
return PyLong_FromLong(res);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you rely on Argument Clinic as os_dup_impl does?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, argument clinic actually complicates the error handling, but I suppose it's the future.

Copy link
Copy Markdown
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@benjaminp benjaminp merged commit bbdb17d into master Dec 29, 2017
@benjaminp benjaminp deleted the benjamin-dup2-return branch December 29, 2017 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants