| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@gpshead This PR introduces 4 new functions which are simple wrappers. The rationale is explained in the main comment, and other details including testing instructions are provided as inline comments on the diffs. For now I have converted #101832 and #101833 to drafts since they require more work and are slightly more ambitious projects. As always, no rush. |
Sorry, something went wrong.
Sorry, something went wrong.
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
|
@erlend-aasland Thank you for being patient. Turns out that unlike unlike openpty(), login_tty(), and forkpty(), these 4 POSIX functions are a part of the standard C library on *nix and not a part of libutil. Therefore, I just needed to add the function names in an AC_CHECK_FUNCS block in configure.ac. However, maybe we need to make a separate PR to modify the openpty(), login_tty(), forkpty() block in configure.ac to use WITH_SAVE_ENV? |
Sorry, something went wrong.
Much better, thanks! There is no need for WITH_SAVE_ENV now, AFAICS. The configure.ac changes look good to me. I'll leave the rest of the review for @zware and @gpshead who've been active on the issue and previous PRs. |
Sorry, something went wrong.
|
@gpshead Just a gentle reminder. No rush. |
Sorry, something went wrong.
| sig_saved = PyOS_setsig(SIGCHLD, SIG_DFL); | ||
| ret = grantpt(fd); | ||
| PyOS_setsig(SIGCHLD, sig_saved); |
There was a problem hiding this comment.
This relies on the GIL. IMO, that shouldn't block this PR for 3.13, but I'd like to follow up.
@colesbury, I plan to do lots of reviews this year. Is there anything I should do when I spot a case like this?
Sorry, something went wrong.
There was a problem hiding this comment.
Let's track these in an issue with the "topic-free-threading" label
Sorry, something went wrong.
There was a problem hiding this comment.
Filed here: #114727
Sorry, something went wrong.
|
Reading the manpages further, I see that Linux has a reentrant variant of ptsname:
IMO, os.ptsname should call this on Linux if it's available (with a char buffer[MAXPATHLEN+1]), and the docs should note that the function isn't thread-safe on other systems. |
Sorry, something went wrong.
|
(If you prefer to leave ptsname_r to another PR, that's an option too.) |
Sorry, something went wrong.
|
@encukou I prefer to include ptsname_r in this PR; thank you for the suggestion and sorry about the late reply. This is on my TODO list; please give me 1 more day (or maybe 2). |
Sorry, something went wrong.
|
There's no rush, take your time :) |
Sorry, something went wrong.
…is available. Make os.grantpt() use saved errno upon failure. Update documentation related to the POSIX pty functions. Add a test for the POSIX pty functions. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
|
I have made the changes you requested and more; please take a look at them when you get time:
Edit: while this is not the focus of this PR, about point 7 above, I have generated some data using grep on the cpython repository and added it here: #85984 (comment) (commenting again for better reach). Edit: Sorry about the obsessive spam; I cannot help myself, I have OCD. Before taking a break from this, I would like to finally point out that a pty is a pair of files and not a pair of fds. Therefore, if we call them main file/main end and second file/second end, then in code the corresponding terms might be main_fd and second_fd. That is misleading: a file can be addressed via multiple fds and upon reopening the second file/second end, we should call the new fd third_fd or second_fd_of_second_end and not reopened_second_fd (since you open files and not file descriptors). ON THE OTHER HAND: main end and secondARY end might be better. I still think we should grep for all possible terms to avoid collision. |
Sorry, something went wrong.
|
1-4. Thank you! Let's pretend second_fd stands for fd of the second file, and reopened_second_fd stands for fd for the reopened second file, or reopened fd for the second file. Does that work better? If not, change this to whatever feels best :) |
Sorry, something went wrong.
|
@encukou you are very kind. Let me work on the reconfigure (and everything else you suggested). |
Sorry, something went wrong.
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
|
@encukou Looks like all checks have passed! Your /Tools/build/regen-configure.sh suggestion worked like a charm... after I moved myself away from a directory that had colons (:) in its name as a time (hr, min, sec) separator; docker does not seem to like that. |
Sorry, something went wrong.
|
@encukou Thank you for merging this! Looks like finally after years I have all the prerequisites for #101833. That one now needs to be updated and maybe broken up into a few parts in case it becomes too difficult to review: one for signal handling, one for winsize handling, one for adjusting the tests, etc. I will do this sometime during the next few months. |
Sorry, something went wrong.
|
Thank you for perservering! If I happen to miss a notification (= I don't reply for about a week), please ping me again. |
Sorry, something went wrong.
The tests for that were my first contribution to CPython in 2020. However, I still need to update the tests to use the various robust functions that I have added to os, termios, and tty over the years instead of the ad-hoc ones that I wrote in Lib/test/test_pty.py. If additional tests are required, I will definitely add them if you wish :) |
Sorry, something went wrong.
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Petr Viktorin <encukou@gmail.com>
| Back | FazBrowse Home | New Git URL |
This follows #101831. This is one in a series of PRs aimed at cleaning-up, fixing bugs in, introducing new features in, and updating the code in "Lib/pty.py".
This PR answers the following question: os.forkpty() and pty.fork() return a pair pid, fd, where fd is a file descriptor of the master end of a pseudo-terminal pair; if at a later stage one needs to make some modifications to the slave end (such as setting termios attributes), then how does one obtain access to it without reimplementing ptsname() in Python or loading it from a shared library like someone is doing here: https://stackoverflow.com/questions/52338062/calling-libc-select-from-python-from-pty-master-side?
This is a dependency of #101833, which only needs os.ptsname(). However, this PR also adds os.posix_openpt(), os.grantpt(), and os.unlockpt() since all of these POSIX functions are "companions" of each other.
Signed-off-by: Soumendra Ganguly soumendraganguly@gmail.com