Expose termination signal option in clone

[tailhook]

I'm talking about this commit:
53cae89

The issue is that according to a man page you need to mix a signal name to a flags in clone function, like this:

pid = clone(childFunc, stackTop, CLONE_NEWUTS | SIGCHLD, argv[1]);

But with bitflags you can't do it. E.g. CloneFlags::from_bits(SIGCHLD) returns None.

Any good idea how to fix it? Revert the commit?

[fiveop]

I would prefer to change the signature of clone to take two parameters, which we then combine in the function. This way we can have our cake and eat it too :)

Thank you for reporting the issue.

[tailhook]

@fiveop, well, in the docs there are variable argument list for the clone system call. So adding an argument might confuse users.

Maybe some another type, like struct CloneConfig(CloneFlags, i32) ? Or just a tuple?

[fiveop]

Our wrapper does not pass any additional arguments currently. And even if we did, I would prefer not to create a type for that. However, documentation for our wrapper would certainly help. Lamentably, this holds for pretty much all of nix-rust.

[tailhook]

Our wrapper does not pass any additional arguments currently

Sure. I mean if I see clone(a, b, c, d) I believe that I can just look at man clone to find out what the arguments mean. I don't think it's reasonable to make documentation better than linux man, so people will always look at man pages on system calls to reason how this system call work.

[kamalmarhubi]

On #348, we came to the conclusion that directly following libc's flags | termination_signal parameter doesn't make sense, even though it's possible via std::ops::BitOr. @posborne said it very clearly:

I've come around to the argument that in this case having a separate argument exposed from nix for signal/flags probably makes most sense. The combination of the signal/flags in the kernel interface (and libc by extension) appears to be purely an optimization.

[kamalmarhubi]

In terms of "maximal Rustiness", at least for matching the APIs in libstd, I think something like this would work:

struct CloneOptions {
    flags : CloneFlags,
    termination_signal: Signal,
}

impl CloneOptions {
    pub fn new() -> CloneOptions { ... }
    pub fn flags(&mut self, flags: CloneFlags) -> &mut CloneOptions { ... }
    pub fn termination_signal(&mut self, signal: Signal) -> &mut CloneOptions { ... }
    pub fn clone(&self, stack: &mut [u8], mut cb: CloneCb) -> Result<pid_t> { ... }
}

This would get used like this:

let stack = vec![0u8; 1024 * 1024];
let child_pid = CloneOptions::new().flags(CLONE_NEWUSER | CLONE_NEWNS)
                                   .termination_signal(SIGUSR1)
                                   .clone(&mut stack[..], || { ... })
                                   .expect("clone");

[kamalmarhubi]

NB I don't think this fits in our current API style, but does fit a vision that @carllerche for either base nix or a higher level crate on top.

[tailhook]

Well, I don't think we should make it so complex. From #348 it looks like most people are comfortable with a separate argument. I think we should do it and move on.

[fiveop]

Which I implemented in my #344.