Skip to content

Commit

Permalink
Normalize consistency of unsafe in function contracts (#59)
Browse files Browse the repository at this point in the history
* normalize consistency of unsafe usage across bonjour-level interfaces

Signed-off-by: Walker Crouse <walkercrouse@hotmail.com>

* normalize consistency of unsafe usage across avahi-level interfaces

Signed-off-by: Walker Crouse <walkercrouse@hotmail.com>

---------

Signed-off-by: Walker Crouse <walkercrouse@hotmail.com>
  • Loading branch information
windy1 authored Aug 12, 2024
1 parent 8da0051 commit 2e5ee10
Show file tree
Hide file tree
Showing 18 changed files with 356 additions and 268 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
such as [Bonjour] or [Avahi], providing an easy and idiomatic way to both register and
browse services.

Check-out [zeroconf-tokio](https://github.com/windy1/zeroconf-tokio) if you are interested in using `zeroconf` in an
async setting.

## Prerequisites

On Linux:
Expand Down
35 changes: 23 additions & 12 deletions zeroconf/src/avahi/avahi_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ pub unsafe fn avahi_address_to_string(addr: *const AvahiAddress) -> String {
}

/// Returns the `&str` message associated with the specified error code.
pub fn get_error<'a>(code: i32) -> &'a str {
unsafe {
CStr::from_ptr(avahi_strerror(code))
.to_str()
.expect("could not fetch Avahi error string")
}
///
/// # Safety
/// This function is unsafe because of internal Avahi calls.
pub unsafe fn get_error<'a>(code: i32) -> &'a str {
CStr::from_ptr(avahi_strerror(code))
.to_str()
.expect("could not fetch Avahi error string")
}

/// Returns the last error message associated with the specified `*mut AvahiClient`.
Expand Down Expand Up @@ -69,7 +70,10 @@ pub fn interface_from_index(index: i32) -> NetworkInterface {
}

/// Executes the specified closure and returns a formatted `Result`
pub fn sys_exec<F: FnOnce() -> i32>(func: F, message: &str) -> Result<()> {
///
/// # Safety
/// This function is unsafe because of the call to `get_error`.
pub unsafe fn sys_exec<F: FnOnce() -> i32>(func: F, message: &str) -> Result<()> {
let err = func();

if err < 0 {
Expand Down Expand Up @@ -110,8 +114,12 @@ pub fn format_sub_type(sub_type: &str, kind: &str) -> String {
)
}

pub fn alternative_service_name(name: &CStr) -> &CStr {
unsafe { CStr::from_ptr(avahi_alternative_service_name(name.as_ptr())) }
/// Returns an alternative service name for the specified `CStr`
///
/// # Safety
/// This function is unsafe because of the call to `avahi_alternative_service_name`.
pub unsafe fn alternative_service_name(name: &CStr) -> &CStr {
CStr::from_ptr(avahi_alternative_service_name(name.as_ptr()))
}

#[cfg(test)]
Expand All @@ -124,13 +132,13 @@ mod tests {

#[test]
fn sys_exec_returns_ok_for_success() {
assert!(sys_exec(|| 0, "test").is_ok());
assert!(unsafe { sys_exec(|| 0, "test") }.is_ok());
}

#[test]
fn sys_exec_returns_error_for_failure() {
assert_eq!(
sys_exec(|| avahi_sys::AVAHI_ERR_FAILURE, "uh oh spaghetti-o"),
unsafe { sys_exec(|| avahi_sys::AVAHI_ERR_FAILURE, "uh oh spaghetti-o") },
Err("uh oh spaghetti-o: `Operation failed`".into())
);
}
Expand Down Expand Up @@ -202,7 +210,10 @@ mod tests {

#[test]
fn get_error_returns_valid_error_string() {
assert_eq!(get_error(avahi_sys::AVAHI_ERR_FAILURE), "Operation failed");
assert_eq!(
unsafe { get_error(avahi_sys::AVAHI_ERR_FAILURE) },
"Operation failed"
);
}

#[test]
Expand Down
31 changes: 16 additions & 15 deletions zeroconf/src/avahi/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,22 @@ impl TMdnsBrowser for AvahiMdnsBrowser {
fn browse_services(&mut self) -> Result<EventLoop> {
debug!("Browsing services: {:?}", self);

self.poll = Some(Arc::new(ManagedAvahiSimplePoll::new()?));

self.client = Some(Arc::new(ManagedAvahiClient::new(
ManagedAvahiClientParams::builder()
.poll(
self.poll
.as_ref()
.ok_or("could not get poll as ref")?
.clone(),
)
.flags(AvahiClientFlags(0))
.callback(Some(client_callback))
.userdata(self.context.as_raw())
.build()?,
)?));
self.poll = Some(Arc::new(unsafe { ManagedAvahiSimplePoll::new() }?));

let poll = self
.poll
.as_ref()
.ok_or("could not get poll as ref")?
.clone();

let client_params = ManagedAvahiClientParams::builder()
.poll(poll)
.flags(AvahiClientFlags(0))
.callback(Some(client_callback))
.userdata(self.context.as_raw())
.build()?;

self.client = Some(Arc::new(unsafe { ManagedAvahiClient::new(client_params) }?));

self.context.client.clone_from(&self.client);

Expand Down
28 changes: 16 additions & 12 deletions zeroconf/src/avahi/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ pub struct ManagedAvahiClient {
impl ManagedAvahiClient {
/// Initializes the underlying `*mut AvahiClient` and verifies it was created; returning
/// `Err(String)` if unsuccessful.
pub fn new(
///
/// # Safety
/// This function is unsafe because of the raw pointer dereference.
pub unsafe fn new(
ManagedAvahiClientParams {
poll,
flags,
Expand All @@ -34,15 +37,13 @@ impl ManagedAvahiClient {
) -> Result<Self> {
let mut err: c_int = 0;

let inner = unsafe {
avahi_client_new(
avahi_simple_poll_get(poll.inner()),
flags,
callback,
userdata,
&mut err,
)
};
let inner = avahi_client_new(
avahi_simple_poll_get(poll.inner()),
flags,
callback,
userdata,
&mut err,
);

if inner.is_null() {
return Err("could not initialize AvahiClient".into());
Expand All @@ -61,8 +62,11 @@ impl ManagedAvahiClient {
/// Delegate function for [`avahi_client_get_host_name()`].
///
/// [`avahi_client_get_host_name()`]: https://avahi.org/doxygen/html/client_8h.html#a89378618c3c592a255551c308ba300bf
pub fn host_name<'a>(&self) -> Result<&'a str> {
unsafe { get_host_name(self.inner) }
///
/// # Safety
/// This function is unsafe because of the raw pointer dereference.
pub unsafe fn host_name<'a>(&self) -> Result<&'a str> {
get_host_name(self.inner)
}
}

Expand Down
44 changes: 31 additions & 13 deletions zeroconf/src/avahi/entry_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,20 @@ pub struct ManagedAvahiEntryGroup {
impl ManagedAvahiEntryGroup {
/// Initializes the underlying `*mut AvahiEntryGroup` and verifies it was created; returning
/// `Err(String)` if unsuccessful.
pub fn new(
///
/// # Safety
/// This function is unsafe because of the raw pointer dereference.
pub unsafe fn new(
ManagedAvahiEntryGroupParams {
client,
callback,
userdata,
}: ManagedAvahiEntryGroupParams,
) -> Result<Self> {
let inner = unsafe { avahi_entry_group_new(client.inner, callback, userdata) };
let inner = avahi_entry_group_new(client.inner, callback, userdata);

if inner.is_null() {
let err = avahi_util::get_error(unsafe { avahi_client_errno(client.inner) });
let err = avahi_util::get_error(avahi_client_errno(client.inner));
Err(format!("could not initialize AvahiEntryGroup: {}", err).into())
} else {
Ok(Self {
Expand All @@ -50,16 +53,22 @@ impl ManagedAvahiEntryGroup {
/// Delegate function for [`avahi_entry_group_is_empty()`].
///
/// [`avahi_entry_group_is_empty()`]: https://avahi.org/doxygen/html/publish_8h.html#af5a78ee1fda6678970536889d459d85c
pub fn is_empty(&self) -> bool {
unsafe { avahi_entry_group_is_empty(self.inner) != 0 }
///
/// # Safety
/// This function is unsafe because of the call to `avahi_entry_group_is_empty()`.
pub unsafe fn is_empty(&self) -> bool {
avahi_entry_group_is_empty(self.inner) != 0
}

/// Delegate function for [`avahi_entry_group_add_service()`].
///
/// Also propagates any error returned into a `Result`.
///
/// [`avahi_entry_group_add_service()`]: https://avahi.org/doxygen/html/publish_8h.html#acb05a7d3d23a3b825ca77cb1c7d00ce4
pub fn add_service(
///
/// # Safety
/// This function is unsafe because of the call to `avahi_entry_group_add_service_strlst()`.
pub unsafe fn add_service(
&mut self,
AddServiceParams {
interface,
Expand All @@ -74,7 +83,7 @@ impl ManagedAvahiEntryGroup {
}: AddServiceParams,
) -> Result<()> {
avahi_util::sys_exec(
|| unsafe {
|| {
avahi_entry_group_add_service_strlst(
self.inner,
interface,
Expand All @@ -97,7 +106,10 @@ impl ManagedAvahiEntryGroup {
/// Also propagates any error returned into a `Result`.
///
/// [`avahi_entry_group_add_service_subtype()`]: https://avahi.org/doxygen/html/publish_8h.html#a93841be69a152d3134b408c25bb4d5d5
pub fn add_service_subtype(
///
/// # Safety
/// This function is unsafe because of the call to `avahi_entry_group_add_service_subtype()`.
pub unsafe fn add_service_subtype(
&mut self,
AddServiceSubtypeParams {
interface,
Expand All @@ -110,7 +122,7 @@ impl ManagedAvahiEntryGroup {
}: AddServiceSubtypeParams,
) -> Result<()> {
avahi_util::sys_exec(
|| unsafe {
|| {
avahi_entry_group_add_service_subtype(
self.inner, interface, protocol, flags, name, kind, domain, subtype,
)
Expand All @@ -124,18 +136,24 @@ impl ManagedAvahiEntryGroup {
/// Also propagates any error returned into a `Result`.
///
/// [`avahi_entry_group_commit()`]: https://avahi.org/doxygen/html/publish_8h.html#a2375338d23af4281399404758840a2de
pub fn commit(&mut self) -> Result<()> {
///
/// # Safety
/// This function is unsafe because of the call to `avahi_entry_group_commit()`.
pub unsafe fn commit(&mut self) -> Result<()> {
avahi_util::sys_exec(
|| unsafe { avahi_entry_group_commit(self.inner) },
|| avahi_entry_group_commit(self.inner),
"could not commit service",
)
}

/// Delegate function for [`avahi_entry_group_reset()`].
///
/// [`avahi_entry_group_reset()`]: https://avahi.org/doxygen/html/publish_8h.html#a1293bbccf878dbeb9916660022bc71b2
pub fn reset(&mut self) {
unsafe { avahi_entry_group_reset(self.inner) };
///
/// # Safety
/// This function is unsafe because of the call to `avahi_entry_group_reset()`.
pub unsafe fn reset(&mut self) {
avahi_entry_group_reset(self.inner);
}

/// Delegate function for [`avahi_entry_group_get_client()`].
Expand Down
2 changes: 1 addition & 1 deletion zeroconf/src/avahi/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ impl TEventLoop for AvahiEventLoop {
/// does not respect the `timeout` parameter, the `timeout` passed
/// here will have no effect -- ie will return immediately.
fn poll(&self, timeout: Duration) -> Result<()> {
self.poll.iterate(timeout)
unsafe { self.poll.iterate(timeout) }
}
}
21 changes: 15 additions & 6 deletions zeroconf/src/avahi/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ pub struct ManagedAvahiSimplePoll(*mut AvahiSimplePoll);
impl ManagedAvahiSimplePoll {
/// Initializes the underlying `*mut AvahiSimplePoll` and verifies it was created; returning
/// `Err(String)` if unsuccessful
pub fn new() -> Result<Self> {
let poll = unsafe { avahi_simple_poll_new() };
///
/// # Safety
/// This function is unsafe because of the raw pointer dereference.
pub unsafe fn new() -> Result<Self> {
let poll = avahi_simple_poll_new();
if poll.is_null() {
Err("could not initialize AvahiSimplePoll".into())
} else {
Expand All @@ -30,24 +33,30 @@ impl ManagedAvahiSimplePoll {
/// Delegate function for [`avahi_simple_poll_loop()`].
///
/// [`avahi_simple_poll_loop()`]: https://avahi.org/doxygen/html/simple-watch_8h.html#a14b4cb29832e8c3de609d4c4e5611985
pub fn start_loop(&self) -> Result<()> {
///
/// # Safety
/// This function is unsafe because of the call to `avahi_simple_poll_loop()`.
pub unsafe fn start_loop(&self) -> Result<()> {
avahi_util::sys_exec(
|| unsafe { avahi_simple_poll_loop(self.0) },
|| avahi_simple_poll_loop(self.0),
"could not start AvahiSimplePoll",
)
}

/// Delegate function for [`avahi_simple_poll_iterate()`].
///
/// [`avahi_simple_poll_iterate()`]: https://avahi.org/doxygen/html/simple-watch_8h.html#ad5b7c9d3b7a6584d609241ee6f472a2e
pub fn iterate(&self, timeout: Duration) -> Result<()> {
///
/// # Safety
/// This function is unsafe because of the call to `avahi_simple_poll_iterate()`.
pub unsafe fn iterate(&self, timeout: Duration) -> Result<()> {
let sleep_time: i32 = timeout
.as_millis() // `avahi_simple_poll_iterate()` expects `sleep_time` in msecs.
.try_into() // `avahi_simple_poll_iterate()` expects `sleep_time` as an i32.
.unwrap_or(i32::MAX); // if converting to an i32 overflows, just use the largest number we can.

// Returns -1 on error, 0 on success and 1 if a quit request has been scheduled
match unsafe { avahi_simple_poll_iterate(self.0, sleep_time) } {
match avahi_simple_poll_iterate(self.0, sleep_time) {
0 | 1 => Ok(()),
-1 => Err(Error::from(
"avahi_simple_poll_iterate(..) threw an error result",
Expand Down
27 changes: 14 additions & 13 deletions zeroconf/src/avahi/raw_browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ pub struct ManagedAvahiServiceBrowser {
impl ManagedAvahiServiceBrowser {
/// Initializes the underlying `*mut AvahiClient` and verifies it was created; returning
/// `Err(String)` if unsuccessful.
pub fn new(
///
/// # Safety
/// This function is unsafe because of the raw pointer dereference.
pub unsafe fn new(
ManagedAvahiServiceBrowserParams {
client,
interface,
Expand All @@ -37,18 +40,16 @@ impl ManagedAvahiServiceBrowser {
userdata,
}: ManagedAvahiServiceBrowserParams,
) -> Result<Self> {
let inner = unsafe {
avahi_service_browser_new(
client.inner,
interface,
protocol,
kind,
domain,
flags,
callback,
userdata,
)
};
let inner = avahi_service_browser_new(
client.inner,
interface,
protocol,
kind,
domain,
flags,
callback,
userdata,
);

if inner.is_null() {
Err("could not initialize Avahi service browser".into())
Expand Down
Loading

0 comments on commit 2e5ee10

Please sign in to comment.