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 eprintln macro #18

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions drivers/char/rust_example/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ struct RustExample {
impl KernelModule for RustExample {
fn init() -> KernelResult<Self> {
println!("Rust Example (init)");
println!("Am I built-in? {}", !cfg!(MODULE));
kprintln!(
level: kernel::printk::KERN_DEBUG,
"Am I built-in? {}",
!cfg!(MODULE)
);
println!("Parameters:");
println!(" my_bool: {}", my_bool.read());
println!(" my_i32: {}", my_i32.read());
Expand All @@ -45,7 +49,6 @@ impl KernelModule for RustExample {
impl Drop for RustExample {
fn drop(&mut self) {
println!("My message is {}", self.message);
println!("Rust Example (exit)");
eprintln!("Rust Example (exit)");
}
}

7 changes: 7 additions & 0 deletions rust/kernel/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,14 @@ const INCLUDED_VARS: &[&str] = &[
"FS_USERNS_MOUNT",
"FS_RENAME_DOES_D_MOVE",
"BINDINGS_GFP_KERNEL",
"KERN_EMERG",
"KERN_ALERT",
"KERN_CRIT",
"KERN_ERR",
"KERN_WARNING",
"KERN_NOTICE",
"KERN_INFO",
"KERN_DEBUG",
"VERIFY_WRITE",
"LINUX_VERSION_CODE",
"SEEK_SET",
Expand Down
12 changes: 2 additions & 10 deletions rust/kernel/src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,8 @@

//! The `kernel` prelude

pub use alloc::{
string::String,
borrow::ToOwned,
};
pub use alloc::{borrow::ToOwned, string::String};

pub use module::module;

pub use super::{
println,
KernelResult,
KernelModule,
};

pub use super::{eprintln, kprintln, println, KernelModule, KernelResult};
71 changes: 59 additions & 12 deletions rust/kernel/src/printk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ use core::fmt;
use crate::bindings;
use crate::c_types::c_int;

pub use crate::bindings::{
KERN_ALERT, KERN_CRIT, KERN_DEBUG, KERN_EMERG, KERN_ERR, KERN_INFO, KERN_NOTICE, KERN_WARNING,
};
Comment on lines +9 to +11
Copy link
Member

Choose a reason for hiding this comment

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

We should not just re-export these as the public API. We should have a type-safe API for specifying these if they're aprt of the public API.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, so a level enum

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that'd be good.


const LEVEL_LEN: usize = 3;

#[doc(hidden)]
pub fn printk(s: &[u8]) {
pub fn printk(s: &[u8], level: &'static [u8; LEVEL_LEN]) {
// Don't copy the trailing NUL from `KERN_INFO`.
Copy link
Member

Choose a reason for hiding this comment

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

Comment should be updated.

let mut fmt_str = [0; bindings::KERN_INFO.len() - 1 + b"%.*s\0".len()];
fmt_str[..bindings::KERN_INFO.len() - 1]
.copy_from_slice(&bindings::KERN_INFO[..bindings::KERN_INFO.len() - 1]);
fmt_str[bindings::KERN_INFO.len() - 1..].copy_from_slice(b"%.*s\0");
let mut fmt_str = [0; LEVEL_LEN - 1 + b"%.*s\0".len()];
fmt_str[..LEVEL_LEN - 1].copy_from_slice(&level[..LEVEL_LEN - 1]);
fmt_str[LEVEL_LEN - 1..].copy_from_slice(b"%.*s\0");

// TODO: I believe printk never fails
unsafe { bindings::printk(fmt_str.as_ptr() as _, s.len() as c_int, s.as_ptr()) };
Expand Down Expand Up @@ -50,6 +55,33 @@ impl fmt::Write for LogLineWriter {
}
}

/// [`kprintln!`] prints to the kernel console with a given level.
/// If no level is given, it will default to `KERN_INFO`.
#[macro_export]
macro_rules! kprintln {
() => ({
kprintln!(level: $crate::printk::KERN_INFO);
});
(level: $level:expr) => ({
$crate::printk::printk("\n".as_bytes(), $level);
});
($msg:expr) => ({
kprintln!(level: $crate::printk::KERN_INFO, $msg);
});
(level: $level:expr, $msg:expr) => ({
$crate::printk::printk(concat!($msg, "\n").as_bytes(), $level);
});
(level: $level:expr, $fmt:expr, $($arg:tt)*) => ({
Copy link
Member

Choose a reason for hiding this comment

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

I'm not wild about this syntax. I think we should just add eprintln!() rather than add an API that's subtly different from the println!() everyone knows.

If we want to add our own API to provide this (e.g. kprintln!()) that'd be ok with me though.

Copy link
Member Author

@kloenk kloenk Oct 5, 2020

Choose a reason for hiding this comment

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

I added kprintln! for that feature.
(It's still untested as I can't build this kernel)

use ::core::fmt;
let mut writer = $crate::printk::LogLineWriter::new();
let _ = fmt::write(&mut writer, format_args!(concat!($fmt, "\n"), $($arg)*)).unwrap();
$crate::printk::printk(writer.as_bytes(), $crate::printk::KERN_INFO);
});
($fmt:expr, $($arg:tt)*) => ({
kprintln!(level: $crate::printk::KERN_INFO, $fmt, $($arg)*);
});
}

/// [`println!`] functions the same as it does in `std`, except instead of
/// printing to `stdout`, it writes to the kernel console at the `KERN_INFO`
/// level.
Expand All @@ -58,15 +90,30 @@ impl fmt::Write for LogLineWriter {
#[macro_export]
macro_rules! println {
() => ({
$crate::printk::printk("\n".as_bytes());
kprintln!(level: $crate::printk::KERN_INFO);
});
($fmt:expr) => ({
$crate::printk::printk(concat!($fmt, "\n").as_bytes());
($msg:expr) => ({
kprintln!(level: $crate::printk::KERN_INFO, $msg);
Copy link
Member

Choose a reason for hiding this comment

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

I don't love this syntax. Putting a "keyword argument" (which is what we're emulating) before positional arguments feels weird to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Took that from the log crate. The do it with the target argument

});
($fmt:expr, $($arg:tt)*) => ({
use ::core::fmt;
let mut writer = $crate::printk::LogLineWriter::new();
let _ = fmt::write(&mut writer, format_args!(concat!($fmt, "\n"), $($arg)*)).unwrap();
$crate::printk::printk(writer.as_bytes());
kprintln!(level: $crate::printk::KERN_INFO, $fmt, $($arg)*);
});
}

/// [`eprintln!`] functions the same as it does in `std`, except instead of
/// printing to `stderr`, it writes to the kernel console at the `KERN_ERR`
/// level.
///
/// [`eprintln!`]: https://doc.rust-lang.org/stable/std/macro.eprintln.html
#[macro_export]
macro_rules! eprintln {
() => ({
kprintln!(level: $crate::printk::KERN_ERR);
});
($msg:expr) => ({
kprintln!(level: $crate::printk::KERN_ERR, $msg);
});
($fmt:expr, $($arg:tt)*) => ({
kprintln!(level: $crate::printk::KERN_ERR, $msg, $($arg)*);
});
}