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

Fix several compiler warnings #62

Merged
merged 29 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
547ad2c
tokenparts: Rename attribute that is never read
chrysn Oct 13, 2023
78dc334
Fix unused variable warning in cfg dependent section
chrysn Oct 13, 2023
f3a34e8
tests: Fix unused import warning
chrysn Oct 13, 2023
da5b6d6
gnrc: Fix unused use warning
chrysn Oct 15, 2023
93a4c98
gcoap: Fix unused mut
chrysn Oct 15, 2023
80f4443
embedded-nal: Rename unused arguments to _ versions
chrysn Oct 15, 2023
b65a0d6
gcoap: Remove unused internal function
chrysn Oct 15, 2023
280bf42
tests/led: Simplify to fix warnings
chrysn Oct 13, 2023
a72a4ab
Fix unused imports
chrysn Oct 15, 2023
28758b1
Silence deprecation warning from pub-use
chrysn Oct 13, 2023
09520f0
Fix deprecated direct access
chrysn Oct 15, 2023
1bfe708
saul: Enum variants that were legacy shifted towards constants need n…
chrysn Oct 15, 2023
614ad9d
Fix several unused item / import / mut warnings
chrysn Oct 20, 2023
88ce347
Fix several warnings, largely around deprecations
chrysn Oct 20, 2023
d99c90b
treewide/doc: Various fixes
chrysn Oct 15, 2023
5bb8849
thread/doc: Add comment on ValueInThread not being Send any more
chrysn Oct 13, 2023
15e0b09
thread: Remove intermediate module layer from constants, fixing warnings
chrysn Oct 13, 2023
d6de3cb
gnrc_pktbuf: Improve debugging experience by showing the writability …
chrysn Oct 15, 2023
afe025c
panic: Refactor
chrysn Oct 13, 2023
6430de2
pktbuf: Fix double free
chrysn Oct 14, 2023
bb73bf8
saul: Don't abuse negative errors with positive values
chrysn Oct 15, 2023
f230e66
shell: Refactor to avoid lint complaining about dropping reference
chrysn Oct 15, 2023
8f42354
tests: Add gnrc-pktbuf
chrysn Oct 15, 2023
7ae33f1
Move gnrc::pktbuf to gnrc_pktbuf
chrysn Oct 15, 2023
13cb379
Documentation improvements
chrysn Oct 20, 2023
e818bc8
Refactorings, partially to address warnings
chrysn Oct 20, 2023
d7d024c
gnrc_pktbuf: Fix double free and add test to catch it
chrysn Oct 20, 2023
dfd7a89
tests/gnrc_pktbuf: Try switching between shared and writable
chrysn Oct 15, 2023
dfb034f
Cleanup and quality-of-life improvements
chrysn Oct 20, 2023
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
4 changes: 2 additions & 2 deletions src/auto_init.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Tools for declaring a function that is run during initialization
//!
//! The [auto_init!] macro is this module's main product.
//! The [`auto_init!`](super::auto_init!) macro is this module's main product.

/// Wrapper around [riot_sys::auto_init_module_t]
///
Expand All @@ -13,7 +13,7 @@ impl AutoInitModule {
/// Initializer for module auto-initialization
///
/// Do not call this directly: Its result must be placed in a static in a special section in
/// memory, which is handled by the [`auto_init!`] macro.
/// memory, which is handled by the [`auto_init!`](super::auto_init!) macro.
pub const fn new(
init_function: extern "C" fn(),
priority: u16,
Expand Down
2 changes: 1 addition & 1 deletion src/bluetil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub enum Error {

impl From<crate::error::NumericError> for Error {
fn from(e: crate::error::NumericError) -> Error {
match e.number as _ {
match e.number() as _ {
riot_sys::BLUETIL_AD_NOTFOUND => Error::NotFound,
riot_sys::BLUETIL_AD_NOMEM => Error::NoMem,
_ => panic!("Invalid bluetil error"),
Expand Down
10 changes: 2 additions & 8 deletions src/gcoap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ unsafe extern "C" fn link_encoder<H: WithLinkEncoder>(
let h: &H = unsafe { &*((*resource).context as *const _) };

let buf = buf as *mut u8; // cast away signedness of char
let mut buf = if buf.is_null() {
let buf = if buf.is_null() {
None
} else {
Some(core::slice::from_raw_parts_mut(buf, buf_len as _))
Expand Down Expand Up @@ -259,7 +259,7 @@ impl<'a, H> SingleHandlerListener<'a, H>
where
H: 'a + Handler + WithLinkEncoder,
{
/// Like [`new()`], but utilizing that the handler is also [WithLinkEncoder] and can thus influence
/// Like [`Self::new()`], but utilizing that the handler is also [WithLinkEncoder] and can thus influence
/// what is reported when the default .well-known/core handler is queried.
pub fn new_with_link_encoder(
path: &'a core::ffi::CStr,
Expand Down Expand Up @@ -344,7 +344,6 @@ pub trait WithLinkEncoder {
}

use riot_sys::{
coap_get_total_hdr_len,
coap_opt_add_opaque,
coap_opt_add_uint,
coap_opt_get_next,
Expand Down Expand Up @@ -376,11 +375,6 @@ impl PacketBuffer {
}) as u8 // odd return type in C
}

/// Wrapper for coap_get_total_hdr_len
fn get_total_hdr_len(&self) -> usize {
(unsafe { coap_get_total_hdr_len(crate::inline_cast(self.pkt)) }) as usize
}

/// Wrapper for gcoap_resp_init
///
/// As it is used and wrapped here, this makes GCOAP_RESP_OPTIONS_BUF bytes unusable, but
Expand Down
3 changes: 2 additions & 1 deletion src/gnrc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ pub mod ipv6;

pub mod netapi;
pub mod netreg;
pub mod pktbuf;
#[deprecated(note = "moved to gnrc_pktbuf toplevel module")]
pub use crate::gnrc_pktbuf as pktbuf;

use riot_sys::{gnrc_netif_iter, gnrc_netif_t};

Expand Down
4 changes: 2 additions & 2 deletions src/gnrc/netreg.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[cfg(feature = "with_msg_v2")]
use core::mem::MaybeUninit;

use riot_sys::{gnrc_netreg_entry_t, gnrc_netreg_register, gnrc_netreg_unregister, gnrc_nettype_t};

#[cfg(feature = "with_msg_v2")]
use crate::error::NegativeErrorExt;

// Transmuting the pointer into a Pktsnip does the right thing by treating it as a smart
Expand Down
23 changes: 18 additions & 5 deletions src/gnrc/pktbuf.rs → src/gnrc_pktbuf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use riot_sys::{
gnrc_pktbuf_realloc_data,
gnrc_pktbuf_release_error,
gnrc_pktsnip_t,
gnrc_udp_hdr_build,
GNRC_NETERR_SUCCESS,
};

Expand Down Expand Up @@ -138,6 +137,8 @@ impl<M: Mode> Pktsnip<M> {
src: core::num::NonZeroU16,
dst: core::num::NonZeroU16,
) -> Result<Pktsnip<Writable>, NotEnoughSpace> {
use riot_sys::gnrc_udp_hdr_build;

let snip = unsafe { gnrc_udp_hdr_build(self.ptr, src.into(), dst.into()) };
if snip == 0 as *mut _ {
// All other errors are caught by the signature
Expand Down Expand Up @@ -253,13 +254,19 @@ impl<'a> Pktsnip<Writable> {
size: usize,
nettype: gnrc_nettype_t,
) -> Result<Self, NotEnoughSpace> {
let next = next.map(|s| s.ptr).unwrap_or(0 as *mut _);
let snip =
unsafe { gnrc_pktbuf_add(next, data as *const _, size.try_into().unwrap(), nettype) };
let next_ptr = next.as_ref().map(|s| s.ptr).unwrap_or(0 as *mut _);
forget(next);
let snip = unsafe {
gnrc_pktbuf_add(
next_ptr,
data as *const _,
size.try_into().unwrap(),
nettype,
)
};
if snip == 0 as *mut _ {
return Err(NotEnoughSpace);
}
forget(next);
Ok(unsafe { Pktsnip::<Writable>::from_ptr(snip) })
}

Expand All @@ -284,7 +291,13 @@ impl<'a> Pktsnip<Writable> {

impl<M: Mode> ::core::fmt::Debug for Pktsnip<M> {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
let mode = core::any::type_name::<M>();
let mode = mode
.rsplit("::")
.next()
.expect("Type name contains :: because it is part of a module");
f.debug_struct("Pktsnip")
.field("M", &mode)
.field("length", &self.len())
.field("snips", &self.count())
.finish()
Expand Down
2 changes: 1 addition & 1 deletion src/interrupt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! * Utility functions can disable interrupts (creating critical sections), check whether
//! interrupts are enabled or to determine whether code is executed in a thread or an ISR.
//!
//! * Some functions (eg. [`ZTimer::set_ticks_during`](crate::ztimer::ZTimer::set_ticks_during))
//! * Some functions (eg. [`ZTimer::set_ticks_during`](crate::ztimer::Clock::set_during))
//! take callbacks that will be called in an interrupt context.
//!
//! These are typechecked to be Send, as they are moved from the thread to the interrupt context.
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ pub mod thread;
pub mod gcoap;
#[cfg(riot_module_gnrc)]
pub mod gnrc;
// Note that this can also exist without gnrc
#[cfg(riot_module_gnrc_pktbuf)]
pub mod gnrc_pktbuf;
#[cfg(riot_module_gnrc)]
pub mod gnrc_util;
#[cfg(riot_module_periph_i2c)]
Expand Down
13 changes: 6 additions & 7 deletions src/main_module.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Tools for providing a RIOT main function
//!
//! The main contribution of this module is the [riot_main] macro.
//! The main contribution of this module is the [`riot_main!`](super::riot_main!) macro.
//!
//! The alternative to using that (other than doing it manually) is to have C code along with the
//! Rust application that occupies the main function.
Expand All @@ -10,6 +10,7 @@
//! C code.

use crate::stdio::println;
use crate::thread::{EndToken, StartToken};

// General alternative to this module: Build the extern "C" main all the time and request that the
// application implement a named function. I never got the main function to be carried to the
Expand Down Expand Up @@ -55,9 +56,9 @@ impl<F: Fn() -> T, T: Termination> UsableAsMain<[u8; 1]> for F {
}
}

impl<F: Fn(crate::thread::StartToken) -> crate::never::Never> Sealed<[u8; 2]> for F {}
impl<F: Fn(StartToken) -> crate::never::Never> Sealed<[u8; 2]> for F {}

impl<F: Fn(crate::thread::StartToken) -> crate::never::Never> UsableAsMain<[u8; 2]> for F {
impl<F: Fn(StartToken) -> crate::never::Never> UsableAsMain<[u8; 2]> for F {
unsafe fn call_main(&self) -> i32 {
// unsafe: By construction of the C main function this only happens at startup time
// with a thread that hasn't done anything relevant before.
Expand All @@ -67,11 +68,9 @@ impl<F: Fn(crate::thread::StartToken) -> crate::never::Never> UsableAsMain<[u8;
}
}

impl<F: Fn(crate::thread::StartToken) -> ((), crate::thread::EndToken)> Sealed<[u8; 3]> for F {}
impl<F: Fn(StartToken) -> ((), EndToken)> Sealed<[u8; 3]> for F {}

impl<F: Fn(crate::thread::StartToken) -> ((), crate::thread::EndToken)> UsableAsMain<[u8; 3]>
for F
{
impl<F: Fn(StartToken) -> ((), EndToken)> UsableAsMain<[u8; 3]> for F {
unsafe fn call_main(&self) -> i32 {
// unsafe: By construction of the C main function this only happens at startup time
// with a thread that hasn't done anything relevant before.
Expand Down
2 changes: 1 addition & 1 deletion src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub enum MsgSender {

impl MsgSender {
fn from_pid(pid: kernel_pid_t) -> Self {
if pid == crate::thread::pid_converted::KERNEL_PID_ISR {
if pid == crate::thread::KERNEL_PID_ISR {
MsgSender::ISR
} else {
KernelPID::new(pid)
Expand Down
1 change: 1 addition & 0 deletions src/never.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
///
/// From <https://github.com/rust-lang/rust/issues/43301#issuecomment-912390203>, adjusted for
/// usability with pub interfaces by using a pub trait in a private module (sealing).
#[cfg(not(feature = "actual_never_type"))]
use crate::helpers::*;

#[cfg(not(feature = "actual_never_type"))]
Expand Down
67 changes: 33 additions & 34 deletions src/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,46 +24,45 @@ fn panic(info: &::core::panic::PanicInfo) -> ! {
cstr::cstr!("RUST PANIC").as_ptr() as _,
)
};
unreachable!()
}

// I *guess* it's OK for a panic to simply make a thread into a zombie -- this does allow other
// threads (including spawned Rust threads) to continue, but my layman's understanding of
// panicking is that that's OK because whatever we were just mutating can simply never be used
// by someone else ever again.
} else {
// I *guess* it's OK for a panic to simply make a thread into a zombie -- this does allow other
// threads (including spawned Rust threads) to continue, but my layman's understanding of
// panicking is that that's OK because whatever we were just mutating can simply never be used
// by someone else ever again.

let me = thread::get_pid();
let me = thread::get_pid();

if cfg!(feature = "panic_handler_format") {
use crate::stdio::println;
if cfg!(feature = "panic_handler_format") {
use crate::stdio::println;

println!(
"Error in thread {:?} ({}):",
me,
me.get_name().unwrap_or("unnamed")
);
println!("{}", info);
} else {
let mut stdio = crate::stdio::Stdio {};
use core::fmt::Write;
let _ = stdio.write_str("Panic in thread ");
let _ = stdio.write_str(me.get_name().unwrap_or("unnamed"));
let _ = stdio.write_str("!\n");
}
println!(
"Error in thread {:?} ({}):",
me,
me.get_name().unwrap_or("unnamed")
);
println!("{}", info);
} else {
let mut stdio = crate::stdio::Stdio {};
use core::fmt::Write;
let _ = stdio.write_str("Panic in thread ");
let _ = stdio.write_str(me.get_name().unwrap_or("unnamed"));
let _ = stdio.write_str("!\n");
}

if cfg!(feature = "panic_handler_crash") {
unsafe {
riot_sys::core_panic(
riot_sys::core_panic_t_PANIC_GENERAL_ERROR,
cstr::cstr!("RUST PANIC").as_ptr() as _,
)
if cfg!(feature = "panic_handler_crash") {
unsafe {
riot_sys::core_panic(
riot_sys::core_panic_t_PANIC_GENERAL_ERROR,
cstr::cstr!("RUST PANIC").as_ptr() as _,
)
}
}
}

// Not trying any unwinding -- this thread is just dead, won't be re-claimed, any mutexes it
// holds are just held indefinitely rather than throwing poison errors.
loop {
thread::sleep();
// Not trying any unwinding -- this thread is just dead, won't be re-claimed, any mutexes it
// holds are just held indefinitely rather than throwing poison errors.
loop {
thread::sleep();
}
}
}

Expand Down
13 changes: 5 additions & 8 deletions src/saul.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,8 @@
//!
//! [SAUL]: https://doc.riot-os.org/group__drivers__saul.html

use riot_sys as raw;
use riot_sys::libc;

use crate::error;
use crate::helpers::PointerToCStr;
use crate::Never;
use error::NegativeErrorExt;

pub mod registration;
Expand Down Expand Up @@ -95,10 +91,9 @@ impl RegistryEntry {
unsafe { riot_sys::saul_reg_write(self.0, &value.values as *const _ as *mut _) }
.negative_to_error()?;
if length != value.length.into() {
// FIXME is this the best way to express the error?
Err(error::NumericError {
number: length as isize,
})
// It's not pretty to synthesize an error here, but neither would be expressing the
// partial write in the context of lengthful phydat items.
Err(error::NumericError::from_constant(riot_sys::EINVAL as _))
} else {
Ok(())
}
Expand Down Expand Up @@ -404,8 +399,10 @@ impl Unit {
// we can just switch over before they go.
#[deprecated(note = "Use the GForce variant instead")]
pub const G: Self = Unit::GForce;
#[allow(non_upper_case_globals)]
#[deprecated(note = "Use the Gram variant instead")]
pub const Gr: Self = Unit::Gram;
#[allow(non_upper_case_globals)]
#[deprecated(note = "Use the Gauss variant instead")]
pub const Gs: Self = Unit::Gauss;

Expand Down
7 changes: 4 additions & 3 deletions src/saul/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ pub trait Drivable: Sized {
/// Set to true if `read` is implemented.
///
/// Doing this on the type level (rather than having read and write return a more
/// differentiated error) allows the driver to point to the shared [riot_sys::saul_notsup]
/// handler rather than to monomorphize a custom erring handler for each device.
/// differentiated error) allows the driver to point to the shared [riot_sys::saul_read_notsup]
/// / [riot_sys::saul_write_notsup] handler rather than to monomorphize a custom erring handler
/// for each device.
const HAS_READ: bool = false;
/// Set to true if `write` is implemented.
const HAS_WRITE: bool = false;
Expand All @@ -50,7 +51,7 @@ pub trait Drivable: Sized {
/// Set the state of an actuator, or reconfigure a sensor
///
/// A &self is passed in on write because there could be concurrent access from multiple SAUL
/// users. One option of handling this is to implement Drivable for Mutex<T>.
/// users. One option of handling this is to implement Drivable for `Mutex<T>`.
///
/// Note that due to the way SAUL is structured, the drivable can not know the number of
/// entries which the user intended to set. The Drivable trait always builds the Rust Phydat
Expand Down
20 changes: 11 additions & 9 deletions src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
//!
//! This module can be used in two ways:
//!
//! * Declare static commands using [static_command]; these only take a `fn` (not a closure)
//! because shell commands don't have an arg pointer.
//! * Declare static commands using [`static_command!`](crate::static_command!); these only take a
//! `fn` (not a closure) because shell commands don't have an arg pointer.
//!
//! This works even in RIOT modules that are included in a C application that starts a shell, and
//! show up in shells created through Rust without explicit inclusion.
Expand All @@ -20,8 +20,8 @@
//! argument. This does allow the Rust wrappers to "just so" use a closure as a command handler,
//! but also needs a lot of code.
//!
//! That complexity is not pulled in when only using [static_command] and running on an otherwise
//! empty command list.
//! That complexity is not pulled in when only using [`static_command!`](crate::static_command!)
//! and running on an otherwise empty command list.

use crate::{mutex, stdio};
use core::ffi::CStr;
Expand Down Expand Up @@ -108,11 +108,13 @@ pub unsafe trait CommandListInternals: Sized {
let sleeve = lock
.as_ref()
.expect("Callback called while no shell set up as running");
// unsafe: A suitable callback is always configured. We can make a &mut out of it for as
// long as we hold the lock.
let root = unsafe { &mut *(sleeve.0 as *mut Self) };
let result = root.find_self_and_run(argc, argv, command_index);
drop(root);
let result;
{
// unsafe: A suitable callback is always configured. We can make a &mut out of it for as
// long as we hold the lock.
let root = unsafe { &mut *(sleeve.0 as *mut Self) };
result = root.find_self_and_run(argc, argv, command_index);
}
drop(lock);
result
}
Expand Down
Loading