Skip to content

Print out errname in impl Debug for Error #287

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

Merged
merged 1 commit into from
May 26, 2021
Merged
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
1 change: 1 addition & 0 deletions rust/kernel/bindings_helper.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */

#include <linux/cdev.h>
#include <linux/errname.h>
#include <linux/fs.h>
#include <linux/module.h>
#include <linux/random.h>
Expand Down
28 changes: 26 additions & 2 deletions rust/kernel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,19 @@
//!
//! C header: [`include/uapi/asm-generic/errno-base.h`](../../../include/uapi/asm-generic/errno-base.h)

use crate::str::CStr;
use crate::{bindings, c_types};
use alloc::{alloc::AllocError, collections::TryReserveError};
use core::convert::From;
use core::{num::TryFromIntError, str::Utf8Error};
use core::fmt;
use core::num::TryFromIntError;
use core::str::{self, Utf8Error};

/// Generic integer kernel error.
///
/// The kernel defines a set of integer generic error codes based on C and
/// POSIX ones. These codes may have a more specific meaning in some contexts.
#[derive(Debug)]
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct Error(c_types::c_int);

impl Error {
Expand Down Expand Up @@ -61,6 +64,27 @@ impl Error {
}
}

impl fmt::Debug for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Copy link
Member

Choose a reason for hiding this comment

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

I like the idea that we could have names instead of values for the Debug::fmt of Error. However, I think it's better that we don't need to call errname if we already have the corresponding const, e.g. Error::EINVAL, because having the const actually means we have names for the error numbers at Rust side.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't currently have constants for all errors defined in Rust. Plus, defining them in Rust essentially requires us to have a giant match, or define a lookup table like errname.c did. It might be better just to reuse errname, C side needs it anyway.

// SAFETY: FFI call.
#[cfg(CONFIG_SYMBOLIC_ERRNAME)]
let name = unsafe { crate::bindings::errname(-self.0) };
#[cfg(not(CONFIG_SYMBOLIC_ERRNAME))]
let name: *const c_types::c_char = core::ptr::null();

if name.is_null() {
// Print out number if no name can be found.
return f.debug_tuple("Error").field(&-self.0).finish();
}

// SAFETY: `'static` string from C, and is not NULL.
let cstr = unsafe { CStr::from_char_ptr(name) };
// SAFETY: These strings are ASCII-only.
let str = unsafe { str::from_utf8_unchecked(&cstr) };
f.debug_tuple(str).finish()
}
}

impl From<TryFromIntError> for Error {
fn from(_: TryFromIntError) -> Error {
Error::EINVAL
Expand Down