Add a FrameBuffer::into_buffer method for taking ownership

[asensio-project]

Hello,

I had a problem related to lifetimes when I was adding support for the logger in my kernel. I got E0597. Framebuffer must outlive 'static. This was strange for me because the logger code is same that is in the bootloader. But I noticed that the methods buffer() and buffer_mut don't use any lifetime.

With this lifetime annotation in the both methods it works.

Thanks.

[Freax13]

This is unsound: Decoupling the lifetime of the FrameBuffer from the returned slice allows creating multiple concurrently-alive mutable references:

// Get two mutable references to the same buffer.
let buffer1 = frame_buffer.buffer_mut();
let buffer2 = frame_buffer.buffer_mut();

// Access the first buffer.
// This shouldn't compile. Calling `buffer_mut` the second time should invalidate the lifetime of `buffer1`.
buffer1[0] = 0;

[asensio-project]

So, how can we fix my issue?

[Freax13]

So, how can we fix my issue? May be using 'static instead.

With these changes, the methods basically already use 'static. Can you provide some of the code which caused the original error?

We could add fn into_buffer(self) -> &'static mut [u8], which would allow creating exactly one mutable static reference.

[asensio-project]

// kernel/src/main.rs

#![no_std]
#![no_main]

use core::panic::PanicInfo;
use kernel::init_logger;
use bootloader_api::{entry_point, BootInfo, BootloaderConfig, info::Optional};

pub static BOOTLOADER_CONFIG: BootloaderConfig = {
let mut config = BootloaderConfig::new_default();
config.log_level = bootloader_api::config::LevelFilter::Error;
config
};

entry_point!(main, config = &BOOTLOADER_CONFIG);

fn main(bootinfo: &'static mut BootInfo) -> ! {
let framebuffer = core::mem::replace(&mut bootinfo.framebuffer, Optional::None);
let mut framebuffer = framebuffer.into_option().unwrap();
let info = framebuffer.info();
let buffer = framebuffer.buffer_mut();
init_logger(buffer, info);
log::info!("Booting up KERNEL 0.1!");
loop {}
}

#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
loop {}
}

[asensio-project]

// kernel/src/lib.rs
#![no_std]

pub mod logger;

use bootloader_api::info::FrameBufferInfo;

pub fn init_logger(framebuffer: &'static mut [u8], info: FrameBufferInfo) {
let logger = logger::LOGGER.call_once(move || logger::LockedLogger::new(framebuffer, info));
log::set_logger(logger).expect("logger already set");
log::set_max_level(log::LevelFilter::Trace);
}

[asensio-project]

// kernel/src/logger.rs

use bootloader_api::info::{FrameBufferInfo, PixelFormat};
use core::{
fmt::{self, Write},
ptr,
};
use font_constants::BACKUP_CHAR;
use noto_sans_mono_bitmap::{
get_raster, get_raster_width, FontWeight, RasterHeight, RasterizedChar,
};
use spin::{Mutex, once::Once};

/// The global logger instance used for the log crate.
pub static LOGGER: Once = Once::new();

/// A [Logger] instance protected by a spinlock.
pub struct LockedLogger(pub Mutex);

/// Additional vertical space between lines
const LINE_SPACING: usize = 2;
/// Additional horizontal space between characters.
const LETTER_SPACING: usize = 0;

/// Padding from the border. Prevent that font is too close to border.
const BORDER_PADDING: usize = 1;

/// Constants for the usage of the [noto_sans_mono_bitmap] crate.
mod font_constants {
use super::*;

/// Height of each char raster. The font size is ~0.84% of this. Thus, this is the line height that
/// enables multiple characters to be side-by-side and appear optically in one line in a natural way.
pub const CHAR_RASTER_HEIGHT: RasterHeight = RasterHeight::Size16;

/// The width of each single symbol of the mono space font.
pub const CHAR_RASTER_WIDTH: usize = get_raster_width(FontWeight::Regular, CHAR_RASTER_HEIGHT);

/// Backup character if a desired symbol is not available by the font.
/// The '�' character requires the feature "unicode-specials".
pub const BACKUP_CHAR: char = '�';

pub const FONT_WEIGHT: FontWeight = FontWeight::Regular;

}

/// Returns the raster of the given char or the raster of [font_constants::BACKUP_CHAR].
fn get_char_raster(c: char) -> RasterizedChar {
fn get(c: char) -> Option {
get_raster(
c,
font_constants::FONT_WEIGHT,
font_constants::CHAR_RASTER_HEIGHT,
)
}
get(c).unwrap_or_else(|| get(BACKUP_CHAR).expect("Should get raster of backup char."))
}

impl LockedLogger {
/// Create a new instance that logs to the given framebuffer.
pub fn new(framebuffer: &'static mut [u8], info: FrameBufferInfo) -> Self {
LockedLogger(Mutex::new(Logger::new(framebuffer, info)))
}

/// Force-unlocks the logger to prevent a deadlock.
///
/// ## Safety
/// This method is not memory safe and should be only used when absolutely necessary.
pub unsafe fn force_unlock(&self) {
    unsafe { self.0.force_unlock() };
}

}

impl log::Log for LockedLogger {
fn enabled(&self, _metadata: &log::Metadata) -> bool {
true
}

fn log(&self, record: &log::Record) {
    let mut logger = self.0.lock();
    writeln!(logger, "{:5}: {}", record.level(), record.args()).unwrap();
}

fn flush(&self) {}

}

/// Allows logging text to a pixel-based framebuffer.
pub struct Logger {
framebuffer: &'static mut [u8],
info: FrameBufferInfo,
x_pos: usize,
y_pos: usize,
}

impl Logger {
/// Creates a new logger that uses the given framebuffer.
pub fn new(framebuffer: &'static mut [u8], info: FrameBufferInfo) -> Self {
let mut logger = Self {
framebuffer,
info,
x_pos: 0,
y_pos: 0,
};
logger.clear();
logger
}

fn newline(&mut self) {
    self.y_pos += font_constants::CHAR_RASTER_HEIGHT.val() + LINE_SPACING;
    self.carriage_return()
}

fn carriage_return(&mut self) {
    self.x_pos = BORDER_PADDING;
}

/// Erases all text on the screen. Resets `self.x_pos` and `self.y_pos`.
pub fn clear(&mut self) {
    self.x_pos = BORDER_PADDING;
    self.y_pos = BORDER_PADDING;
    self.framebuffer.fill(0);
}

fn width(&self) -> usize {
    1080
}

fn height(&self) -> usize {
    1080
}

/// Writes a single char to the framebuffer. Takes care of special control characters, such as
/// newlines and carriage returns.
fn write_char(&mut self, c: char) {
    match c {
        '\n' => self.newline(),
        '\r' => self.carriage_return(),
        c => {
            let new_xpos = self.x_pos + font_constants::CHAR_RASTER_WIDTH;
            if new_xpos >= self.width() {
                self.newline();
            }
            let new_ypos =
                self.y_pos + font_constants::CHAR_RASTER_HEIGHT.val() + BORDER_PADDING;
            if new_ypos >= self.height() {
                self.clear();
            }
            self.write_rendered_char(get_char_raster(c));
        }
    }
}

/// Prints a rendered char into the framebuffer.
/// Updates `self.x_pos`.
fn write_rendered_char(&mut self, rendered_char: RasterizedChar) {
    for (y, row) in rendered_char.raster().iter().enumerate() {
        for (x, byte) in row.iter().enumerate() {
            self.write_pixel(self.x_pos + x, self.y_pos + y, *byte);
        }
    }
    self.x_pos += rendered_char.width() + LETTER_SPACING;
}

fn write_pixel(&mut self, x: usize, y: usize, intensity: u8) {
    let pixel_offset = y * self.info.stride + x;
    let color = match self.info.pixel_format {
        PixelFormat::Rgb => [intensity, intensity, intensity / 2, 0],
        PixelFormat::Bgr => [intensity / 2, intensity, intensity, 0],
        PixelFormat::U8 => [if intensity > 200 { 0xf } else { 0 }, 0, 0, 0],
        other => {
            // set a supported (but invalid) pixel format before panicking to avoid a double
            // panic; it might not be readable though
            self.info.pixel_format = PixelFormat::Rgb;
            panic!("pixel format {:?} not supported in logger", other)
        }
    };
    let bytes_per_pixel = self.info.bytes_per_pixel;
    let byte_offset = pixel_offset * bytes_per_pixel;
    self.framebuffer[byte_offset..(byte_offset + bytes_per_pixel)]
        .copy_from_slice(&color[..bytes_per_pixel]);
    let _ = unsafe { ptr::read_volatile(&self.framebuffer[byte_offset]) };
}

}

unsafe impl Send for Logger {}
unsafe impl Sync for Logger {}

impl fmt::Write for Logger {
fn write_str(&mut self, s: &str) -> fmt::Result {
for c in s.chars() {
self.write_char(c);
}
Ok(())
}
}

[asensio-project]

Sorry for the formatting.

[Freax13]

Ok, so to fix that, we could either add the into_buffer function I just described, or you could just pass the whole FrameBuffer struct to init_logger instead of passing the buffers. Either should fix the problem.

[asensio-project]

The first option, will do the same that my change, I think.

[Freax13]

The difference is that into_buffer takes ownership of the FrameBuffer, so it's not possible to call into_buffer twice.

[asensio-project]

OK, thanks! I will create a commit for fixing it.

[phil-opp]

Looks good, thanks!