Skip to content

Commit

Permalink
Define a boxed type for MacAdd6
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
swsnr committed Dec 10, 2024
1 parent 5041229 commit 249b65f
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/app/commandline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn list_devices(
color,
indicator,
device.label(),
device.mac_addr6(),
device.mac_address(),
device.host()
));
}
Expand Down
14 changes: 9 additions & 5 deletions src/app/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<FuturesOrdered<_>>()
.collect::<Vec<_>>(),
std::iter::once(Device::new(
"localhost",
MacAddr6::nil().into(),
"localhost",
))
.chain(model.into_iter().map(|d| d.unwrap().downcast().unwrap()))
.map(ping_device)
.collect::<FuturesOrdered<_>>()
.collect::<Vec<_>>(),
)
.await;
DebugInfo {
Expand Down
36 changes: 9 additions & 27 deletions src/app/model/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<imp::Device>);
}

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))
Expand Down Expand Up @@ -82,15 +70,17 @@ 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.
#[property(get, set)]
pub label: RefCell<String>,
/// The MAC address of the device to wake.
#[property(get, set)]
pub mac_address: RefCell<glib::Bytes>,
pub mac_address: RefCell<MacAddr6Boxed>,
/// The host name or IP 4/6 address of the device, to check whether it is reachable.
#[property(get, set)]
pub host: RefCell<String>,
Expand All @@ -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]
Expand Down
8 changes: 4 additions & 4 deletions src/app/searchprovider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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"]));
Expand All @@ -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"]));
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl From<Device> for StoredDevice {
StoredDevice {
label: device.label(),
host: device.host(),
mac_address: device.mac_addr6(),
mac_address: *device.mac_address(),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/widgets/device_row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
8 changes: 4 additions & 4 deletions src/app/widgets/edit_device_dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
51 changes: 51 additions & 0 deletions src/net/macaddr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright Sebastian Wiesner <[email protected]>

// 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<MacAddr6> for MacAddr6Boxed {
fn from(value: MacAddr6) -> Self {
Self(value)
}
}

impl FromStr for MacAddr6Boxed {
type Err = macaddr::ParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
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)
}
}

0 comments on commit 249b65f

Please sign in to comment.