这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@2096779623
Copy link
Member

CC @5ec1cff .

@twaik
Copy link
Member

twaik commented Aug 12, 2024

Is it supposed to do something like echo $(pm path com.termux 2>&1 </dev/null)? I mean using anonymous pipe to avoid selinux related problems?

@2096779623
Copy link
Member Author

Is it supposed to do something like echo $(pm path com.termux 2>&1 </dev/null)? I mean using anonymous pipe to avoid selinux related problems?

Yes.

@agnostic-apollo
Copy link
Member

  • pump_stderr definition should be above set_fd() definition.
  • Separate blocks of actions done on each fd with a newline in both child and parent, like close/set_fd/dup2/pthread_create.
  • It's good practice to close fds explicitly after using them (in child after pumps).
  • The code will block on read() in pump_stdout() instead of waitpid() and after parent process exits, read() should return with a 0/EOF after all data parent wrote before exiting has been read, and then code will move to waitpid(). The issue here is that stderr may not have pumped completely before child exists after waitpid(). So after pump_stdout(), it should wait for stderr thread, like with pthread_join() (instead of using pthread_detach()). You can test by running some command that prints an error. This should not be necessary for stdin thread as if waitpid() returns, parent should have already read it completely if it needed to.

@2096779623
Copy link
Member Author

  • pump_stderr definition should be above set_fd() definition.
  • Separate blocks of actions done on each fd with a newline in both child and parent, like close/set_fd/dup2/pthread_create.
  • It's good practice to close fds explicitly after using them (in child after pumps).
  • The code will block on read() in pump_stdout() instead of waitpid() and after parent process exits, read() should return with a 0/EOF after all data parent wrote before exiting has been read, and then code will move to waitpid(). The issue here is that stderr may not have pumped completely before child exists after waitpid(). So after pump_stdout(), it should wait for stderr thread, like with pthread_join() (instead of using pthread_detach()). You can test by running some command that prints an error. This should not be necessary for stdin thread as if waitpid() returns, parent should have already read it completely if it needed to.

Done.

@agnostic-apollo
Copy link
Member

Looks good, thanks. Maybe replace p2 with t_stdout and p_in with p_stdout, etc as currently p_in and p_out looks like some kind of input and output, instead of stdin/stdout.

Did you to test to see if stderr is fully printing? It should though.

@agnostic-apollo
Copy link
Member

You didn't rename p_* variables, otherwise looks fine.

@2096779623
Copy link
Member Author

2096779623 commented Aug 13, 2024

Did you to test to see if stderr is fully printing? It should though.

Yes,I did.

Co-authored-by: 5ec1cff <ewtqyqyewtqyqy@gmail.com>
Co-authored-by: Henrik Grimler <henrik@grimler.se>
@2096779623
Copy link
Member Author

You didn't rename p_* variables, otherwise looks fine.

Done.

@agnostic-apollo
Copy link
Member

Now you have added a _ after std, like p_std_out, so it's now different from t_stdout and pump_stdout. But you can merge anyways.

@2096779623 2096779623 merged commit e0e223b into termux:master Aug 13, 2024
fornwall added a commit to termux/termux-packages that referenced this pull request Aug 27, 2024
Starting with termux/termux-tools#111, released
in v1.43.4, this package contains native code.
termux-pacman-bot added a commit to termux-pacman/termux-packages that referenced this pull request Aug 27, 2024
Starting with termux/termux-tools#111, released
in v1.43.4, this package contains native code.
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.

3 participants