Skip to content
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

Missing killpg #644

Closed
mmstick opened this issue Jul 3, 2017 · 8 comments
Closed

Missing killpg #644

mmstick opened this issue Jul 3, 2017 · 8 comments

Comments

@mmstick
Copy link

mmstick commented Jul 3, 2017

Needed by Ion to send signals to all members of a process group.

@Susurrus
Copy link
Contributor

Susurrus commented Jul 4, 2017

@mmstick Would you be willing to file a PR adding this to nix? I'd be happy to mentor you through the process if you would. Always looking for new contributors!

@mmstick
Copy link
Author

mmstick commented Jul 4, 2017

Sure. Where can I start?

@Susurrus
Copy link
Contributor

Susurrus commented Jul 5, 2017

The first step is actually to get killpg and all related constants into the https://github.com/rust-lang/libc repo for all appropriate targets (see the supported list of targets for nix in the README).

Once that's done, you can file a PR here adding killpg after kill in src/sys/signal.rs. Be sure to add doc comments and ideally a test if you could (spawn a new process group of something and try to kill it, which should work without sudo privileges I would think so it can be safely run in CI).

@SanchayanMaity
Copy link
Contributor

@Susurrus killpg has two signatures.

int killpg(int pgrp, int sig);
int killpg(pid_t pgrp, int sig);

How to handle this? Or should the pid_t signature alone be sufficient? Do I understand correctly that the below change in rust libc alone is sufficient to carry this forward?.

diff --git a/src/unix/mod.rs b/src/unix/mod.rs
index 35a7f448..2e65b43c 100644
--- a/src/unix/mod.rs
+++ b/src/unix/mod.rs
@@ -562,6 +562,7 @@ extern {
     #[cfg_attr(all(target_os = "macos", target_arch = "x86"),
                    link_name = "kill$UNIX2003")]
     pub fn kill(pid: pid_t, sig: ::c_int) -> ::c_int;
+    pub fn killpg(pgrp: pid_t, sig: ::c_int) -> ::c_int;
 
     pub fn mlock(addr: *const ::c_void, len: ::size_t) -> ::c_int;
     pub fn munlock(addr: *const ::c_void, len: ::size_t) -> ::c_int;

@asomers
Copy link
Member

asomers commented Dec 17, 2017

AFAIK pid_t is equal to int everywhere. But POSIX says that the first argument should be pid_t.. Your suggested patch looks correct to me.

@SanchayanMaity
Copy link
Contributor

killpg in rust-lang/libc#879 is now merged.

@Susurrus
Copy link
Contributor

@mmstick @SanchayanMaity With killpg in libc, please feel free to fille a PR adding this to all relevant platforms.

@ghost
Copy link

ghost commented May 10, 2018

I too need killpkg for my project to kill all processes associated by group id since std::process::Child::kill() is not good enough for what I'm doing. It would be useful to have instead of relying on the libc crate.

@DanSnow DanSnow mentioned this issue Mar 4, 2019
bors bot added a commit that referenced this issue Mar 16, 2019
1034: Add killpg r=asomers a=DanSnow

It's seem that #644 is inactively about 1 year. But I really want this API that can land in nix.

Actually I not really understand how to check the availability of an API for other platform except Linux. But I saw that this API in `libc` is wrapping inside a `#[cfg(unix)]`. So I don't need to add any `#[cfg]` on this API, am I right?

Resolved #644 

Co-authored-by: DanSnow <[email protected]>
@bors bors bot closed this as completed in #1034 Mar 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants