-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
gh-132742: Refactor fcntl.fcntl() and fcntl.ioctl() #132768
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
Conversation
e42daf4
to
29cb8b9
Compare
29cb8b9
to
ec30275
Compare
@vstinner, could you please take a look? Other changes are planned for other PRs. |
char buf[IOCTL_BUFSZ+1]; /* argument plus NUL byte */ | ||
if (mutate_arg && !PyBytes_Check(arg) && !PyUnicode_Check(arg)) { | ||
if (PyObject_GetBuffer(arg, &view, PyBUF_WRITABLE) == 0) { | ||
if (view.len <= IOCTL_BUFSZ) { |
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.
There are many indentation levels (nested if), did you consider to split this code into sub-functions?
Co-authored-by: Victor Stinner <vstinner@python.org>
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.
LGTM
Thank you for your review, @vstinner. I just tried to split the code on smaller functions. This adds many duplications: declarations of local variables |
|
📚 Documentation preview 📚: https://cpython-previews--132768.org.readthedocs.build/