Merge pull request #15 from windy1/refactor/cleanup

Minor cleanup / refactoring

.github/workflows/rust.yml

@@ -2,9 +2,9 @@ name: Rust
 
 on:
   push:
-    branches: [ master ]
+    branches: [master]
   pull_request:
-    branches: [ master ]
+    branches: [master]
 
 env:
   CARGO_TERM_COLOR: always
@@ -14,8 +14,14 @@ jobs:
     runs-on: macos-latest
     steps:
       - uses: actions/checkout@v2
+      - name: Prepare
+        run: rustup component add clippy
       - name: Build
-      run: cargo build --verbose
+        run: ./scripts/buildall.sh
+      - name: Check formatting
+        run: ./scripts/checkfmt.sh
+      - name: Run clippy
+        run: ./scripts/lintall.sh
       - name: Run tests
         run: cargo test --verbose
 
@@ -24,8 +30,12 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       - name: Prepare
-      run: "sudo apt -y install avahi-daemon libavahi-client-dev && sudo systemctl start avahi-daemon.service"
+        run: "rustup component add clippy && sudo apt -y install avahi-daemon libavahi-client-dev && sudo systemctl start avahi-daemon.service"
       - name: Build
-      run: cargo build --verbose
+        run: ./scripts/buildall.sh
+      - name: Check formatting
+        run: ./scripts/checkfmt.sh
+      - name: Run clippy
+        run: ./scripts/lintall.sh
       - name: Run tests
         run: cargo test --verbose

scripts/buildall.sh

@@ -0,0 +1,9 @@
+#!/bin/bash
+
+set -e
+
+cargo build --workspace --verbose
+(
+    cd examples
+    cargo build --workspace --verbose
+)

scripts/checkfmt.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+
+set -e
+
+find examples/browser/src examples/service/src zeroconf/src zeroconf-macros/src -type f -name *.rs -print0 | xargs -0 -n1 rustfmt --check --verbose

scripts/lintall.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+cargo clippy -- -D warnings
+cargo clippy --all-targets --all-features -- -D warnings
+(
+    cd examples
+    cargo clippy -- -D warnings
+    cargo clippy --all-targets --all-features -- -D warnings
+)

zeroconf/src/ffi/mod.rs

@@ -1,13 +1,9 @@
 //! Utilities related to FFI bindings
 
-use crate::Result;
+use libc::c_void;
-#[cfg(target_os = "linux")]
+use std::ptr;
-use libc::{c_char, in_addr, sockaddr_in};
-use libc::{c_void, fd_set, suseconds_t, time_t, timeval};
-use std::time::Duration;
-use std::{mem, ptr};
 
-pub mod c_str;
+pub(crate) mod c_str;
 
 /// Helper trait to convert a raw `*mut c_void` to it's rust type
 pub trait FromRaw<T> {
@@ -43,36 +39,6 @@ pub trait AsRaw {
     }
 }
 
-/// Performs a unix `select()` on the specified `sock_fd` and `timeout`. Returns the select result
-/// or `Err` if the result is negative.
-///
-/// # Safety
-/// This function is unsafe because it directly interfaces with C-library system calls.
-pub unsafe fn read_select(sock_fd: i32, timeout: Duration) -> Result<u32> {
-    let mut read_flags: fd_set = mem::zeroed();
-
-    libc::FD_ZERO(&mut read_flags);
-    libc::FD_SET(sock_fd, &mut read_flags);
-
-    let tv_sec = timeout.as_secs() as time_t;
-    let tv_usec = timeout.subsec_micros() as suseconds_t;
-    let mut timeout = timeval { tv_sec, tv_usec };
-
-    let result = libc::select(
-        sock_fd + 1,
-        &mut read_flags,
-        ptr::null_mut(),
-        ptr::null_mut(),
-        &mut timeout,
-    );
-
-    if result < 0 {
-        Err("select(): returned error status".into())
-    } else {
-        Ok(result as u32)
-    }
-}
-
 /// Helper trait to unwrap a type to a `*const T` or a null-pointer if not present.
 pub trait UnwrapOrNull<T> {
     /// Unwraps this type to `*const T` or `ptr::null()` if not present.
@@ -97,18 +63,40 @@ impl<T> UnwrapMutOrNull<T> for Option<*mut T> {
     }
 }
 
-/// Returns a human-readable address of the specified raw address
+#[cfg(target_vendor = "apple")]
-///
+pub(crate) mod macos {
-/// # Safety
+    use crate::Result;
-/// This function is unsafe because of calls to C-library system calls
+    use libc::{fd_set, suseconds_t, time_t, timeval};
-#[cfg(target_os = "linux")]
+    use std::time::Duration;
-pub unsafe fn get_ip(address: *const sockaddr_in) -> String {
+    use std::{mem, ptr};
-    assert_not_null!(address);
-    let raw = inet_ntoa(&(*address).sin_addr as *const in_addr);
-    String::from(c_str::raw_to_str(raw))
-}
 
-#[cfg(target_os = "linux")]
+    /// Performs a unix `select()` on the specified `sock_fd` and `timeout`. Returns the select result
-extern "C" {
+    /// or `Err` if the result is negative.
-    fn inet_ntoa(addr: *const in_addr) -> *const c_char;
+    ///
+    /// # Safety
+    /// This function is unsafe because it directly interfaces with C-library system calls.
+    pub unsafe fn read_select(sock_fd: i32, timeout: Duration) -> Result<u32> {
+        let mut read_flags: fd_set = mem::zeroed();
+
+        libc::FD_ZERO(&mut read_flags);
+        libc::FD_SET(sock_fd, &mut read_flags);
+
+        let tv_sec = timeout.as_secs() as time_t;
+        let tv_usec = timeout.subsec_micros() as suseconds_t;
+        let mut timeout = timeval { tv_sec, tv_usec };
+
+        let result = libc::select(
+            sock_fd + 1,
+            &mut read_flags,
+            ptr::null_mut(),
+            ptr::null_mut(),
+            &mut timeout,
+        );
+
+        if result < 0 {
+            Err("select(): returned error status".into())
+        } else {
+            Ok(result as u32)
+        }
+    }
 }

zeroconf/src/lib.rs

@@ -133,17 +133,17 @@ extern crate maplit;
 
 #[macro_use]
 mod macros;
+mod ffi;
 mod interface;
+mod service_type;
 #[cfg(test)]
 mod tests;
 
 pub mod browser;
 pub mod error;
 pub mod event_loop;
-pub mod ffi;
 pub mod prelude;
 pub mod service;
-pub mod service_type;
 pub mod txt_record;
 
 #[cfg(target_os = "linux")]

zeroconf/src/linux/avahi_util.rs

@@ -1,6 +1,5 @@
 //! Utilities related to Avahi
 
-use super::constants;
 use crate::NetworkInterface;
 use avahi_sys::{avahi_address_snprint, avahi_strerror, AvahiAddress};
 use libc::c_char;
@@ -16,11 +15,11 @@ use std::ffi::CStr;
 pub unsafe fn avahi_address_to_string(addr: *const AvahiAddress) -> String {
     assert_not_null!(addr);
 
-    let addr_str = c_string!(alloc(constants::AVAHI_ADDRESS_STR_MAX));
+    let addr_str = c_string!(alloc(avahi_sys::AVAHI_ADDRESS_STR_MAX as usize));
 
     avahi_address_snprint(
         addr_str.as_ptr() as *mut c_char,
-        constants::AVAHI_ADDRESS_STR_MAX,
+        avahi_sys::AVAHI_ADDRESS_STR_MAX as usize,
         addr,
     );
 
@@ -43,7 +42,7 @@ pub fn get_error<'a>(code: i32) -> &'a str {
 /// [`NetworkInterface`]: ../../enum.NetworkInterface.html
 pub fn interface_index(interface: NetworkInterface) -> i32 {
     match interface {
-        NetworkInterface::Unspec => constants::AVAHI_IF_UNSPEC,
+        NetworkInterface::Unspec => avahi_sys::AVAHI_IF_UNSPEC,
         NetworkInterface::AtIndex(i) => i as i32,
     }
 }

zeroconf/src/linux/browser.rs

@@ -2,7 +2,6 @@
 
 use super::avahi_util;
 use super::client::{ManagedAvahiClient, ManagedAvahiClientParams};
-use super::constants;
 use super::poll::ManagedAvahiSimplePoll;
 use super::raw_browser::{ManagedAvahiServiceBrowser, ManagedAvahiServiceBrowserParams};
 use super::{
@@ -48,7 +47,7 @@ impl TMdnsBrowser for AvahiMdnsBrowser {
             browser: None,
             kind: c_string!(service_type.to_string()),
             context: Box::into_raw(Box::default()),
-            interface_index: constants::AVAHI_IF_UNSPEC,
+            interface_index: avahi_sys::AVAHI_IF_UNSPEC,
         }
     }
 
@@ -86,9 +85,9 @@ impl TMdnsBrowser for AvahiMdnsBrowser {
 
             self.browser = Some(ManagedAvahiServiceBrowser::new(
                 ManagedAvahiServiceBrowserParams::builder()
-                    .client(&(*self.context).client.as_ref().unwrap())
+                    .client((*self.context).client.as_ref().unwrap())
                     .interface(self.interface_index)
-                    .protocol(constants::AVAHI_PROTO_UNSPEC)
+                    .protocol(avahi_sys::AVAHI_PROTO_UNSPEC)
                     .kind(self.kind.as_ptr())
                     .domain(ptr::null_mut())
                     .flags(0)
@@ -123,7 +122,7 @@ impl AvahiBrowserContext {
         if let Some(f) = &self.service_discovered_callback {
             f(result, self.user_context.clone());
         } else {
-            warn!("attempted to invoke callback but none was set");
+            panic!("attempted to invoke browser callback but none was set");
         }
     }
 }
@@ -191,7 +190,7 @@ fn handle_browser_new(
             .name(name)
             .kind(kind)
             .domain(domain)
-            .aprotocol(constants::AVAHI_PROTO_UNSPEC)
+            .aprotocol(avahi_sys::AVAHI_PROTO_UNSPEC)
             .flags(0)
             .callback(Some(resolve_callback))
             .userdata(raw_context)

zeroconf/src/linux/client.rs

@@ -15,7 +15,7 @@ use libc::{c_int, c_void};
 /// This struct allocates a new `*mut AvahiClient` when `ManagedAvahiClient::new()` is invoked and
 /// calls the Avahi function responsible for freeing the client on `trait Drop`.
 #[derive(Debug)]
-pub struct ManagedAvahiClient(pub(super) *mut AvahiClient);
+pub struct ManagedAvahiClient(*mut AvahiClient);
 
 impl ManagedAvahiClient {
     /// Initializes the underlying `*mut AvahiClient` and verifies it was created; returning
@@ -32,7 +32,7 @@ impl ManagedAvahiClient {
 
         let client = unsafe {
             avahi_client_new(
-                avahi_simple_poll_get(poll.0),
+                avahi_simple_poll_get(poll.inner()),
                 flags,
                 callback,
                 userdata,
@@ -60,6 +60,10 @@ impl ManagedAvahiClient {
     pub fn host_name<'a>(&self) -> Result<&'a str> {
         unsafe { get_host_name(self.0) }
     }
+
+    pub(super) fn inner(&self) -> *mut AvahiClient {
+        self.0
+    }
 }
 
 impl Drop for ManagedAvahiClient {

zeroconf/src/linux/constants.rs

@@ -1,5 +0,0 @@
-use avahi_sys::{AvahiIfIndex, AvahiProtocol};
-
-pub const AVAHI_IF_UNSPEC: AvahiIfIndex = -1;
-pub const AVAHI_PROTO_UNSPEC: AvahiProtocol = -1;
-pub const AVAHI_ADDRESS_STR_MAX: usize = 40;

zeroconf/src/linux/entry_group.rs

@@ -72,7 +72,7 @@ impl ManagedAvahiEntryGroup {
                 domain,
                 host,
                 port,
-                txt.map(|t| t.0).unwrap_mut_or_null()
+                txt.map(|t| t.inner()).unwrap_mut_or_null()
             ),
             "could not register service"
         )?;

zeroconf/src/linux/mod.rs

@@ -6,8 +6,6 @@
 //! [Bonjour]: https://en.wikipedia.org/wiki/Bonjour_(software)
 //! [Avahi]: https://en.wikipedia.org/wiki/Avahi_(software)
 
-pub(crate) mod constants;
-
 pub mod avahi_util;
 pub mod browser;
 pub mod client;

zeroconf/src/linux/poll.rs

@@ -11,7 +11,7 @@ use avahi_sys::{
 /// This struct allocates a new `*mut AvahiSimplePoll` when `ManagedAvahiClient::new()` is invoked
 /// and calls the Avahi function responsible for freeing the poll on `trait Drop`.
 #[derive(Debug)]
-pub struct ManagedAvahiSimplePoll(pub(super) *mut AvahiSimplePoll);
+pub struct ManagedAvahiSimplePoll(*mut AvahiSimplePoll);
 
 impl ManagedAvahiSimplePoll {
     /// Initializes the underlying `*mut AvahiSimplePoll` and verifies it was created; returning
@@ -41,6 +41,10 @@ impl ManagedAvahiSimplePoll {
     pub fn iterate(&self, sleep_time: i32) {
         unsafe { avahi_simple_poll_iterate(self.0, sleep_time) };
     }
+
+    pub(super) fn inner(&self) -> *mut AvahiSimplePoll {
+        self.0
+    }
 }
 
 impl Drop for ManagedAvahiSimplePoll {

zeroconf/src/linux/raw_browser.rs

@@ -32,7 +32,14 @@ impl ManagedAvahiServiceBrowser {
     ) -> Result<Self> {
         let browser = unsafe {
             avahi_service_browser_new(
-                client.0, interface, protocol, kind, domain, flags, callback, userdata,
+                client.inner(),
+                interface,
+                protocol,
+                kind,
+                domain,
+                flags,
+                callback,
+                userdata,
             )
         };
 

zeroconf/src/linux/resolver.rs

@@ -36,7 +36,15 @@ impl ManagedAvahiServiceResolver {
     ) -> Result<Self> {
         let resolver = unsafe {
             avahi_service_resolver_new(
-                client.0, interface, protocol, name, kind, domain, aprotocol, flags, callback,
+                client.inner(),
+                interface,
+                protocol,
+                name,
+                kind,
+                domain,
+                aprotocol,
+                flags,
+                callback,
                 userdata,
             )
         };

zeroconf/src/linux/service.rs

@@ -2,7 +2,6 @@
 
 use super::avahi_util;
 use super::client::{self, ManagedAvahiClient, ManagedAvahiClientParams};
-use super::constants;
 use super::entry_group::{AddServiceParams, ManagedAvahiEntryGroup, ManagedAvahiEntryGroupParams};
 use super::poll::ManagedAvahiSimplePoll;
 use crate::ffi::{c_str, AsRaw, FromRaw, UnwrapOrNull};
@@ -121,7 +120,7 @@ impl AvahiServiceContext {
             port,
             group: None,
             txt_record: None,
-            interface_index: constants::AVAHI_IF_UNSPEC,
+            interface_index: avahi_sys::AVAHI_IF_UNSPEC,
             domain: None,
             host: None,
             registered_callback: None,
@@ -133,7 +132,7 @@ impl AvahiServiceContext {
         if let Some(f) = &self.registered_callback {
             f(result, self.user_context.clone());
         } else {
-            warn!("attempted to invoke callback but none was set");
+            panic!("attempted to invoke service callback but none was set");
         }
     }
 }
@@ -203,7 +202,7 @@ unsafe fn create_service(
         group.add_service(
             AddServiceParams::builder()
                 .interface(context.interface_index)
-                .protocol(constants::AVAHI_PROTO_UNSPEC)
+                .protocol(avahi_sys::AVAHI_PROTO_UNSPEC)
                 .flags(0)
                 .name(context.name.as_ref().unwrap().as_ptr())
                 .kind(context.kind.as_ptr())

zeroconf/src/linux/string_list.rs

@@ -15,7 +15,7 @@ use std::ptr;
 ///
 /// `zeroconf::TxtRecord` provides the cross-platform bindings for this functionality.
 #[derive(Debug)]
-pub struct ManagedAvahiStringList(pub(crate) *mut AvahiStringList);
+pub struct ManagedAvahiStringList(*mut AvahiStringList);
 
 impl ManagedAvahiStringList {
     /// Creates a new empty TXT record
@@ -69,9 +69,13 @@ impl ManagedAvahiStringList {
         AvahiStringListNode::new(self.0)
     }
 
-    pub(crate) fn clone_raw(raw: *mut AvahiStringList) -> Self {
+    pub(super) fn clone_raw(raw: *mut AvahiStringList) -> Self {
         Self(unsafe { avahi_string_list_copy(raw) })
     }
+
+    pub(super) fn inner(&self) -> *mut AvahiStringList {
+        self.0
+    }
 }
 
 impl Clone for ManagedAvahiStringList {

zeroconf/src/linux/txt_record.rs

@@ -6,7 +6,6 @@ use crate::Result;
 use libc::c_char;
 use std::cell::UnsafeCell;
 
-#[derive(Debug)]
 pub struct AvahiTxtRecord(UnsafeCell<ManagedAvahiStringList>);
 
 impl TTxtRecord for AvahiTxtRecord {

zeroconf/src/macos/event_loop.rs

@@ -21,7 +21,7 @@ impl<'a> TEventLoop for BonjourEventLoop<'a> {
     /// new data, the blocking call is not made.
     fn poll(&self, timeout: Duration) -> Result<()> {
         let service = self.service.lock().unwrap();
-        let select = unsafe { ffi::read_select(service.sock_fd(), timeout)? };
+        let select = unsafe { ffi::macos::read_select(service.sock_fd(), timeout)? };
         if select > 0 {
             service.process_result()
         } else {

zeroconf/src/macos/service.rs

@@ -80,13 +80,13 @@ impl TMdnsService for BonjourMdnsService {
         let txt_len = self
             .txt_record
             .as_ref()
-            .map(|t| t.0.get_length())
+            .map(|t| t.inner().get_length())
             .unwrap_or(0);
 
         let txt_record = self
             .txt_record
             .as_ref()
-            .map(|t| t.0.get_bytes_ptr())
+            .map(|t| t.inner().get_bytes_ptr())
             .unwrap_or_null();
 
         self.service.lock().unwrap().register_service(

zeroconf/src/macos/txt_record.rs

@@ -9,8 +9,8 @@ use std::ffi::CString;
 use std::{mem, ptr};
 
 /// Interface for interfacting with Bonjour's TXT record capabilities.
-#[derive(Debug, Clone)]
+#[derive(Clone)]
-pub struct BonjourTxtRecord(pub(crate) ManagedTXTRecordRef);
+pub struct BonjourTxtRecord(ManagedTXTRecordRef);
 
 impl TTxtRecord for BonjourTxtRecord {
     fn new() -> Self {
@@ -76,6 +76,12 @@ impl TTxtRecord for BonjourTxtRecord {
     }
 }
 
+impl BonjourTxtRecord {
+    pub(super) fn inner(&self) -> &ManagedTXTRecordRef {
+        &self.0
+    }
+}
+
 impl From<ManagedTXTRecordRef> for BonjourTxtRecord {
     fn from(txt: ManagedTXTRecordRef) -> Self {
         Self(txt)

zeroconf/src/service_type.rs

@@ -36,9 +36,9 @@ impl ServiceType {
     }
 
     fn check_part(part: &str) -> Result<&str> {
-        if part.contains(".") {
+        if part.contains('.') {
             Err("invalid character: .".into())
-        } else if part.contains(",") {
+        } else if part.contains(',') {
             Err("invalid character: ,".into())
         } else if part.is_empty() {
             Err("cannot be empty".into())
@@ -48,8 +48,8 @@ impl ServiceType {
     }
 
     fn lstrip_underscore(s: &str) -> &str {
-        if s.starts_with("_") {
+        if let Some(stripped) = s.strip_prefix('_') {
-            &s[1..]
+            stripped
         } else {
             s
         }
@@ -75,12 +75,12 @@ impl FromStr for ServiceType {
     type Err = crate::error::Error;
 
     fn from_str(s: &str) -> Result<Self> {
-        let parts: Vec<&str> = s.split(",").collect();
+        let parts: Vec<&str> = s.split(',').collect();
         if parts.is_empty() {
             return Err("could not parse ServiceType from string".into());
         }
 
-        let head: Vec<&str> = parts[0].split(".").collect();
+        let head: Vec<&str> = parts[0].split('.').collect();
         if head.len() != 2 {
             return Err("invalid name and protocol".into());
         }
@@ -90,12 +90,12 @@ impl FromStr for ServiceType {
 
         let mut sub_types: Vec<&str> = vec![];
         if parts.len() > 1 {
-            for i in 1..parts.len() {
+            for part in parts.iter().skip(1) {
-                sub_types.push(Self::lstrip_underscore(parts[i]));
+                sub_types.push(Self::lstrip_underscore(part));
             }
         }
 
-        Ok(ServiceType::with_sub_types(name, protocol, sub_types)?)
+        ServiceType::with_sub_types(name, protocol, sub_types)
     }
 }
 
@@ -105,17 +105,17 @@ mod tests {
 
     #[test]
     fn new_invalid() {
-        ServiceType::new(".http", "tcp").expect_err("invalid character: .".into());
+        ServiceType::new(".http", "tcp").expect_err("invalid character: .");
-        ServiceType::new("http", ".tcp").expect_err("invalid character: .".into());
+        ServiceType::new("http", ".tcp").expect_err("invalid character: .");
-        ServiceType::new(",http", "tcp").expect_err("invalid character: ,".into());
+        ServiceType::new(",http", "tcp").expect_err("invalid character: ,");
-        ServiceType::new("http", ",tcp").expect_err("invalid character: ,".into());
+        ServiceType::new("http", ",tcp").expect_err("invalid character: ,");
-        ServiceType::new("", "tcp").expect_err("cannot be empty".into());
+        ServiceType::new("", "tcp").expect_err("cannot be empty");
-        ServiceType::new("http", "").expect_err("cannot be empty".into());
+        ServiceType::new("http", "").expect_err("cannot be empty");
     }
 
     #[test]
     fn must_have_name_and_protocol() {
-        ServiceType::from_str("_http").expect_err("invalid name and protocol".into());
+        ServiceType::from_str("_http").expect_err("invalid name and protocol");
     }
 
     #[test]

zeroconf/src/tests/mod.rs

@@ -3,7 +3,7 @@ use std::sync::Once;
 static INIT: Once = Once::new();
 
 pub(crate) fn setup() {
-    INIT.call_once(|| env_logger::init());
+    INIT.call_once(env_logger::init);
 }
 
 mod service_test;

zeroconf/src/txt_record.rs

@@ -5,11 +5,11 @@ use serde::de::{MapAccess, Visitor};
 use serde::ser::SerializeMap;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::collections::HashMap;
-use std::fmt;
+use std::fmt::{self, Debug};
 use std::marker::PhantomData;
 
 /// Interface for interacting with underlying mDNS implementation TXT record capabilities
-pub trait TTxtRecord: Clone + PartialEq + Eq {
+pub trait TTxtRecord: Clone + PartialEq + Eq + Debug {
     /// Constructs a new TXT record
     fn new() -> Self;
 
@@ -130,6 +130,14 @@ impl<'de> Deserialize<'de> for TxtRecord {
     }
 }
 
+impl Debug for TxtRecord {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("TxtRecord")
+            .field("data", &self.to_map())
+            .finish()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;