From 249b65f3467e2b56c9793cb86cb0d32ccd6e24da Mon Sep 17 00:00:00 2001 From: Sebastian Wiesner Date: Tue, 10 Dec 2024 20:09:45 +0100 Subject: [PATCH] Define a boxed type for MacAdd6 This removes all the manual type conversion between glib::Bytes and MacAddr6, which is a lot safer, as there's no need to validate the property value anymore. --- src/app/commandline.rs | 4 +-- src/app/debuginfo.rs | 14 +++++--- src/app/model/device.rs | 36 +++++-------------- src/app/searchprovider.rs | 8 ++--- src/app/storage.rs | 2 +- src/app/widgets/device_row.rs | 2 +- src/app/widgets/edit_device_dialog.rs | 8 ++--- src/net.rs | 2 ++ src/net/macaddr.rs | 51 +++++++++++++++++++++++++++ 9 files changed, 83 insertions(+), 44 deletions(-) create mode 100644 src/net/macaddr.rs diff --git a/src/app/commandline.rs b/src/app/commandline.rs index 9d69788..a2e5d2e 100644 --- a/src/app/commandline.rs +++ b/src/app/commandline.rs @@ -29,7 +29,7 @@ async fn turn_on_device( "option.turn-on-device.message", "Sent magic packet to mac address %1 of device %2\n", ) - .replace("%1", &device.mac_addr6().to_string()) + .replace("%1", &device.mac_address().to_string()) .replace("%2", &device.label()), ); glib::ExitCode::SUCCESS @@ -148,7 +148,7 @@ pub fn list_devices( color, indicator, device.label(), - device.mac_addr6(), + device.mac_address(), device.host() )); } diff --git a/src/app/debuginfo.rs b/src/app/debuginfo.rs index 7efc872..65a0969 100644 --- a/src/app/debuginfo.rs +++ b/src/app/debuginfo.rs @@ -86,11 +86,15 @@ impl DebugInfo { // Give network monitor time to actually figure out what the state of the network is, // especially inside a flatpak sandbox, see https://gitlab.gnome.org/GNOME/glib/-/issues/1718 glib::timeout_future(Duration::from_millis(500)).map(|_| monitor.connectivity()), - std::iter::once(Device::new("localhost", MacAddr6::nil(), "localhost")) - .chain(model.into_iter().map(|d| d.unwrap().downcast().unwrap())) - .map(ping_device) - .collect::>() - .collect::>(), + std::iter::once(Device::new( + "localhost", + MacAddr6::nil().into(), + "localhost", + )) + .chain(model.into_iter().map(|d| d.unwrap().downcast().unwrap())) + .map(ping_device) + .collect::>() + .collect::>(), ) .await; DebugInfo { diff --git a/src/app/model/device.rs b/src/app/model/device.rs index ccf2d40..92690ce 100644 --- a/src/app/model/device.rs +++ b/src/app/model/device.rs @@ -9,45 +9,33 @@ use std::time::Duration; use futures_util::{select_biased, FutureExt}; use gtk::gio::IOErrorEnum; use gtk::glib; -use macaddr::MacAddr6; use crate::config::G_LOG_DOMAIN; -use crate::net::wol; +use crate::net::{wol, MacAddr6Boxed}; glib::wrapper! { pub struct Device(ObjectSubclass); } impl Device { - pub fn new(label: &str, mac_address: MacAddr6, host: &str) -> Self { + pub fn new(label: &str, mac_address: MacAddr6Boxed, host: &str) -> Self { glib::Object::builder() .property("label", label) - .property("mac_address", glib::Bytes::from(mac_address.as_bytes())) + .property("mac_address", mac_address) .property("host", host) .build() } - pub fn mac_addr6(&self) -> MacAddr6 { - // We unwrap, because we try very hard to make sure that mac_address - // contains 6 bytes. - let data: [u8; 6] = (*self.mac_address()).try_into().unwrap(); - MacAddr6::from(data) - } - - pub fn set_mac_addr6(&self, mac_address: MacAddr6) { - self.set_mac_address(glib::Bytes::from(mac_address.as_bytes())); - } - /// Send the magic packet to this device. pub async fn wol(&self) -> Result<(), glib::Error> { - let mac_address = self.mac_addr6(); + let mac_address = self.mac_address(); glib::info!( "Sending magic packet for mac address {mac_address} of device {}", self.label() ); let wol_timeout = Duration::from_secs(5); let result = select_biased! { - result = wol(mac_address).fuse() => result, + result = wol(*mac_address).fuse() => result, _ = glib::timeout_future(wol_timeout).fuse() => { let message = &format!("Failed to send magic packet within {wol_timeout:#?}"); Err(glib::Error::new(IOErrorEnum::TimedOut, message)) @@ -82,7 +70,9 @@ mod imp { use glib::subclass::prelude::*; use gtk::glib; - #[derive(Debug, glib::Properties)] + use crate::net::MacAddr6Boxed; + + #[derive(Debug, Default, glib::Properties)] #[properties(wrapper_type = super::Device)] pub struct Device { /// The human-readable label for this device, for display in the UI. @@ -90,7 +80,7 @@ mod imp { pub label: RefCell, /// The MAC address of the device to wake. #[property(get, set)] - pub mac_address: RefCell, + pub mac_address: RefCell, /// The host name or IP 4/6 address of the device, to check whether it is reachable. #[property(get, set)] pub host: RefCell, @@ -101,14 +91,6 @@ mod imp { const NAME: &'static str = "Device"; type Type = super::Device; - - fn new() -> Self { - Self { - label: Default::default(), - mac_address: RefCell::new(glib::Bytes::from_static(&[0; 6])), - host: Default::default(), - } - } } #[glib::derived_properties] diff --git a/src/app/searchprovider.rs b/src/app/searchprovider.rs index cb56cab..64c9a3f 100644 --- a/src/app/searchprovider.rs +++ b/src/app/searchprovider.rs @@ -85,7 +85,7 @@ async fn activate_result( "search-provider.notification.body", "Failed to send magic packet to mac address %1 of device %2.", ) - .replace("%1", &device.mac_addr6().to_string()) + .replace("%1", &device.mac_address().to_string()) .replace("%2", &device.label()), )); notification.set_priority(NotificationPriority::Urgent); @@ -103,7 +103,7 @@ async fn activate_result( "search-provider.notification.body", "Sent magic packet to mac address %1 of device %2.", ) - .replace("%1", &device.mac_addr6().to_string()) + .replace("%1", &device.mac_address().to_string()) .replace("%2", &device.label()), )); let id = glib::uuid_string_random(); @@ -229,7 +229,7 @@ mod tests { #[test] fn device_matches_terms_case_insensitive() { - let device = Device::new("Server", MacAddr6::nil(), "foo.example.com"); + let device = Device::new("Server", MacAddr6::nil().into(), "foo.example.com"); assert!(matches_terms(&device, &["server"])); assert!(matches_terms(&device, &["SERVER"])); assert!(matches_terms(&device, &["SeRvEr"])); @@ -239,7 +239,7 @@ mod tests { #[test] fn device_matches_terms_in_label_and_host() { - let device = Device::new("Server", MacAddr6::nil(), "foo.example.com"); + let device = Device::new("Server", MacAddr6::nil().into(), "foo.example.com"); assert!(matches_terms(&device, &["Server", "foo"])); } diff --git a/src/app/storage.rs b/src/app/storage.rs index 1d96dd2..0d8e93c 100644 --- a/src/app/storage.rs +++ b/src/app/storage.rs @@ -48,7 +48,7 @@ impl From for StoredDevice { StoredDevice { label: device.label(), host: device.host(), - mac_address: device.mac_addr6(), + mac_address: *device.mac_address(), } } } diff --git a/src/app/widgets/device_row.rs b/src/app/widgets/device_row.rs index bf5355e..26d6541 100644 --- a/src/app/widgets/device_row.rs +++ b/src/app/widgets/device_row.rs @@ -80,7 +80,7 @@ mod imp { impl DeviceRow { #[template_callback] pub fn device_mac_address(_row: &super::DeviceRow, device: &Device) -> String { - device.mac_addr6().to_string() + device.mac_address().to_string() } #[template_callback] diff --git a/src/app/widgets/edit_device_dialog.rs b/src/app/widgets/edit_device_dialog.rs index 3a86202..eb9747e 100644 --- a/src/app/widgets/edit_device_dialog.rs +++ b/src/app/widgets/edit_device_dialog.rs @@ -68,9 +68,9 @@ mod imp { use gtk::glib::Properties; use gtk::CompositeTemplate; use gtk::{glib, template_callbacks}; - use macaddr::MacAddr6; use crate::app::model::Device; + use crate::net::MacAddr6Boxed; use super::super::ValidationIndicator; @@ -203,12 +203,12 @@ mod imp { klass.install_action("device.save", None, |dialog, _, _| { if dialog.is_valid() { // At this point we know that the MAC address is valid, hence we can unwrap - let mac_address = MacAddr6::from_str(&dialog.mac_address()).unwrap(); + let mac_address = MacAddr6Boxed::from_str(&dialog.mac_address()).unwrap(); let device = match dialog.device() { Some(device) => { // The dialog edits an existing device, so update its fields. device.set_label(dialog.label()); - device.set_mac_addr6(mac_address); + device.set_mac_address(mac_address); device.set_host(dialog.host()); device } @@ -245,7 +245,7 @@ mod imp { if let Some(device) = self.obj().device() { // Initialize properties from device self.obj().set_label(device.label()); - self.obj().set_mac_address(device.mac_addr6().to_string()); + self.obj().set_mac_address(device.mac_address().to_string()); self.obj().set_host(device.host()); } // After initialization, update validation status. diff --git a/src/net.rs b/src/net.rs index fdf6079..d01c6ce 100644 --- a/src/net.rs +++ b/src/net.rs @@ -8,10 +8,12 @@ //! //! Contains a dead simple and somewhat inefficient ping implementation. +mod macaddr; mod monitor; mod ping; mod wol; +pub use macaddr::MacAddr6Boxed; pub use monitor::monitor; pub use ping::{ping_address_with_timeout, PingDestination}; pub use wol::wol; diff --git a/src/net/macaddr.rs b/src/net/macaddr.rs new file mode 100644 index 0000000..787f621 --- /dev/null +++ b/src/net/macaddr.rs @@ -0,0 +1,51 @@ +// Copyright Sebastian Wiesner + +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +//! Simple MAC address type on top of [`glib::Bytes`]. +//! +//! While this is not the most efficient approach it allows storing the MAC +//! address as a GLib property. + +use std::fmt::Display; +use std::ops::Deref; +use std::str::FromStr; + +use macaddr::MacAddr6; + +/// Boxed [`MacAddr6`]. +/// +/// Define a MAC address type for GLib, by boxing a [`MacAdd6`]. +#[derive(Default, Debug, Copy, Clone, Eq, PartialEq, PartialOrd, Ord, glib::Boxed)] +#[boxed_type(name = "MacAdd6")] +pub struct MacAddr6Boxed(MacAddr6); + +impl From for MacAddr6Boxed { + fn from(value: MacAddr6) -> Self { + Self(value) + } +} + +impl FromStr for MacAddr6Boxed { + type Err = macaddr::ParseError; + + fn from_str(s: &str) -> Result { + MacAddr6::from_str(s).map(Into::into) + } +} + +impl Deref for MacAddr6Boxed { + type Target = MacAddr6; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl Display for MacAddr6Boxed { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +}