bleh

pjsua/src/lib.rs

@@ -11,6 +11,7 @@
 
 mod pjsua {
     #![allow(unnecessary_transmutes)]
+    #![allow(unsafe_op_in_unsafe_fn)]
     include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
 }
 

sipcord-bridge/src/audio/mod.rs

@@ -1,27 +1,17 @@
-//! Audio parsing utilities
+//! Audio parsing utilities for WAV and FLAC, used by the sound module.
-//!
-//! This module provides audio file parsing for WAV and FLAC formats.
-//! Used by the `sound` module for loading audio files from disk.
 
 pub mod flac;
 pub mod simd;
 pub mod wav;
 
-/// Errors that can occur while parsing a WAV or FLAC file.
 #[derive(thiserror::Error, Debug)]
 pub enum AudioParseError {
-    /// File header, chunk structure, or sample data was malformed for the
-    /// format implied by the magic bytes. Carries a short human-readable
-    /// reason (chunk name, byte offset, etc.).
     #[error("malformed audio data: {0}")]
     Malformed(String),
 
-    /// Audio format is recognised but not supported by this parser (e.g.
-    /// non-PCM WAV, or a FLAC stream with an exotic bit depth).
     #[error("unsupported audio: {0}")]
     Unsupported(String),
 
-    /// Underlying claxon FLAC decoder error.
     #[error("FLAC decode error: {0}")]
     Flac(#[from] claxon::Error),
 }

sipcord-bridge/src/config.rs

@@ -120,8 +120,7 @@ impl EnvConfig {
         Ok(())
     }
 
-    /// Access the global `EnvConfig` (panics if `init()` was not called — a
+    /// Access the global `EnvConfig`. Panics if `init()` was not called.
-    /// programmer error, not a recoverable condition).
     pub fn global() -> &'static EnvConfig {
         ENV_CONFIG.get().unwrap_or_else(|| {
             panic!("EnvConfig not initialized — call EnvConfig::init() first")
@@ -335,8 +334,7 @@ impl AppConfig {
         })
     }
 
-    /// Get the global application config (panics if not initialized — a
+    /// Get the global application config. Panics if `AppConfig::load(...)` was not called.
-    /// programmer error: caller must `AppConfig::load(...)` first).
     pub fn global() -> &'static AppConfig {
         APP_CONFIG.get().unwrap_or_else(|| {
             panic!("AppConfig not initialized — call AppConfig::load() first")

sipcord-bridge/src/error.rs

@@ -1,8 +1,4 @@
 //! Top-level error type for the `sipcord-bridge` crate.
-//!
-//! [`BridgeError`] aggregates every subsystem error so callers (main binaries,
-//! adapter crates) can use a single `Result` type and rely on `?` propagation
-//! via `#[from]` conversions.
 
 use crate::audio::AudioParseError;
 use crate::config::ConfigError;
@@ -12,7 +8,6 @@ use crate::services::sound::SoundError;
 use crate::transport::discord::DiscordError;
 use crate::transport::sip::error::SipError;
 
-/// Umbrella error for the entire bridge crate.
 #[derive(thiserror::Error, Debug)]
 pub enum BridgeError {
     #[error(transparent)]
@@ -36,8 +31,6 @@ pub enum BridgeError {
     #[error(transparent)]
     AudioParse(#[from] AudioParseError),
 
-    /// Generic I/O at the top level (file ops in main, etc.) that aren't tied
-    /// to a particular subsystem.
     #[error("I/O ({context}): {source}")]
     Io {
         context: String,

sipcord-bridge/src/fax/mod.rs

@@ -17,24 +17,14 @@ pub mod session;
 pub mod spandsp;
 pub mod tiff_decoder;
 
-/// Errors from the fax subsystem.
-///
-/// Variants are intentionally coarse — fax flows are end-to-end best-effort
-/// (a missed page or codec mismatch logs and aborts the session) and the
-/// detailed `String` payloads carry enough context for triage. Where a more
-/// structured upstream type already exists (`serenity::Error`, `io::Error`),
-/// we wrap it via `#[from]` / `#[source]`.
 #[derive(thiserror::Error, Debug)]
 pub enum FaxError {
-    /// Discord REST / gateway error while posting or editing fax status.
     #[error("Discord post failed: {0}")]
     Discord(#[from] serenity::Error),
 
-    /// Token parsing failure when constructing the fax-posting client.
     #[error("invalid Discord bot token: {0}")]
     InvalidToken(String),
 
-    /// I/O error reading/writing TIFFs or working with paths.
     #[error("fax I/O ({context}): {source}")]
     Io {
         context: String,
@@ -42,24 +32,18 @@ pub enum FaxError {
         source: std::io::Error,
     },
 
-    /// Path couldn't be converted to UTF-8 for the SpanDSP / TIFF API.
     #[error("path is not valid UTF-8: {0}")]
     NonUtf8Path(String),
 
-    /// SpanDSP FFI returned an error from one of its setters or state-init
-    /// functions.
     #[error("SpanDSP ({operation}): {detail}")]
     SpanDsp {
         operation: &'static str,
         detail: String,
     },
 
-    /// TIFF parsing / decoding failure. Carries a human-readable reason.
     #[error("TIFF decode: {0}")]
     Tiff(String),
 
-    /// A received fax produced no pages (decoder bail-out, session closed
-    /// before any page was completed, etc.).
     #[error("no pages in received fax")]
     NoPages,
 }

sipcord-bridge/src/fax/spandsp.rs

@@ -76,8 +76,6 @@ fn configure_t30(
     tiff_path: &str,
     callback_state: &mut FaxCallbackState,
 ) -> Result<(), FaxError> {
-    /// Local macro: tag a SpanDSP error with the operation name. Avoids
-    /// boilerplate at every setter call site.
     macro_rules! spandsp_err {
         ($op:expr) => {
             |e| FaxError::SpanDsp {

sipcord-bridge/src/fax/tiff_decoder.rs

@@ -12,8 +12,6 @@ use std::path::Path;
 use std::sync::OnceLock;
 use tracing::debug;
 
-/// Convenience: most failures in this module are malformed-TIFF conditions
-/// that map cleanly onto `FaxError::Tiff(String)`.
 macro_rules! tiff_bail {
     ($($arg:tt)*) => {
         return Err(FaxError::Tiff(format!($($arg)*)))

sipcord-bridge/src/lib.rs

@@ -8,10 +8,6 @@
 //! and authentication. A built-in `StaticBackend` (TOML dialplan) is included.
 
 #![feature(portable_simd)]
-// Lock down the no-unwrap policy. Test modules opt out via the
-// `#[cfg_attr(test, allow(...))]` shim at their boundary (or `#[allow]` at
-// the test fn level for isolated cases). See feedback memories
-// `feedback-no-unwrap-in-production` and `feedback-fix-clippy-at-source`.
 #![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used))]
 
 pub mod audio;

sipcord-bridge/src/main.rs

@@ -20,9 +20,6 @@ use sipcord_bridge::transport::sip::SipTransport;
 
 #[tokio::main]
 async fn main() -> Result<(), BridgeError> {
-    // Pre-init failures here are programmer errors (missing rustls feature
-    // flag, double-init of the global crypto provider) — panicking is the
-    // right behaviour and there's no caller that could recover.
     if rustls::crypto::ring::default_provider()
         .install_default()
         .is_err()

sipcord-bridge/src/services/sound/mod.rs

@@ -18,10 +18,8 @@ use tracing::{debug, info, warn};
 
 pub use streaming::{StreamingError, StreamingPlayer};
 
-/// Errors raised by sound loading and parsing.
 #[derive(thiserror::Error, Debug)]
 pub enum SoundError {
-    /// Failed to read the sound file from disk.
     #[error("failed to read sound file {path:?}: {source}")]
     Read {
         path: PathBuf,
@@ -29,7 +27,6 @@ pub enum SoundError {
         source: std::io::Error,
     },
 
-    /// Parse failure from the WAV/FLAC decoder.
     #[error("failed to parse audio for {name}: {source}")]
     Parse {
         name: String,
@@ -37,7 +34,6 @@ pub enum SoundError {
         source: AudioParseError,
     },
 
-    /// Sound's sample rate didn't match the bridge's configured rate.
     #[error("sound {name} has wrong sample rate: {got} Hz (expected {expected} Hz)")]
     WrongSampleRate {
         name: String,
@@ -45,11 +41,9 @@ pub enum SoundError {
         expected: u32,
     },
 
-    /// File header doesn't match any supported format (WAV / FLAC).
     #[error("unknown audio format for {name}: header bytes {header:02x?}")]
     UnknownFormat { name: String, header: Vec<u8> },
 
-    /// Streaming player setup failure.
     #[error(transparent)]
     Streaming(#[from] StreamingError),
 }

sipcord-bridge/src/services/sound/streaming.rs

@@ -16,7 +16,6 @@ use symphonia::core::io::MediaSourceStream;
 use symphonia::core::meta::MetadataOptions;
 use symphonia::core::probe::Hint;
 
-/// Errors raised by the streaming player.
 #[derive(thiserror::Error, Debug)]
 pub enum StreamingError {
     #[error("failed to open streaming file {path:?}: {source}")]

sipcord-bridge/src/transport/discord/mod.rs

@@ -179,10 +179,7 @@ fn create_resampler() -> Async<f64> {
         window: WindowFunction::BlackmanHarris2,
     };
 
-    // 16kHz to 48kHz = ratio of 3.0, mono input, 320 samples per chunk (20ms at 16kHz).
+    // 16kHz → 48kHz, mono, 320 samples per chunk (20ms at 16kHz).
-    // Params are static and known-good; if rubato rejects them it's a programmer
-    // error (e.g. ratio constants changed inconsistently) and the program can't
-    // run anyway — panic explicitly with the rubato error for diagnostics.
     Async::new_sinc(
         48000.0 / 16000.0,
         1.1,
@@ -675,9 +672,7 @@ impl DiscordVoiceConnection {
                     // This allows the pjsua audio thread to bypass tokio entirely
                     let audio_sender = RegisteredAudioSender::new(channel_id, producer);
 
-                    // Create shared timestamp for health tracking. The system
+                    // Create shared timestamp for health tracking
-                    // clock would have to be set before 1970 to produce Err
-                    // here; default to 0 in that case rather than panic.
                     let now_ms = SystemTime::now()
                         .duration_since(UNIX_EPOCH)
                         .unwrap_or_default()

sipcord-bridge/src/transport/discord/voice.rs

@@ -181,10 +181,8 @@ impl Read for StreamingAudioSource {
             return Ok(buf.len());
         }
 
-        // Read samples from ring buffer directly into output buffer.
         // `samples_to_read <= samples_available` by construction, so this
-        // should never error; if rtrb's state ever desyncs, log + silence
+        // should never error; on rtrb desync, silence rather than panic.
-        // rather than panic on the audio thread.
         let chunk = match consumer.read_chunk(samples_to_read) {
             Ok(c) => c,
             Err(e) => {

sipcord-bridge/src/transport/sip/error.rs

@@ -1,19 +1,4 @@
 //! Typed error types for the SIP transport layer.
-//!
-//! Three sibling enums cover the three phases of the SIP path:
-//!
-//! - [`SipInitError`] — startup: pjsua create/init/start, transports, codecs,
-//!   account registration. One-shot failures that take down the process.
-//! - [`SipResponseError`] — building or sending an individual SIP response
-//!   (401/302/200 etc.) from inside an FFI callback. Per-call, recoverable
-//!   in the sense that we log and continue.
-//! - [`SipAudioError`] — runtime audio plumbing: hooking players into a
-//!   call's conference port. Surfaces when media isn't ready yet, when a
-//!   port name has an interior NUL, or when pjsua refuses a connect.
-//!
-//! [`SipError`] is the umbrella for callers that want to handle any of them
-//! uniformly. Conversion is via `#[from]`, so `?` propagation works through
-//! the hierarchy without explicit `map_err`.
 
 use std::ffi::NulError;
 
@@ -36,8 +21,6 @@ pub enum SipError {
 /// Errors raised by outbound-call setup (`make_outbound_call`).
 #[derive(thiserror::Error, Debug)]
 pub enum SipCallError {
-    /// A URI / display-name string couldn't be converted to CString because
-    /// of an interior NUL byte.
     #[error("invalid {field} for outbound call: {source}")]
     InvalidString {
         field: &'static str,
@@ -45,7 +28,6 @@ pub enum SipCallError {
         source: NulError,
     },
 
-    /// `pjsua_call_make_call` returned non-success.
     #[error("pjsua_call_make_call failed (status {0})")]
     MakeCall(i32),
 }
@@ -54,25 +36,21 @@ pub enum SipCallError {
 /// `process_pjsua_events`, and friends.
 #[derive(thiserror::Error, Debug)]
 pub enum SipInitError {
-    /// A pjsua API returned a non-success status code. `operation` names the
+    /// A pjsua API returned a non-success status code; `operation` names the
-    /// specific call (`"pjsua_create"`, `"pjsua_init"`, `"pjsua_start"`,
+    /// specific call (e.g. `"pjsua_init"`, `"pjsua_acc_add"`).
-    /// `"pjsua_acc_add"`, `"pjsua_set_null_snd_dev"`, `"pjsua_handle_events"`,
-    /// etc.).
     #[error("pjsua {operation} failed (status {status})")]
     Pjsua {
         operation: &'static str,
         status: i32,
     },
 
-    /// `pjsua_transport_create` failed for `kind` ("UDP", "TCP", or "TLS").
+    /// `pjsua_transport_create` failed; `kind` is `"UDP"`, `"TCP"`, or `"TLS"`.
     #[error("transport create ({kind}) failed (status {status})")]
     TransportCreate {
         kind: &'static str,
         status: i32,
     },
 
-    /// A configuration string (host name, URI, etc.) couldn't be converted
-    /// to a `CString` because of an interior NUL byte.
     #[error("invalid {field} string for FFI: {source}")]
     InvalidString {
         field: &'static str,
@@ -80,83 +58,59 @@ pub enum SipInitError {
         source: NulError,
     },
 
-    /// A `Path` to be passed into pjsua wasn't valid UTF-8.
     #[error("{field} path is not valid UTF-8")]
     NonUtf8Path { field: &'static str },
 }
 
-/// Errors raised by audio-port plumbing (`play_audio_to_call_direct`,
+/// Errors raised by audio-port plumbing (players, conf port hookup).
-/// `start_loop`, `start_test_tone_to_call`, etc.) and the helpers in
-/// `frame_utils`.
 #[derive(thiserror::Error, Debug)]
 pub enum SipAudioError {
-    /// The call doesn't have a conference port yet — media negotiation is
+    /// Media negotiation hasn't produced a conference port yet (or the call
-    /// still in progress, or the call has just ended. Caller can retry or
+    /// has just ended). Caller may retry or drop the audio.
-    /// drop the audio.
     #[error("no conference port for call {call_id} (media not ready yet)")]
     NoConfPort { call_id: super::ffi::types::CallId },
 
-    /// A port name (used to identify the player in pjsua's mixer) contains
-    /// an interior NUL.
     #[error("invalid port name: {0}")]
     InvalidPortName(#[from] NulError),
 
-    /// `pjsua_conf_add_port`, `pjsua_conf_connect`, etc. returned non-success.
     #[error("pjsua conf {operation} failed (status {status})")]
     Pjsua {
         operation: &'static str,
         status: i32,
     },
 
-    /// Frame size / port count mismatch between the audio source and the
-    /// pjsua port.
     #[error("frame mismatch: {0}")]
     FrameMismatch(String),
 
-    /// Failure setting up a streaming player (file read, decoder, etc.).
     #[error(transparent)]
     Streaming(#[from] crate::services::sound::StreamingError),
 }
 
-/// Errors that can occur while building or sending a SIP response.
+/// Errors raised while building or sending a SIP response from inside an
-///
+/// FFI callback. The typical caller logs and continues.
-/// Surfaces failures from the pjsua/pjsip FFI surface — CString conversion,
-/// pool allocation, header creation, and the final stateless / transactional
-/// send. Variants stay coarse-grained because the typical caller is a pjsip
-/// callback that can only log and continue.
 #[derive(thiserror::Error, Debug)]
 pub enum SipResponseError {
-    /// A runtime string contained an interior NUL byte and could not be
-    /// converted to `CString`.
     #[error("CString conversion failed (interior NUL)")]
     CStringNul(#[from] NulError),
 
-    /// `pjsua_pool_create` returned null — out of memory or pjsua not
-    /// initialised.
     #[error("pjsua pool allocation failed")]
     PoolAlloc,
 
-    /// `pjsip_generic_string_hdr_create` returned null.
     #[error("pjsip header creation failed")]
     HeaderCreate,
 
-    /// `pjsua_get_pjsip_endpt` returned null — pjsua not initialised.
     #[error("pjsip endpoint is null (pjsua not initialised)")]
     EndpointNull,
 
-    /// `pjsip_endpt_respond_stateless` returned a non-success pj status code.
     #[error("pjsip stateless send failed (status {0})")]
     StatelessSend(i32),
 
-    /// `pjsip_tsx_create_uas2` returned a non-success pj status code.
     #[error("pjsip UAS transaction creation failed (status {0})")]
     TsxCreate(i32),
 
-    /// `pjsip_endpt_create_response` returned a non-success pj status code.
     #[error("pjsip response build failed (status {0})")]
     ResponseBuild(i32),
 
-    /// `pjsua_call_answer` returned a non-success pj status code.
     #[error("pjsua_call_answer failed (status {0})")]
     CallAnswer(i32),
 }

sipcord-bridge/src/transport/sip/ffi/pj_str.rs

@@ -1,22 +1,5 @@
-//! Safe(r) helpers around the pjsua/pjsip C-string and header-building idioms
+//! Helpers around the pjsua/pjsip C-string and header-building idioms that
-//! that recur across the SIP transport layer.
+//! recur across the SIP transport layer.
-//!
-//! Before this module existed, every callback that built a SIP header
-//! re-implemented the same pattern:
-//!
-//! ```ignore
-//! let name = CString::new("Contact").unwrap();
-//! let value = CString::new(runtime_str).unwrap();
-//! let name_pj = pj_str(name.as_ptr() as *mut c_char);
-//! let value_pj = pj_str(value.as_ptr() as *mut c_char);
-//! let hdr = pjsip_generic_string_hdr_create(pool, &name_pj, &value_pj);
-//! // ...
-//! ```
-//!
-//! That sprouted two unwraps per header (so any header value containing a NUL
-//! byte from upstream data would panic), repeated lifetime traps, and zero
-//! shared failure handling. The helpers in this module turn those calls into
-//! a single fallible call returning [`SipResponseError`].
 
 use crate::transport::sip::error::SipResponseError;
 use pjsua::*;
@@ -26,22 +9,13 @@ use std::ptr;
 
 /// Convert a [`CStr`] (typically a `c"..."` literal) into a [`pj_str_t`].
 ///
-/// Zero-cost — `pj_str` just wraps the pointer and length. The caller must
+/// Caller must keep `s` alive for the resulting `pj_str_t`'s usage window.
-/// keep the `CStr` alive for the `pj_str_t`'s usage window. For `&'static`
-/// literals (the common case) that's trivially satisfied.
 #[inline]
 pub unsafe fn pj_str_from_cstr(s: &CStr) -> pj_str_t {
     unsafe { pj_str(s.as_ptr() as *mut c_char) }
 }
 
-// A `pj_str_owned(&str) -> Result<(CString, pj_str_t), _>` helper was considered
+/// Initialise a `pjsip_hdr` as an empty list head.
-// but turned out unused: every runtime-string call site in this codebase ends
-// up either inside `make_string_hdr` (which does the conversion internally) or
-// in a function that already chains `CString::new(...).context("...")?` for a
-// site-specific error message. Add it back if a true caller appears.
-
-/// Initialise a `pjsip_hdr` as an empty list head (equivalent to the
-/// `pj_list_init` C macro).
 #[inline]
 pub unsafe fn pj_list_init_hdr(hdr: *mut pjsip_hdr) {
     unsafe {
@@ -52,11 +26,8 @@ pub unsafe fn pj_list_init_hdr(hdr: *mut pjsip_hdr) {
 
 /// Create a generic string header in `pool`.
 ///
-/// `name` is a static `CStr` (use `c"Contact"` etc); `value` is a runtime
+/// pjsip duplicates `name` and `value` into the pool, so the temporary
-/// string that gets converted to a `CString` and duplicated into the pool
+/// `CString` for `value` is dropped before this returns.
-/// by pjsip. The temporary `CString` is dropped before this returns;
-/// `pjsip_generic_string_hdr_create` uses `pj_strdup` internally to copy
-/// the bytes.
 pub unsafe fn make_string_hdr(
     pool: *mut pj_pool_t,
     name: &CStr,
@@ -93,17 +64,11 @@ pub unsafe fn append_tdata_hdr(
 
 /// Answer a pjsua call with N custom headers attached to the response.
 ///
-/// Wraps the recurring `pjsua_msg_data_init` + pool + header build +
+/// The pool is intentionally NOT released — pjsua continues referencing the
-/// `pjsua_call_answer` dance used in 401 / 302 / 4xx code paths.
+/// header data after `pjsua_call_answer` returns, so releasing here triggers
+/// use-after-free. Each call leaks ~512 bytes, reclaimed when pjsua shuts down.
 ///
-/// **The pool is intentionally NOT released.** pjsua may continue to reference
+/// The caller is responsible for `pjsua_call_hangup` on Err.
-/// the header data after `pjsua_call_answer` returns; releasing the pool here
-/// triggers use-after-free. Each call leaks ~512 bytes that's reclaimed when
-/// pjsua shuts down. (Tracking pools per-call and releasing them on call-end
-/// would be a cleaner fix; not in scope here.)
-///
-/// On error, the caller typically follows up with `pjsua_call_hangup` — this
-/// helper does not hang up on its own so the caller can choose the strategy.
 pub unsafe fn answer_call_with_headers(
     call_id: i32,
     status_code: u32,
@@ -120,7 +85,6 @@ pub unsafe fn answer_call_with_headers(
         if pool.is_null() {
             return Err(SipResponseError::PoolAlloc);
         }
-        // Intentionally leaked — see doc comment above.
 
         for (name, value) in headers {
             let hdr = make_string_hdr(pool, name, value)?;
@@ -141,12 +105,7 @@ pub unsafe fn answer_call_with_headers(
 
 /// Send a stateless SIP response with N string headers.
 ///
-/// Wraps the recurring `pjsua_pool_create` → list-head alloc → header
+/// `reason` is the SIP reason phrase (e.g. `Some(c"Unauthorized")`); pass
-/// build → `pjsip_endpt_respond_stateless` → `pj_pool_release` dance. Each
-/// header in `headers` is a `(name, value)` pair where `name` is typically
-/// a `c"..."` literal and `value` is any runtime string.
-///
-/// `reason` is the SIP reason phrase (e.g. `Some(c"Unauthorized")`) or
 /// `None` to let pjsip pick the default for `status_code`.
 pub unsafe fn respond_stateless_with_headers(
     rdata: *mut pjsip_rx_data,
@@ -165,8 +124,7 @@ pub unsafe fn respond_stateless_with_headers(
             return Err(SipResponseError::PoolAlloc);
         }
 
-        // Belt-and-braces: ensure the pool is released even if a step
+        // Closure so the pool gets released even if a step `?`-returns.
-        // between here and the send returns Err via `?`.
         let result =
             (|| -> Result<i32, SipResponseError> {
                 let hdr_list =

sipcord-bridge/src/transport/sip/mod.rs

@@ -207,8 +207,6 @@ impl SipTransport {
             }
         });
 
-        // JoinError -> log only; pjsua loop errors are already logged inside the
-        // spawned task.
         if let Err(e) = pjsua_handle.await {
             tracing::error!("pjsua event loop join error: {}", e);
         }