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

Add reboot() support on Linux #386

Merged
merged 10 commits into from
Jul 16, 2016
3 changes: 3 additions & 0 deletions src/sys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ pub mod stat;
#[cfg(any(target_os = "linux", target_os = "android"))]
pub mod syscall;

#[cfg(any(target_os = "linux"))]
pub mod reboot;

#[cfg(not(target_os = "ios"))]
pub mod termios;

Expand Down
50 changes: 50 additions & 0 deletions src/sys/reboot.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//! Reboot/shutdown or enable/disable Ctrl-Alt-Delete.

use {Errno, Error, Result};
use libc;
use void::Void;
use std::mem::drop;

/// How exactly should the system be rebooted.
///
/// See [`set_cad_enabled()`](fn.set_cad_enabled.html) for
/// enabling/disabling Ctrl-Alt-Delete.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum RebootMode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Somehow a comment went missing yesterday; I might have forgotten to press comment).

What problem did occur when you tried to use #[repr(i32)]? All constants have type libc::c_int which is an alias for i32. The enumeration Signal in sys/signal.rs does the same. You could then get rid of the match bellow.
As you can see there, I used the name of the corresponding constants for the elements of the enumeration. I would prefere if we did that here as well, even though we have no convention yet. However, bitflags types use this convention, so it would be consistent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having looked at your commit history, your problem probably was that you still have to cast the enumeration elements to libc::c_int in order to pass them to reboot, i.e.

reboot(cmd as libc::c_int)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see from this commit, that's exactly what I tried before, and here's the Travis failure that GitHub UI displays as a little red cross next to the commit -- however now that I look carefully I see that the commit hash is wrong and the link back gives me 404 -- is it a bug or something (I definetely commited that after rebasing)?

On your "middle" point -- do you mean to call the enum variants RebootMode::RB_POWER_OFF as so on? Doesn't seem beautiful; but if that's the convention -- OK ¯\_(ツ)_/¯. I would rather prefer to name them appropriately and list the C analogues in the documentation, but we obviously can't rename all of the bitflags now, so consistency wins, yeah.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the names, you understood me correctly.

I had a look at the Travis output and I dont understand it. The constants are libc::c_int in libc, which is i32 hence an enumeration with #[repr(i32)] should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe it is a bug in GH/Travis. Let me change the names and try #[repr(i32)] one more time then.

Halt,
kexec,
PowerOff,
Restart,
// we do not support Restart2,
Suspend,
}

pub fn reboot(how: RebootMode) -> Result<Void> {
let cmd = match how {
RebootMode::Halt => libc::RB_HALT_SYSTEM,
RebootMode::kexec => libc::RB_KEXEC,
RebootMode::PowerOff => libc::RB_POWER_OFF,
RebootMode::Restart => libc::RB_AUTOBOOT,
// we do not support Restart2,
RebootMode::Suspend => libc::RB_SW_SUSPEND,
};
unsafe {
libc::reboot(cmd)
};
Err(Error::Sys(Errno::last()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errno::result(res).map(drop) should work here just like in set_cad_enabled, so that we consistently have Result<()> as the return value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the whole point is that reboot() never returns, unless there's an error. Same thing as execvp and friends.
set_cad_enabled() (btw is that a good name?) on the other hand returns if everything went fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what the manual says:

For the values of cmd that stop or restart the system, a successful call to reboot() does not return. For the other cmd values, zero is returned on success. In all cases, -1 is returned on failure, and errno is set appropriately.

}

/// Enable or disable the reboot keystroke (Ctrl-Alt-Delete).
///
/// Corresponds to calling `reboot(RB_ENABLE_CAD)` or `reboot(RB_DISABLE_CAD)` in C.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we mention in RebootMode's documentation, that this function exists, in case someone is looking for that functionality.

Copy link
Contributor Author

@bugaevc bugaevc Jul 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will do. Maybe I should just document everything.

pub fn set_cad_enabled(enable: bool) -> Result<()> {
let cmd = if enable {
libc::RB_ENABLE_CAD
} else {
libc::RB_DISABLE_CAD
};
let res = unsafe {
libc::reboot(cmd)
};
Errno::result(res).map(drop)
}