From 173ca7ee23db4bf570045fde1f17366fb2b70c0b Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Tue, 11 Mar 2025 00:10:08 +0100
Subject: [PATCH 1/9] vlcrs-core: tracer: redefine license headers as comments
These should not be considered as documentation, in some cases, it
messes with the formatter and possibly rust-doc.
---
src/rust/vlcrs-core/src/tracer/mod.rs | 33 ++++++++++++++-------------
src/rust/vlcrs-core/src/tracer/sys.rs | 33 ++++++++++++++-------------
2 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index 48b3c18b34af..1efa689995fa 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -1,19 +1,20 @@
-/// SPDX-License-Identifier: LGPL-2.1-or-later
-/// Copyright (C) 2024-2025 Alexandre Janniaux <ajanni@videolabs.io>
-///
-/// This program is free software; you can redistribute it and/or modify it
-/// under the terms of the GNU Lesser General Public License as published by
-/// the Free Software Foundation; either version 2.1 of the License, or
-/// (at your option) any later version.
-///
-/// This program is distributed in the hope that it will be useful,
-/// but WITHOUT ANY WARRANTY; without even the implied warranty of
-/// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-/// GNU Lesser General Public License for more details.
-///
-/// You should have received a copy of the GNU Lesser General Public License
-/// along with this program; if not, write to the Free Software Foundation,
-/// Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2024-2025 Alexandre Janniaux <ajanni@videolabs.io>
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation; either version 2.1 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with this program; if not, write to the Free Software Foundation,
+// Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+
use std::{
cell::UnsafeCell,
ffi::{c_char, c_void, CStr},
diff --git a/src/rust/vlcrs-core/src/tracer/sys.rs b/src/rust/vlcrs-core/src/tracer/sys.rs
index 7ddabf0dcafd..1ff964d272ce 100644
--- a/src/rust/vlcrs-core/src/tracer/sys.rs
+++ b/src/rust/vlcrs-core/src/tracer/sys.rs
@@ -1,25 +1,26 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2024-2025 Alexandre Janniaux <ajanni@videolabs.io>
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation; either version 2.1 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with this program; if not, write to the Free Software Foundation,
+// Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+
#![allow(rustdoc::bare_urls)]
#![allow(rustdoc::broken_intra_doc_links)]
#![allow(non_upper_case_globals)]
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]
-/// SPDX-License-Identifier: LGPL-2.1-or-later
-/// Copyright (C) 2024-2025 Alexandre Janniaux <ajanni@videolabs.io>
-///
-/// This program is free software; you can redistribute it and/or modify it
-/// under the terms of the GNU Lesser General Public License as published by
-/// the Free Software Foundation; either version 2.1 of the License, or
-/// (at your option) any later version.
-///
-/// This program is distributed in the hope that it will be useful,
-/// but WITHOUT ANY WARRANTY; without even the implied warranty of
-/// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-/// GNU Lesser General Public License for more details.
-///
-/// You should have received a copy of the GNU Lesser General Public License
-/// along with this program; if not, write to the Free Software Foundation,
-/// Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
use std::{
ffi::{c_char, c_double, c_void, CStr},
ptr::NonNull,
--
GitLab
From 6d30dc06243cb33e836788b4ffed6a389857e950 Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Thu, 6 Mar 2025 14:42:58 +0100
Subject: [PATCH 2/9] vlcrs-core: tracer: move higher level bindings out of
`sys`
Reserve the sys module for raw VLC bindings. The moved abstraction are
meant to be transformed into safe encapsulations of sys content.
---
src/rust/vlcrs-core/src/tracer/mod.rs | 126 +++++++++++++++++++++++++-
src/rust/vlcrs-core/src/tracer/sys.rs | 125 ++-----------------------
2 files changed, 130 insertions(+), 121 deletions(-)
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index 1efa689995fa..3829b47d667d 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -25,7 +25,6 @@ use std::{
use crate::{object::Object, plugin::ModuleProtocol};
pub mod sys;
-pub use sys::{Trace, TraceField};
pub struct Tick(pub i64);
@@ -94,11 +93,16 @@ pub type TracerCapabilityActivate =
opaque: &mut MaybeUninit<*mut c_void>,
) -> Option<&'static sys::vlc_tracer_operations>;
-extern "C" fn tracer_trace(opaque: *const c_void, tick: sys::vlc_tick, entries: sys::Trace) {
+extern "C" fn tracer_trace(
+ opaque: *const c_void,
+ tick: sys::vlc_tick,
+ entries: NonNull<sys::vlc_tracer_trace>,
+) {
{
let tracer: &dyn TracerCapability =
unsafe { &**(opaque as *const Box<dyn TracerCapability>) };
- tracer.trace(Tick(tick), &entries);
+ let trace = Trace { entries };
+ tracer.trace(Tick(tick), &trace);
}
}
@@ -205,7 +209,7 @@ impl Tracer {
// SAFETY: the pointer `tracer` is guaranteed to be non-null and
// nobody else has reference to it.
- sys::vlc_tracer_TraceWithTs(tracer, tick.0, entries);
+ sys::vlc_tracer_TraceWithTs(tracer, tick.0, entries.entries);
}
}
}
@@ -214,3 +218,117 @@ impl Tracer {
macro_rules! trace {
($tracer: ident, $ts: expr, $($key:ident = $value:expr)*) => {};
}
+
+#[derive(PartialEq, Copy, Clone)]
+#[repr(transparent)]
+pub struct Trace {
+ pub entries: NonNull<sys::vlc_tracer_trace>,
+}
+
+#[derive(PartialEq, Copy, Clone, Debug)]
+#[repr(transparent)]
+pub struct TraceField {
+ pub entry: NonNull<sys::vlc_tracer_entry>,
+}
+
+impl TraceField {
+ pub fn key(&self) -> &str {
+ unsafe {
+ let key = CStr::from_ptr(self.entry.read().key);
+ key.to_str().unwrap()
+ }
+ }
+
+ pub fn kind(&self) -> sys::vlc_tracer_value_type {
+ unsafe { self.entry.read().kind }
+ }
+
+ pub fn value(&self) -> sys::vlc_tracer_value {
+ unsafe { self.entry.read().value }
+ }
+}
+
+pub struct TraceIterator {
+ current_field: NonNull<sys::vlc_tracer_entry>,
+}
+
+impl IntoIterator for Trace {
+ type Item = TraceField;
+ type IntoIter = TraceIterator;
+ fn into_iter(self) -> Self::IntoIter {
+ TraceIterator {
+ current_field: unsafe { self.entries.read().entries },
+ }
+ }
+}
+
+impl Iterator for TraceIterator {
+ type Item = TraceField;
+
+ fn next(&mut self) -> Option<Self::Item> {
+ unsafe {
+ if self.current_field.read().key.is_null() {
+ return None;
+ }
+ let output = Some(TraceField {
+ entry: self.current_field,
+ });
+ self.current_field = self.current_field.add(1);
+ output
+ }
+ }
+}
+
+#[cfg(test)]
+mod test {
+ #[test]
+ fn test_trace_interop() {
+ use super::*;
+ use sys::*;
+ let entries = [
+ vlc_tracer_entry {
+ kind: vlc_tracer_value_type::String,
+ value: vlc_tracer_value {
+ string: c"value1".as_ptr(),
+ },
+ key: c"test1".as_ptr(),
+ },
+ vlc_tracer_entry {
+ kind: vlc_tracer_value_type::Integer,
+ value: vlc_tracer_value { integer: 0 },
+ key: std::ptr::null(),
+ },
+ ];
+
+ let trace_field = TraceField {
+ entry: NonNull::from(&entries[0]),
+ };
+ assert_eq!(trace_field.kind(), vlc_tracer_value_type::String);
+ assert_eq!(trace_field.key(), "test1");
+
+ let trace_field = TraceField {
+ entry: NonNull::from(&entries[1]),
+ };
+ assert_eq!(trace_field.kind(), vlc_tracer_value_type::Integer);
+
+ let trace = vlc_tracer_trace {
+ entries: NonNull::from(&entries[0]),
+ };
+
+ let trace = Trace {
+ entries: NonNull::from(&trace),
+ };
+
+ let mut iterator = trace.into_iter();
+
+ let first = iterator.next().expect("First field must be valid");
+ assert_eq!(first.kind(), vlc_tracer_value_type::String);
+ assert_eq!(first.key(), "test1");
+ unsafe {
+ assert_eq!(CStr::from_ptr(first.value().string), c"value1");
+ }
+
+ let second = iterator.next();
+ assert_eq!(second, None);
+ }
+}
diff --git a/src/rust/vlcrs-core/src/tracer/sys.rs b/src/rust/vlcrs-core/src/tracer/sys.rs
index 1ff964d272ce..afbb14039671 100644
--- a/src/rust/vlcrs-core/src/tracer/sys.rs
+++ b/src/rust/vlcrs-core/src/tracer/sys.rs
@@ -22,7 +22,7 @@
#![allow(non_snake_case)]
use std::{
- ffi::{c_char, c_double, c_void, CStr},
+ ffi::{c_char, c_double, c_void},
ptr::NonNull,
};
@@ -69,128 +69,19 @@ pub struct vlc_tracer {
_unused: [u8; 0],
}
-#[derive(PartialEq, Copy, Clone)]
-#[repr(transparent)]
-pub struct Trace {
- pub entries: NonNull<vlc_tracer_trace>,
-}
-
-#[derive(PartialEq, Copy, Clone, Debug)]
-#[repr(transparent)]
-pub struct TraceField {
- pub entry: NonNull<vlc_tracer_entry>,
-}
-
-impl TraceField {
- pub fn key(&self) -> &str {
- unsafe {
- let key = CStr::from_ptr(self.entry.read().key);
- key.to_str().unwrap()
- }
- }
-
- pub fn kind(&self) -> vlc_tracer_value_type {
- unsafe { self.entry.read().kind }
- }
-
- pub fn value(&self) -> vlc_tracer_value {
- unsafe { self.entry.read().value }
- }
-}
-
-pub struct TraceIterator {
- current_field: NonNull<vlc_tracer_entry>,
-}
-
-impl IntoIterator for Trace {
- type Item = TraceField;
- type IntoIter = TraceIterator;
- fn into_iter(self) -> Self::IntoIter {
- TraceIterator {
- current_field: unsafe { self.entries.read().entries },
- }
- }
-}
-
-impl Iterator for TraceIterator {
- type Item = TraceField;
-
- fn next(&mut self) -> Option<Self::Item> {
- unsafe {
- if self.current_field.read().key.is_null() {
- return None;
- }
- let output = Some(TraceField {
- entry: self.current_field,
- });
- self.current_field = self.current_field.add(1);
- output
- }
- }
-}
-
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct vlc_tracer_operations {
- pub trace: extern "C" fn(opaque: *const c_void, ts: vlc_tick, entries: Trace),
+ pub trace:
+ extern "C" fn(opaque: *const c_void, ts: vlc_tick, entries: NonNull<vlc_tracer_trace>),
pub destroy: extern "C" fn(*mut c_void),
}
extern "C" {
pub fn vlc_tracer_Create(parent: &Object, name: *const c_char) -> Option<NonNull<vlc_tracer>>;
-
- pub fn vlc_tracer_TraceWithTs(tracer: NonNull<vlc_tracer>, tick: vlc_tick, entries: Trace);
-}
-
-#[cfg(test)]
-mod test {
- #[test]
- fn test_trace_interop() {
- use super::*;
- let entries = [
- vlc_tracer_entry {
- kind: vlc_tracer_value_type::String,
- value: vlc_tracer_value {
- string: c"value1".as_ptr(),
- },
- key: c"test1".as_ptr(),
- },
- vlc_tracer_entry {
- kind: vlc_tracer_value_type::Integer,
- value: vlc_tracer_value { integer: 0 },
- key: std::ptr::null(),
- },
- ];
-
- let trace_field = TraceField {
- entry: NonNull::from(&entries[0]),
- };
- assert_eq!(trace_field.kind(), vlc_tracer_value_type::String);
- assert_eq!(trace_field.key(), "test1");
-
- let trace_field = TraceField {
- entry: NonNull::from(&entries[1]),
- };
- assert_eq!(trace_field.kind(), vlc_tracer_value_type::Integer);
-
- let trace = vlc_tracer_trace {
- entries: NonNull::from(&entries[0]),
- };
-
- let trace = Trace {
- entries: NonNull::from(&trace),
- };
-
- let mut iterator = trace.into_iter();
-
- let first = iterator.next().expect("First field must be valid");
- assert_eq!(first.kind(), vlc_tracer_value_type::String);
- assert_eq!(first.key(), "test1");
- unsafe {
- assert_eq!(CStr::from_ptr(first.value().string), c"value1");
- }
-
- let second = iterator.next();
- assert_eq!(second, None);
- }
+ pub fn vlc_tracer_TraceWithTs(
+ tracer: NonNull<vlc_tracer>,
+ tick: vlc_tick,
+ entries: NonNull<vlc_tracer_trace>,
+ );
}
--
GitLab
From 629731341e1e24acd2f122916cdcb94fca189633 Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Sat, 8 Mar 2025 01:13:09 +0100
Subject: [PATCH 3/9] vlcrs-core: tracer: define `Trace` as tuple struct
Since `Trace` is a wrapper around the VLC tracer object. `entries` as a
name was not actually giving access to trace entries but to the
internals of the object.
---
src/rust/vlcrs-core/src/tracer/mod.rs | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index 3829b47d667d..07ac25840108 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -101,7 +101,7 @@ extern "C" fn tracer_trace(
{
let tracer: &dyn TracerCapability =
unsafe { &**(opaque as *const Box<dyn TracerCapability>) };
- let trace = Trace { entries };
+ let trace = Trace(entries);
tracer.trace(Tick(tick), &trace);
}
}
@@ -209,7 +209,7 @@ impl Tracer {
// SAFETY: the pointer `tracer` is guaranteed to be non-null and
// nobody else has reference to it.
- sys::vlc_tracer_TraceWithTs(tracer, tick.0, entries.entries);
+ sys::vlc_tracer_TraceWithTs(tracer, tick.0, entries.0);
}
}
}
@@ -221,9 +221,7 @@ macro_rules! trace {
#[derive(PartialEq, Copy, Clone)]
#[repr(transparent)]
-pub struct Trace {
- pub entries: NonNull<sys::vlc_tracer_trace>,
-}
+pub struct Trace(NonNull<sys::vlc_tracer_trace>);
#[derive(PartialEq, Copy, Clone, Debug)]
#[repr(transparent)]
@@ -257,7 +255,7 @@ impl IntoIterator for Trace {
type IntoIter = TraceIterator;
fn into_iter(self) -> Self::IntoIter {
TraceIterator {
- current_field: unsafe { self.entries.read().entries },
+ current_field: unsafe { self.0.read().entries },
}
}
}
@@ -315,9 +313,7 @@ mod test {
entries: NonNull::from(&entries[0]),
};
- let trace = Trace {
- entries: NonNull::from(&trace),
- };
+ let trace = Trace(NonNull::from(&trace));
let mut iterator = trace.into_iter();
--
GitLab
From b4235480a5d97ffe8519bc4707100d69fef9d1d3 Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Sat, 8 Mar 2025 01:13:09 +0100
Subject: [PATCH 4/9] vlcrs-core: tracer: unify trace parameter names
---
modules/logger/telegraf-rs/src/lib.rs | 8 ++++----
src/rust/vlcrs-core/src/tracer/mod.rs | 14 +++++++-------
src/rust/vlcrs-core/src/tracer/sys.rs | 5 ++---
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/modules/logger/telegraf-rs/src/lib.rs b/modules/logger/telegraf-rs/src/lib.rs
index eceede51e318..32d404b492a6 100644
--- a/modules/logger/telegraf-rs/src/lib.rs
+++ b/modules/logger/telegraf-rs/src/lib.rs
@@ -61,15 +61,15 @@ impl TracerCapability for TelegrafTracer {
Some(Self { endpoint })
}
- fn trace(&self, _tick: vlcrs_core::tracer::Tick, entries: &'_ vlcrs_core::tracer::Trace) {
- if !entries
+ fn trace(&self, _tick: vlcrs_core::tracer::Tick, trace: &'_ vlcrs_core::tracer::Trace) {
+ if !trace
.into_iter()
.any(|e| e.kind() != vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
{
/* We cannot support events for now. */
return;
}
- let tags: Vec<(String, String)> = entries
+ let tags: Vec<(String, String)> = trace
.into_iter()
.filter(|e| e.kind() == vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
.map(|entry| unsafe {
@@ -81,7 +81,7 @@ impl TracerCapability for TelegrafTracer {
})
.collect();
- let record: Vec<(String, Box<dyn IntoFieldData + 'static>)> = entries
+ let record: Vec<(String, Box<dyn IntoFieldData + 'static>)> = trace
.into_iter()
.filter(|e| e.kind() != vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
.map(|entry| {
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index 07ac25840108..98b7c6ae916d 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -53,7 +53,7 @@ pub struct Tick(pub i64);
/// Some(Self{ last_trace_tick: Cell::from(Tick(0)) })
/// }
///
-/// fn trace(&self, tick: Tick, entries: &Trace) {
+/// fn trace(&self, tick: Tick, trace: &Trace) {
/// let mut state = self.last_trace_tick.get_mut();
/// *state = tick;
/// }
@@ -83,7 +83,7 @@ pub trait TracerCapability: Sync {
where
Self: Sized;
- fn trace(&self, tick: Tick, entries: &Trace);
+ fn trace(&self, tick: Tick, trace: &Trace);
}
#[allow(non_camel_case_types)]
@@ -96,12 +96,12 @@ pub type TracerCapabilityActivate =
extern "C" fn tracer_trace(
opaque: *const c_void,
tick: sys::vlc_tick,
- entries: NonNull<sys::vlc_tracer_trace>,
+ trace: NonNull<sys::vlc_tracer_trace>,
) {
{
let tracer: &dyn TracerCapability =
unsafe { &**(opaque as *const Box<dyn TracerCapability>) };
- let trace = Trace(entries);
+ let trace = Trace(trace);
tracer.trace(Tick(tick), &trace);
}
}
@@ -149,7 +149,7 @@ extern "C" fn activate_tracer<T: TracerCapability>(
/// fn open(obj: &mut Object) -> Option<impl TracerCapability> {
/// Some(Self{})
/// }
-/// fn trace(&self, _tick: Tick, _entries: &Trace) {}
+/// fn trace(&self, _tick: Tick, _trace: &Trace) {}
/// }
///
/// module!{
@@ -202,14 +202,14 @@ impl Tracer {
///
/// Register the new point at time [tick] with the metadata [entries].
///
- pub fn trace(&self, tick: Tick, entries: Trace) {
+ pub fn trace(&self, tick: Tick, trace: Trace) {
unsafe {
// SAFETY: TODO
let tracer = *self.tracer.get();
// SAFETY: the pointer `tracer` is guaranteed to be non-null and
// nobody else has reference to it.
- sys::vlc_tracer_TraceWithTs(tracer, tick.0, entries.0);
+ sys::vlc_tracer_TraceWithTs(tracer, tick.0, trace.0);
}
}
}
diff --git a/src/rust/vlcrs-core/src/tracer/sys.rs b/src/rust/vlcrs-core/src/tracer/sys.rs
index afbb14039671..4501bac5f238 100644
--- a/src/rust/vlcrs-core/src/tracer/sys.rs
+++ b/src/rust/vlcrs-core/src/tracer/sys.rs
@@ -72,8 +72,7 @@ pub struct vlc_tracer {
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct vlc_tracer_operations {
- pub trace:
- extern "C" fn(opaque: *const c_void, ts: vlc_tick, entries: NonNull<vlc_tracer_trace>),
+ pub trace: extern "C" fn(opaque: *const c_void, ts: vlc_tick, trace: NonNull<vlc_tracer_trace>),
pub destroy: extern "C" fn(*mut c_void),
}
@@ -82,6 +81,6 @@ extern "C" {
pub fn vlc_tracer_TraceWithTs(
tracer: NonNull<vlc_tracer>,
tick: vlc_tick,
- entries: NonNull<vlc_tracer_trace>,
+ trace: NonNull<vlc_tracer_trace>,
);
}
--
GitLab
From 1986a3d2dcb31bdeda7513b2dc26f252b5f77104 Mon Sep 17 00:00:00 2001
From: Alaric Senat <alaric@videolabs.io>
Date: Thu, 13 Mar 2025 12:02:49 +0100
Subject: [PATCH 5/9] vlcrs-core: tracer: add a ref-only iteration function
Introduce a small accessor to the entries that communicates the intent
better than `into_iter()`.
---
modules/logger/telegraf-rs/src/lib.rs | 6 ++--
src/rust/vlcrs-core/src/tracer/mod.rs | 44 ++++++++++++++++++++-------
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/modules/logger/telegraf-rs/src/lib.rs b/modules/logger/telegraf-rs/src/lib.rs
index 32d404b492a6..e0b6a4c88d77 100644
--- a/modules/logger/telegraf-rs/src/lib.rs
+++ b/modules/logger/telegraf-rs/src/lib.rs
@@ -63,14 +63,14 @@ impl TracerCapability for TelegrafTracer {
fn trace(&self, _tick: vlcrs_core::tracer::Tick, trace: &'_ vlcrs_core::tracer::Trace) {
if !trace
- .into_iter()
+ .entries()
.any(|e| e.kind() != vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
{
/* We cannot support events for now. */
return;
}
let tags: Vec<(String, String)> = trace
- .into_iter()
+ .entries()
.filter(|e| e.kind() == vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
.map(|entry| unsafe {
let value = CStr::from_ptr(entry.value().string);
@@ -82,7 +82,7 @@ impl TracerCapability for TelegrafTracer {
.collect();
let record: Vec<(String, Box<dyn IntoFieldData + 'static>)> = trace
- .into_iter()
+ .entries()
.filter(|e| e.kind() != vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
.map(|entry| {
(
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index 98b7c6ae916d..bd39f5ea6f2b 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -219,10 +219,42 @@ macro_rules! trace {
($tracer: ident, $ts: expr, $($key:ident = $value:expr)*) => {};
}
+/// A trace record obtained from VLC.
+///
+/// Trace records are a set of `entries`, a list of key-values describing the traced event.
+///
+/// # Examples
+///
+/// ```
+/// impl TracerCapability for T {
+/// fn trace(&self, trace: &Trace) {
+/// for entry in trace.entries() {
+/// println!("{}", entry.key);
+/// }
+/// }
+/// }
+/// ```
#[derive(PartialEq, Copy, Clone)]
#[repr(transparent)]
pub struct Trace(NonNull<sys::vlc_tracer_trace>);
+impl Trace {
+ /// Get an iterator over the trace entries.
+ pub fn entries(&self) -> TraceIterator {
+ self.into_iter()
+ }
+}
+
+impl IntoIterator for Trace {
+ type Item = TraceField;
+ type IntoIter = TraceIterator;
+ fn into_iter(self) -> Self::IntoIter {
+ TraceIterator {
+ current_field: unsafe { self.0.read().entries },
+ }
+ }
+}
+
#[derive(PartialEq, Copy, Clone, Debug)]
#[repr(transparent)]
pub struct TraceField {
@@ -250,16 +282,6 @@ pub struct TraceIterator {
current_field: NonNull<sys::vlc_tracer_entry>,
}
-impl IntoIterator for Trace {
- type Item = TraceField;
- type IntoIter = TraceIterator;
- fn into_iter(self) -> Self::IntoIter {
- TraceIterator {
- current_field: unsafe { self.0.read().entries },
- }
- }
-}
-
impl Iterator for TraceIterator {
type Item = TraceField;
@@ -315,7 +337,7 @@ mod test {
let trace = Trace(NonNull::from(&trace));
- let mut iterator = trace.into_iter();
+ let mut iterator = trace.entries();
let first = iterator.next().expect("First field must be valid");
assert_eq!(first.kind(), vlc_tracer_value_type::String);
--
GitLab
From 667447d5bdc5afc814b1577d83b080e925aab228 Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Sat, 8 Mar 2025 18:41:49 +0100
Subject: [PATCH 6/9] logger: telegraf: reduce iterations over trace entries
Use `fold` to avoid two iterations over the entries while keeping the
functional code style.
---
modules/logger/telegraf-rs/src/lib.rs | 49 ++++++++++++---------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/modules/logger/telegraf-rs/src/lib.rs b/modules/logger/telegraf-rs/src/lib.rs
index e0b6a4c88d77..f9b4ea48328f 100644
--- a/modules/logger/telegraf-rs/src/lib.rs
+++ b/modules/logger/telegraf-rs/src/lib.rs
@@ -62,40 +62,35 @@ impl TracerCapability for TelegrafTracer {
}
fn trace(&self, _tick: vlcrs_core::tracer::Tick, trace: &'_ vlcrs_core::tracer::Trace) {
- if !trace
- .entries()
- .any(|e| e.kind() != vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
- {
+ let (tags, records) = trace.entries().fold(
+ (Vec::new(), Vec::new()),
+ |(mut tags, mut records), entry| {
+ if entry.kind() == vlcrs_core::tracer::sys::vlc_tracer_value_type::String {
+ let value = unsafe { CStr::from_ptr(entry.value().string) };
+ tags.push((
+ String::from(entry.key()),
+ String::from(value.to_str().unwrap()),
+ ));
+ } else {
+ records.push((
+ String::from(entry.key()),
+ Box::new(TraceField(entry)) as Box<dyn IntoFieldData>,
+ ));
+ }
+
+ (tags, records)
+ },
+ );
+
+ if records.is_empty() {
/* We cannot support events for now. */
return;
}
- let tags: Vec<(String, String)> = trace
- .entries()
- .filter(|e| e.kind() == vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
- .map(|entry| unsafe {
- let value = CStr::from_ptr(entry.value().string);
- (
- String::from(entry.key()),
- String::from(value.to_str().unwrap()),
- )
- })
- .collect();
-
- let record: Vec<(String, Box<dyn IntoFieldData + 'static>)> = trace
- .entries()
- .filter(|e| e.kind() != vlcrs_core::tracer::sys::vlc_tracer_value_type::String)
- .map(|entry| {
- (
- String::from(entry.key()),
- Box::new(TraceField(entry)) as Box<dyn IntoFieldData>,
- )
- })
- .collect();
let p = Point::new(
String::from("measurement"),
tags,
- record,
+ records,
None, //Some(tick.0 as u64),
);
--
GitLab
From e0aabc1d8bce7c155b27188fe5ae227f712fd94b Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Mon, 10 Mar 2025 23:45:48 +0100
Subject: [PATCH 7/9] vlcrs-core: add helper trait for unchecked conversions
See trait documentation for extended context.
---
src/rust/vlcrs-core/src/convert.rs | 67 ++++++++++++++++++++++++++++++
src/rust/vlcrs-core/src/lib.rs | 2 +
2 files changed, 69 insertions(+)
create mode 100644 src/rust/vlcrs-core/src/convert.rs
diff --git a/src/rust/vlcrs-core/src/convert.rs b/src/rust/vlcrs-core/src/convert.rs
new file mode 100644
index 000000000000..864f33dae9e4
--- /dev/null
+++ b/src/rust/vlcrs-core/src/convert.rs
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2025 Alaric Senat <alaric@videolabs.io>
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation; either version 2.1 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with this program; if not, write to the Free Software Foundation,
+// Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+
+/// Unchecked conversions on internal VLC values.
+///
+/// VLC core APIs often have implicit constraints or internal check on exposed data that allow
+/// conversions without the usual safety checks.
+///
+/// Implementers of this trait must ensure that the conversion is safe within the context of
+/// VLC, and new implementations must be thoroughly reviewed.
+///
+/// # Safety
+///
+/// Calling `assume_valid` is **always unsafe**, as it bypasses Rust's usual validity checks.
+/// The caller must ensure that the input is indeed valid and checked in some way beforehand by VLC.
+///
+/// # Examples
+///
+/// [`AssumeValid`]`<&`[`str`](std::str)`>` is implemented for [`CStr`](std::ffi::CStr) as the internals of
+/// VLC manipulate already checked UTF8 strings. This lift the bindings from the obligation to
+/// perform UTF8 validity verifications:
+///
+/// ```
+/// extern "C" fn tracer_trace(trace: *const std::ffi::c_char) {
+/// // `trace` is from VLC and is assumed to be valid UTF8.
+/// let trace: &str = unsafe { std::ffi::CStr::from_ptr(trace).assume_valid() };
+/// }
+/// ```
+pub trait AssumeValid<T> {
+ /// Converts `self` into `T`, without safety checks.
+ unsafe fn assume_valid(self) -> T;
+}
+
+impl<'a> AssumeValid<&'a str> for &'a std::ffi::CStr {
+ /// Convert a [`CStr`](std::ffi::CStr) into an `&`[`str`](std::str).
+ ///
+ /// # Safety
+ ///
+ /// Should only be used with VLC strings which are already UTF8 validated. Otherwise, the
+ /// string will be left unchecked and will lead to undefined behaviors.
+ ///
+ /// # Panics
+ ///
+ /// In **debug builds only**, this function will perform the UTF8 checks and panic on failure to
+ /// highlight and help resolve VLC core missing checks.
+ unsafe fn assume_valid(self) -> &'a str {
+ if cfg!(debug_assertions) {
+ str::from_utf8(self.to_bytes()).expect("Unexpected invalid UTF8 coming from VLC")
+ } else {
+ unsafe { str::from_utf8_unchecked(self.to_bytes()) }
+ }
+ }
+}
diff --git a/src/rust/vlcrs-core/src/lib.rs b/src/rust/vlcrs-core/src/lib.rs
index 90950409bca3..5f9a99a2c4b0 100644
--- a/src/rust/vlcrs-core/src/lib.rs
+++ b/src/rust/vlcrs-core/src/lib.rs
@@ -34,3 +34,5 @@ pub mod plugin;
pub mod object;
pub mod tracer;
+
+pub(crate) mod convert;
--
GitLab
From 6bfea2228a99288fdf5a5ec4eea5cf31928d3a89 Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Sat, 8 Mar 2025 19:04:00 +0100
Subject: [PATCH 8/9] vlcrs-core: tracer: add safe bindings for trace iteration
Introduce a lightweight safe `TraceValue` wrapper of the VLC tracer
tagged union.
---
modules/logger/telegraf-rs/src/lib.rs | 58 ++++++---------
src/rust/vlcrs-core/src/tracer/mod.rs | 103 +++++++++++++-------------
2 files changed, 75 insertions(+), 86 deletions(-)
diff --git a/modules/logger/telegraf-rs/src/lib.rs b/modules/logger/telegraf-rs/src/lib.rs
index f9b4ea48328f..fce30d9964f6 100644
--- a/modules/logger/telegraf-rs/src/lib.rs
+++ b/modules/logger/telegraf-rs/src/lib.rs
@@ -15,30 +15,20 @@
// along with this program; if not, write to the Free Software Foundation,
// Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
-use std::{cell::UnsafeCell, ffi::CStr, sync::Mutex};
-use telegraf::{Client, IntoFieldData, Point};
-use vlcrs_core::tracer::{sys::vlc_tracer_value_type, TracerCapability, TracerModuleLoader};
+use std::{cell::UnsafeCell, sync::Mutex};
+use telegraf::{Client, IntoFieldData};
+use vlcrs_core::tracer::{TraceValue, TracerCapability, TracerModuleLoader};
use vlcrs_macros::module;
-struct TraceField(vlcrs_core::tracer::TraceField);
+struct TraceValueWrapper<'a>(TraceValue<'a>);
-impl IntoFieldData for TraceField {
+impl<'a> IntoFieldData for TraceValueWrapper<'a> {
fn field_data(&self) -> telegraf::FieldData {
- match self.0.kind() {
- vlc_tracer_value_type::String => unsafe {
- let value = CStr::from_ptr(self.0.value().string);
- telegraf::FieldData::Str(value.to_str().unwrap().to_string())
- },
- vlc_tracer_value_type::Integer => unsafe {
- telegraf::FieldData::Number(self.0.value().integer)
- },
- vlc_tracer_value_type::Unsigned => unsafe {
- telegraf::FieldData::UNumber(self.0.value().unsigned)
- },
- vlc_tracer_value_type::Double => unsafe {
- telegraf::FieldData::Float(self.0.value().double)
- },
- _ => unreachable!(),
+ match self.0 {
+ TraceValue::String(value) => telegraf::FieldData::Str(String::from(value)),
+ TraceValue::Integer(value) => telegraf::FieldData::Number(value),
+ TraceValue::Unsigned(value) => telegraf::FieldData::UNumber(value),
+ TraceValue::Double(value) => telegraf::FieldData::Float(value),
}
}
}
@@ -61,21 +51,17 @@ impl TracerCapability for TelegrafTracer {
Some(Self { endpoint })
}
- fn trace(&self, _tick: vlcrs_core::tracer::Tick, trace: &'_ vlcrs_core::tracer::Trace) {
+ fn trace(&self, _tick: vlcrs_core::tracer::Tick, trace: &vlcrs_core::tracer::Trace) {
let (tags, records) = trace.entries().fold(
(Vec::new(), Vec::new()),
|(mut tags, mut records), entry| {
- if entry.kind() == vlcrs_core::tracer::sys::vlc_tracer_value_type::String {
- let value = unsafe { CStr::from_ptr(entry.value().string) };
- tags.push((
- String::from(entry.key()),
- String::from(value.to_str().unwrap()),
- ));
+ let name = String::from(entry.key);
+ if let TraceValue::String(value) = entry.value {
+ let value = String::from(value);
+ tags.push(telegraf::protocol::Tag { name, value });
} else {
- records.push((
- String::from(entry.key()),
- Box::new(TraceField(entry)) as Box<dyn IntoFieldData>,
- ));
+ let value = TraceValueWrapper(entry.value).field_data();
+ records.push(telegraf::protocol::Field { name, value });
}
(tags, records)
@@ -87,12 +73,12 @@ impl TracerCapability for TelegrafTracer {
return;
}
- let p = Point::new(
- String::from("measurement"),
+ let p = telegraf::Point {
+ measurement: String::from("measurement"),
tags,
- records,
- None, //Some(tick.0 as u64),
- );
+ fields: records,
+ timestamp: None, //Some(tick.0 as u64),
+ };
let mut endpoint = self.endpoint.lock().unwrap();
if let Err(err) = endpoint.get_mut().write_point(&p) {
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index bd39f5ea6f2b..5320c6948b69 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -22,7 +22,7 @@ use std::{
ptr::NonNull,
};
-use crate::{object::Object, plugin::ModuleProtocol};
+use crate::{convert::AssumeValid, object::Object, plugin::ModuleProtocol};
pub mod sys;
@@ -245,60 +245,74 @@ impl Trace {
}
}
-impl IntoIterator for Trace {
- type Item = TraceField;
- type IntoIter = TraceIterator;
+impl<'a> IntoIterator for &'a Trace {
+ type Item = TraceEntry<'a>;
+ type IntoIter = TraceIterator<'a>;
fn into_iter(self) -> Self::IntoIter {
TraceIterator {
current_field: unsafe { self.0.read().entries },
+ _plt: std::marker::PhantomData,
}
}
}
-#[derive(PartialEq, Copy, Clone, Debug)]
-#[repr(transparent)]
-pub struct TraceField {
- pub entry: NonNull<sys::vlc_tracer_entry>,
-}
-
-impl TraceField {
- pub fn key(&self) -> &str {
- unsafe {
- let key = CStr::from_ptr(self.entry.read().key);
- key.to_str().unwrap()
- }
- }
-
- pub fn kind(&self) -> sys::vlc_tracer_value_type {
- unsafe { self.entry.read().kind }
- }
-
- pub fn value(&self) -> sys::vlc_tracer_value {
- unsafe { self.entry.read().value }
- }
-}
-
-pub struct TraceIterator {
+/// Iterate over trace record entries.
+pub struct TraceIterator<'a> {
current_field: NonNull<sys::vlc_tracer_entry>,
+ _plt: std::marker::PhantomData<&'a sys::vlc_tracer_entry>,
}
-impl Iterator for TraceIterator {
- type Item = TraceField;
+impl<'a> Iterator for TraceIterator<'a> {
+ type Item = TraceEntry<'a>;
fn next(&mut self) -> Option<Self::Item> {
unsafe {
if self.current_field.read().key.is_null() {
return None;
}
- let output = Some(TraceField {
- entry: self.current_field,
- });
+ let output = Some(TraceEntry::from(self.current_field.read()));
self.current_field = self.current_field.add(1);
output
}
}
}
+/// A key-value pair recorded in a trace event.
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub struct TraceEntry<'a> {
+ pub key: &'a str,
+ pub value: TraceValue<'a>,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum TraceValue<'a> {
+ Integer(i64),
+ Double(f64),
+ String(&'a str),
+ Unsigned(u64),
+}
+
+impl<'a> From<sys::vlc_tracer_entry> for TraceEntry<'a> {
+ fn from(entry: sys::vlc_tracer_entry) -> Self {
+ // SAFETY: Key is guaranteed to be non-null by the iterator.
+ let key = unsafe { CStr::from_ptr(entry.key).assume_valid() };
+
+ // SAFETY: Union accesses are only made with the associated entry tag.
+ let value = unsafe {
+ match entry.kind {
+ sys::vlc_tracer_value_type::Integer => TraceValue::Integer(entry.value.integer),
+ sys::vlc_tracer_value_type::Double => TraceValue::Double(entry.value.double),
+ sys::vlc_tracer_value_type::String => {
+ TraceValue::String(CStr::from_ptr(entry.value.string).assume_valid())
+ }
+ sys::vlc_tracer_value_type::Unsigned => TraceValue::Unsigned(entry.value.unsigned),
+ }
+ };
+
+ Self { key, value }
+ }
+}
+
#[cfg(test)]
mod test {
#[test]
@@ -320,16 +334,9 @@ mod test {
},
];
- let trace_field = TraceField {
- entry: NonNull::from(&entries[0]),
- };
- assert_eq!(trace_field.kind(), vlc_tracer_value_type::String);
- assert_eq!(trace_field.key(), "test1");
-
- let trace_field = TraceField {
- entry: NonNull::from(&entries[1]),
- };
- assert_eq!(trace_field.kind(), vlc_tracer_value_type::Integer);
+ let trace_field = TraceEntry::from(entries[0]);
+ assert_eq!(trace_field.key, "test1");
+ assert_eq!(trace_field.value, TraceValue::String("value1"));
let trace = vlc_tracer_trace {
entries: NonNull::from(&entries[0]),
@@ -340,13 +347,9 @@ mod test {
let mut iterator = trace.entries();
let first = iterator.next().expect("First field must be valid");
- assert_eq!(first.kind(), vlc_tracer_value_type::String);
- assert_eq!(first.key(), "test1");
- unsafe {
- assert_eq!(CStr::from_ptr(first.value().string), c"value1");
- }
+ assert_eq!(first.value, TraceValue::String("value1"));
+ assert_eq!(first.key, "test1");
- let second = iterator.next();
- assert_eq!(second, None);
+ assert_eq!(iterator.next(), None);
}
}
--
GitLab
From 562bc831b2668901fb1df202990d250600441ef2 Mon Sep 17 00:00:00 2001
From: Alaric Senat <dev.asenat@posteo.net>
Date: Sun, 9 Mar 2025 01:30:28 +0100
Subject: [PATCH 9/9] vlcrs-core: tracer: privatize unsafe sys bindings
The core unsafe bindings aren't needed anymore outside the scope of the
bindings.
---
src/rust/vlcrs-core/src/tracer/mod.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rust/vlcrs-core/src/tracer/mod.rs b/src/rust/vlcrs-core/src/tracer/mod.rs
index 5320c6948b69..d0656ae07e4e 100644
--- a/src/rust/vlcrs-core/src/tracer/mod.rs
+++ b/src/rust/vlcrs-core/src/tracer/mod.rs
@@ -24,7 +24,7 @@ use std::{
use crate::{convert::AssumeValid, object::Object, plugin::ModuleProtocol};
-pub mod sys;
+mod sys;
pub struct Tick(pub i64);
--
GitLab