Backport tracer changes making it more tolerant to older HCI pre v2

Author: FrancescoConti

tracer/Cargo.lock

@@ -2,7 +2,9 @@
 name = "hci-tracer"
 version = "0.1.0"
 edition = "2021"
-rust-version = "1.85"
+rust-version = "1.81"
+# MSRV-aware resolution: pick dependency versions that build on `rust-version`.
+resolver = "3"
 description = "Compare HCI-Core and HWPE-Stream transaction logs produced by hci_transaction_tracer and hwpe_stream_transaction_tracer"
 authors = ["Francesco Conti <f.conti@unibo.it>"]
 license = "Apache-2.0"
@@ -19,5 +21,7 @@ path = "src/lib.rs"
 [dependencies]
 serde = { version = "1.0.228", features = ["derive"] }
 serde_json = "1.0.150"
-clap = { version = "4.6.1", features = ["derive"] }
+# Capped below 4.6: clap 4.6 and its `clap_lex` are written in edition
+# 2024, which needs Rust >= 1.85. 4.5 is edition 2021 and builds on 1.81+.
+clap = { version = "~4.5", features = ["derive"] }
 owo-colors = "4.3.0"

tracer/Cargo.toml

@@ -22,6 +22,24 @@ cargo test                     # unit + integration tests
 Dependencies are `serde`, `serde_json`, `clap` and `owo-colors`; `--offline`
 works if the crates are already in the local registry.
 
+The crate is edition 2021 and builds with **Rust 1.81 or newer**, which is what
+`rust-version` in `Cargo.toml` declares. Two things keep it that way, and both
+matter if you touch the dependencies:
+
+* `clap` is capped below 4.6 (`~4.5`). clap 4.6 and its `clap_lex` are written in
+  edition 2024 and need Rust 1.85, which fails on an older toolchain with
+  `feature `edition2024` is required`.
+* `resolver = "3"` makes Cargo pick dependency versions compatible with the
+  declared `rust-version` rather than the newest ones. It needs Cargo 1.84+.
+
+`Cargo.lock` is committed, so a plain `cargo build` uses a known-good set
+regardless. If you bump a dependency, re-check with an old toolchain:
+
+```sh
+rustup toolchain install 1.81 --profile minimal
+cargo +1.81 test
+```
+
 ## Usage
 
 ```
@@ -54,6 +72,13 @@ note saying how many.
 Never compared: `cycle` and `seq`. Off by default: `user`/`r_user`, `id`/`r_id`,
 `ecc`/`r_ecc` — switch them on with `--check-user`, `--check-id`, `--check-ecc`.
 
+Older HCI-Core interfaces have fewer side channels than current ones: a log whose
+header declares `IW = 0` or `EW = 0` carries no `id` or `ecc` fields at all. Those
+are left out of the comparison, so a trace taken on an old interface diffs cleanly
+against one taken on a new interface. Asking for a side channel that a log does not
+carry is refused outright rather than compared against a stub value, which would
+report a difference for every transaction.
+
 Byte enables are canonicalized to a bit-level mask before being compared, so
 `be = 0xf` with `BW = 8` is *equal* to `strb = 0xff` with `ELEMENT_WIDTH = 4` on a
 32-bit bus: both describe the same 32 enabled data bits. Bytes that neither side

tracer/README.md

@@ -22,7 +22,7 @@
 use std::path::Path;
 
 use crate::error::{Error, Result};
-use crate::model::{Beat, LoadedLog, LogKind, Payload};
+use crate::model::{Beat, Iface, LoadedLog, LogKind, Payload};
 use crate::value::Value;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -564,6 +564,45 @@ pub fn describe(beat: &Beat) -> String {
     }
 }
 
+fn side_channel_width(iface: &Iface, name: &str) -> u32 {
+    match iface {
+        Iface::Hci { uw, iw, ew, .. } => match name {
+            "user" => *uw,
+            "id" => *iw,
+            _ => *ew,
+        },
+        Iface::Stream { .. } => 0,
+    }
+}
+
+/// Refuse to compare a side channel that one of the logs does not carry.
+///
+/// Older HCI-Core interfaces have no `id` or `ecc` signals at all, so their logs
+/// declare `IW = 0` / `EW = 0` and leave those fields out. Comparing them anyway
+/// would silently treat the absent side as zero and report a difference for
+/// every transaction -- a misalignment that says nothing about the design.
+pub fn check_side_channels(a: &LoadedLog, b: &LoadedLog, ctx: &CmpCtx) -> Result<()> {
+    for (on, name, flag) in [
+        (ctx.spec.user, "user", "--check-user"),
+        (ctx.spec.id, "id", "--check-id"),
+        (ctx.spec.ecc, "ecc", "--check-ecc"),
+    ] {
+        if !on {
+            continue;
+        }
+        for (tag, log) in [("A", a), ("B", b)] {
+            if side_channel_width(&log.iface, name) == 0 {
+                return Err(Error::Usage(format!(
+                    "{flag} was given, but {tag} ({}) declares no `{name}` side channel: \
+                     comparing it would report a difference for every transaction",
+                    log.file.display()
+                )));
+            }
+        }
+    }
+    Ok(())
+}
+
 /// `--x-policy=error`: refuse to go on if any *enabled* byte carries x/z.
 pub fn check_unknowns(log: &LoadedLog, side: Side, ctx: &CmpCtx) -> Result<()> {
     if ctx.opts.x != XPolicy::Error {
@@ -702,6 +741,27 @@ mod tests {
         assert!(beats_equal(&a, &b, &lenient));
     }
 
+    #[test]
+    fn refuses_to_compare_a_side_channel_that_is_not_there() {
+        // An old HCI-Core interface has no `id` signal at all, so its log
+        // declares IW = 0; comparing `id` anyway would flag every transaction.
+        let no_id = req_log(vec![req_beat(0, 1, "0x0", "0x1", "0xf")]);
+        let c = CmpCtx::new(
+            Mode::HciReq,
+            &no_id,
+            &no_id,
+            32,
+            CompareOptions { check_id: true, ..Default::default() },
+        );
+        let err = check_side_channels(&no_id, &no_id, &c).unwrap_err();
+        assert!(err.to_string().contains("--check-id"), "{err}");
+
+        // With the side channel present, it is compared as asked.
+        let mut with_id = req_log(vec![req_beat(0, 1, "0x0", "0x1", "0xf")]);
+        with_id.iface = Iface::Hci { dw: 32, aw: 32, bw: 8, uw: 0, iw: 8, ew: 0, ehw: 0 };
+        assert!(check_side_channels(&with_id, &with_id, &c).is_ok());
+    }
+
     #[test]
     fn keep_writes_drops_loads() {
         let mut log = req_log(vec![

tracer/src/compare.rs

@@ -11,7 +11,9 @@ use std::path::Path;
 
 use crate::cli::{Cli, Command, CompareArgs};
 use crate::color::{use_color, Palette, RealEnv};
-use crate::compare::{beats_equal, check_unknowns, key_of, keep_writes, CmpCtx, Mode, Side};
+use crate::compare::{
+    beats_equal, check_side_channels, check_unknowns, key_of, keep_writes, CmpCtx, Mode, Side,
+};
 use crate::diff::{diff, downgrade_replace, DiffOp, DiffStats};
 use crate::error::{Error, Result};
 use crate::load::{load_hci_request, load_hci_response, load_hwpe_stream, probe_kind, RepairMode};
@@ -152,6 +154,7 @@ fn compare(mode: Mode, mut a: LoadedLog, mut b: LoadedLog, args: &CompareArgs) -
     let width = normalize_widths(&mut a, &mut b, args.split, args.drop_empty)?;
     let ctx = CmpCtx::new(mode, &a, &b, width, args.options());
 
+    check_side_channels(&a, &b, &ctx)?;
     check_unknowns(&a, Side::A, &ctx)?;
     check_unknowns(&b, Side::B, &ctx)?;