Skip to content

Commit edb7ecf

Browse files
committed
sameold: remove slice conversion from Window
`SliceRingBuffer` is unsound and has multiple security advisories [1] [2]. There has been no apparent progress on fixes since May 2025 [3]. The crate's large volume of interdependent `unsafe` blocks will likely make it very tricky to maintain. While a memory-mapped deque is a very cool idea, it is time to move on for now. Let's replace `SliceRingBuffer` with a more traditional double-ended queue. These queues don't covert to slice, but we don't really *need* a slice. At present, slice is mostly used to align the end of `Window` (with the most recent samples) to the filter taps. We can do that just as easily with a `DoubleEndedIterator`. * Make `Window` convertible to an iterator of numbers, via `IntoIterator`. Replace all uses of slices with iterators. * Remove slice conversions and accessors from Window. They're still technically accessible via `inner()`, but nothing uses them. [1]: https://rustsec.org/advisories/RUSTSEC-2025-0044 [2]: https://rustsec.org/advisories/RUSTSEC-2020-0158 [3]: LiquidityC/slice_ring_buffer#12
1 parent f817716 commit edb7ecf

4 files changed

Lines changed: 77 additions & 62 deletions

File tree

crates/sameold/src/receiver/dcblock.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ mod tests {
168168
output_history.push_scalar(uut.filter(100.0f32 + clk));
169169
clk = -1.0 * clk;
170170
}
171-
assert_approx_eq!(output_history.as_slice()[0], 1.0f32, 1.0e-2);
172-
assert_approx_eq!(output_history.as_slice()[1], -1.0f32, 1.0e-2);
171+
assert_approx_eq!(output_history.inner()[0], 1.0f32, 1.0e-2);
172+
assert_approx_eq!(output_history.inner()[1], -1.0f32, 1.0e-2);
173173
}
174174
}

crates/sameold/src/receiver/demod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,8 @@ impl FskDemod {
155155
// * `< 0` for space
156156
fn demod_now(&self) -> f32 {
157157
// matched filter
158-
let window = self.window_input.as_slice();
159-
let mark = self.coeff_mark.filter(window);
160-
let space = self.coeff_space.filter(window);
158+
let mark = self.coeff_mark.filter(&self.window_input);
159+
let space = self.coeff_space.filter(&self.window_input);
161160

162161
// non-coherently sum matched filter powers to obtain
163162
// the symbol estimate

crates/sameold/src/receiver/equalize.rs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ impl Equalizer {
133133
let feedforward_wind = Window::new(nfeedforward);
134134
let feedback_wind = Window::new(nfeedback);
135135

136+
assert_eq!(feedforward_coeff.inner().len(), feedforward_wind.len());
137+
assert_eq!(feedback_coeff.inner().len(), feedback_wind.len());
138+
136139
Self {
137140
relaxation,
138141
regularization,
@@ -314,14 +317,16 @@ impl Equalizer {
314317
self.relaxation,
315318
self.regularization,
316319
error,
317-
self.feedforward_wind.as_ref(),
320+
&self.feedforward_wind,
318321
self.feedforward_coeff.as_mut(),
319322
);
323+
324+
assert_eq!(self.feedback_wind.inner().len(), self.feedback_coeff.len());
320325
nlms_update(
321326
self.relaxation,
322327
self.regularization,
323328
-error,
324-
self.feedback_wind.as_ref(),
329+
&self.feedback_wind,
325330
self.feedback_coeff.as_mut(),
326331
);
327332
}
@@ -346,17 +351,14 @@ enum EqualizerState {
346351
// variable gain. The gain is computed based on the power
347352
// of the input. The given `filter` taps are updated based
348353
// on the input samples in `window`.
349-
fn nlms_update(
350-
relaxation: f32,
351-
regularization: f32,
352-
error: f32,
353-
window: &[f32],
354-
filter: &mut [f32],
355-
) {
356-
assert_eq!(window.len(), filter.len());
357-
358-
let gain = nlms_gain(relaxation, regularization, window);
359-
for (coeff, data) in filter.iter_mut().zip(window.iter()) {
354+
fn nlms_update<W>(relaxation: f32, regularization: f32, error: f32, window: W, filter: &mut [f32])
355+
where
356+
W: IntoIterator<Item = f32>,
357+
W::IntoIter: DoubleEndedIterator + Clone,
358+
{
359+
let window = window.into_iter();
360+
let gain = nlms_gain(relaxation, regularization, window.clone());
361+
for (coeff, data) in filter.iter_mut().zip(window) {
360362
*coeff += gain * error * data;
361363
}
362364
}
@@ -371,7 +373,10 @@ fn nlms_update(
371373
//
372374
// where `norm(x, 2)` is the L² norm of `x`
373375
#[inline]
374-
fn nlms_gain(relaxation: f32, regularization: f32, window: &[f32]) -> f32 {
376+
fn nlms_gain<'a, W>(relaxation: f32, regularization: f32, window: W) -> f32
377+
where
378+
W: IntoIterator<Item = f32>,
379+
{
375380
let mut sumsq = 0.0f32;
376381
for w in window {
377382
sumsq += w * w;
@@ -473,7 +478,7 @@ mod tests {
473478
RELAXATION,
474479
REGULARIZATION,
475480
err,
476-
inverse_wind.as_ref(),
481+
&inverse_wind,
477482
inverse_coeff.as_mut(),
478483
);
479484
}

crates/sameold/src/receiver/filter.rs

Lines changed: 53 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -116,25 +116,26 @@ where
116116
self.0.len()
117117
}
118118

119-
/// Perform FIR filtering with the given sample history slice
119+
/// Perform FIR filtering with the given sample history
120120
///
121121
/// Computes the current output sample of the filter assuming
122-
/// the given `history`. `history` should be a slice where
123-
/// `history[N-1]` is the most recent sample and `history[0]`
124-
/// is the least recent/oldest sample.
122+
/// the given `history`. `history` must be a
123+
/// `DoubledEndedIterator` which outputs the oldest sample
124+
/// first and the newest sample last. The newest sample is
125+
/// used for feedforward lag 0. `history` SHOULD contain at
126+
/// least `self.len()` samples, but no error is raised if it
127+
/// is shorter.
125128
///
126-
/// The caller should maintain a deque of history. New samples
127-
/// should be pushed to the end of the queue, and old samples
128-
/// should age off the head of the queue. The queue *should*
129-
/// contain `self.len()` samples, but it is not an error if
130-
/// it contains more or less.
131-
pub fn filter<I, In, Out>(&self, history: I) -> Out
129+
/// The caller is encouraged to use a deque for `history`,
130+
/// but this is not enforced.
131+
pub fn filter<W, In, Out>(&self, history: W) -> Out
132132
where
133-
I: AsRef<[In]>,
133+
W: IntoIterator<Item = In>,
134+
W::IntoIter: DoubleEndedIterator,
134135
In: Copy + Scalar + std::ops::Mul<T, Output = Out>,
135136
Out: Copy + Scalar + Zero + std::ops::AddAssign,
136137
{
137-
multiply_accumulate(history.as_ref(), self.as_ref())
138+
multiply_accumulate(history, self.as_ref())
138139
}
139140

140141
/// Reset to identity filter
@@ -275,7 +276,7 @@ where
275276
///
276277
/// Appends the `input` slice to the right side of the Window.
277278
/// The last sample of `input` becomes the right-most / most recent
278-
/// sample of the Window slice.
279+
/// sample of the Window.
279280
///
280281
/// If the length of `input` exceeds the length of the Window,
281282
/// then the right-most chunk of `input` will be taken.
@@ -301,8 +302,8 @@ where
301302
/// Append a scalar to the sample window
302303
///
303304
/// Appends the `input` scalar to the right side of the Window.
304-
/// It becomes the last / most recent sample of the Window
305-
/// slice. Returns the sample that was formerly the oldest
305+
/// It becomes the last / most recent sample of Window.
306+
/// Returns the sample that was formerly the oldest
306307
/// sample in the Window.
307308
#[inline]
308309
pub fn push_scalar(&mut self, input: T) -> T {
@@ -311,19 +312,26 @@ where
311312
out
312313
}
313314

314-
/// Obtain the inner SliceRingBuffer
315-
pub fn inner(&self) -> &SliceRingBuffer<T> {
316-
&self.0
315+
/// Iterator over window contents
316+
///
317+
/// The iterator outputs the least recent sample first.
318+
/// The most recent sample is output last.
319+
pub fn iter(&self) -> <&Window<T> as IntoIterator>::IntoIter {
320+
self.into_iter()
317321
}
318322

319-
/// Obtain current window contents, as a slice
323+
/// Convert window contents to a vector
320324
///
321-
/// The zeroth sample of the slice is the least recent
322-
/// sample in the window. The last sample of the slice
323-
/// is the most recent sample in the window.
324-
#[inline]
325-
pub fn as_slice(&self) -> &[T] {
326-
self.0.as_slice()
325+
/// Copy the current contents of the window to a freshly-allocated
326+
/// vector. Vector elements are ordered from least recent sample
327+
/// to most recent sample.
328+
pub fn to_vec(&self) -> Vec<T> {
329+
self.iter().collect()
330+
}
331+
332+
/// Obtain the inner SliceRingBuffer
333+
pub fn inner(&self) -> &SliceRingBuffer<T> {
334+
&self.0
327335
}
328336

329337
/// Most recent element pushed into the Window
@@ -339,12 +347,16 @@ where
339347
}
340348
}
341349

342-
impl<T> AsRef<[T]> for Window<T>
350+
impl<'a, T> IntoIterator for &'a Window<T>
343351
where
344352
T: Copy + Scalar + Zero,
345353
{
346-
fn as_ref(&self) -> &[T] {
347-
self.as_slice()
354+
type Item = T;
355+
356+
type IntoIter = std::iter::Copied<core::slice::Iter<'a, T>>;
357+
358+
fn into_iter(self) -> Self::IntoIter {
359+
self.0.as_slice().into_iter().copied()
348360
}
349361
}
350362

@@ -363,7 +375,7 @@ where
363375
// stored *reversed* in `rev_coeff`, with `rev_coeff[N-1]` being
364376
// the zeroth filter coefficient.
365377
//
366-
// The two slices need not be the same length. If `history` is shorter
378+
// The two inputs need not be the same length. If `history` is shorter
367379
// than `rev_coeff`, then the sample history is assumed to be zero
368380
// outside of its range.
369381
//
@@ -373,19 +385,18 @@ where
373385
//
374386
// The output value is returned. Any compatible arithmetic types may
375387
// be used, including complex numbers.
376-
fn multiply_accumulate<In, Coeff, Out>(history: &[In], rev_coeff: &[Coeff]) -> Out
388+
fn multiply_accumulate<W, In, Coeff, Out>(history: W, rev_coeff: &[Coeff]) -> Out
377389
where
390+
W: IntoIterator<Item = In>,
391+
W::IntoIter: DoubleEndedIterator,
378392
In: Copy + Scalar + std::ops::Mul<Coeff, Output = Out>,
379393
Coeff: Copy + Scalar,
380394
Out: Copy + Scalar + Zero + std::ops::AddAssign,
381395
{
382-
let mul_len = usize::min(history.len(), rev_coeff.len());
383-
let history = &history[history.len() - mul_len..];
384-
let rev_coeff = &rev_coeff[rev_coeff.len() - mul_len..];
385-
396+
let history = history.into_iter();
386397
let mut out = Out::zero();
387-
for (hi, co) in history.iter().zip(rev_coeff.iter()) {
388-
out += *hi * *co;
398+
for (hi, co) in history.rev().zip(rev_coeff.iter().rev()) {
399+
out += hi * *co;
389400
}
390401
out
391402
}
@@ -446,26 +457,26 @@ mod tests {
446457
fn test_window() {
447458
let mut wind: Window<f32> = Window::new(4);
448459
assert_eq!(4, wind.len());
449-
assert_eq!(&[0.0f32, 0.0f32, 0.0f32, 0.0f32], wind.as_slice());
460+
assert_eq!(vec![0.0f32, 0.0f32, 0.0f32, 0.0f32], wind.to_vec());
450461
wind.push(&[1.0f32]);
451-
assert_eq!(&[0.0f32, 0.0f32, 0.0f32, 1.0f32], wind.as_slice());
462+
assert_eq!(vec![0.0f32, 0.0f32, 0.0f32, 1.0f32], wind.to_vec());
452463

453464
wind.push(&[2.0f32]);
454-
assert_eq!(&[0.0f32, 0.0f32, 1.0f32, 2.0f32], wind.as_slice());
465+
assert_eq!(vec![0.0f32, 0.0f32, 1.0f32, 2.0f32], wind.to_vec());
455466

456467
wind.push(&[-1.0f32, -2.0f32, 1.0f32, 2.0f32, 3.0f32, 4.0f32]);
457-
assert_eq!(&[1.0f32, 2.0f32, 3.0f32, 4.0f32], wind.as_slice());
468+
assert_eq!(vec![1.0f32, 2.0f32, 3.0f32, 4.0f32], wind.to_vec());
458469
assert_eq!(4.0f32, wind.back());
459470
assert_eq!(1.0f32, wind.front());
460471
assert_eq!(4, wind.len());
461472

462473
// push individual samples works too
463474
assert_eq!(1.0f32, wind.push_scalar(10.0f32));
464475
assert_eq!(4, wind.len());
465-
assert_eq!(&[2.0f32, 3.0f32, 4.0f32, 10.0f32], wind.as_slice());
476+
assert_eq!(vec![2.0f32, 3.0f32, 4.0f32, 10.0f32], wind.to_vec());
466477

467478
wind.reset();
468479
assert_eq!(4, wind.len());
469-
assert_eq!(&[0.0f32, 0.0f32, 0.0f32, 0.0f32], wind.as_slice());
480+
assert_eq!(vec![0.0f32, 0.0f32, 0.0f32, 0.0f32], wind.to_vec());
470481
}
471482
}

0 commit comments

Comments
 (0)