Conversation
There was a problem hiding this comment.
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Files not reviewed (1)
- native/src/Android.mk: Language not supported
Comments suppressed due to low confidence (1)
native/src/core/su/pts.rs:79
- Consider checking the return value of signalfd to handle the potential error case if it returns -1.
signalfd(-1, &mask, SFD_CLOEXEC)
| // output to stdout, which cause the target process lost input. | ||
| pump_tty(ptmx, (atty & ATTY_IN) ? ptmx : -1); |
There was a problem hiding this comment.
The comment contains grammatical errors ('cause' should be 'causes' and 'lost' should be 'lose') to improve clarity.
| // output to stdout, which cause the target process lost input. | |
| pump_tty(ptmx, (atty & ATTY_IN) ? ptmx : -1); | |
| // output to stdout, which causes the target process to lose input. |
|
@topjohnwu I am not sure if you you have noticed, but last patch in this series effectively breaks su -c behavior that some apps have been relying on since 2012 (appending tty is koush's su default behaviour since then). Now these apps cannot react to external signals. Yes, I know it is different from linux su behaviour, but su is not posix, there isn't a standard for this. At least, this behavior change should be noticed in change log. |
…n a tty as required by Magisk now Related pull topjohnwu/Magisk#8927 Related pull termux/termux-packages#24684
| cfmakeraw(&termios); | ||
|
|
||
| if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &termios) < 0) { | ||
| // https://blog.zhanghai.me/fixing-line-editing-on-android-8-0/ |
There was a problem hiding this comment.
I think we should keep this note
|
Круто! |
After this, only one thread when running
su:Also fix:
su | catecho id | sucrtl + zwhensu -c sleep 100