ld: exact product_id match wins across all profile sections

find_by_drive_id ran the four-field fallback per section, so an earlier
section's fallback beat a later section's exact match. Exact pass now spans
all sections first; fallback uniqueness is checked across all sections.
This commit is contained in:
Matthew Jackson
2026-09-26 20:10:06 -07:00
parent 1d20be4a41
commit 2e2025e51c
+63 -47
View File
@@ -308,12 +308,12 @@ fn load_from_str(data: &str) -> Result<Profiles> {
/// Find a profile matching a drive's INQUIRY fields, whitespace-trimmed. /// Find a profile matching a drive's INQUIRY fields, whitespace-trimmed.
/// ///
/// Per platform section (MT1959-A, -B, Renesas): (1) exact match including /// Over ALL platform sections (MT1959-A, -B, Renesas, in order): (1) exact
/// `product_id` when the drive reports one; (2) product-id-blind fallback on /// match including `product_id` when the drive reports one; then (2) a
/// the four-field identity (vendor/revision/vendor_specific/firmware_date). /// product-id-blind fallback on vendor/revision/vendor_specific/firmware_date.
/// Catalogs store a GENERIC product_id (e.g. "BD-RE") vs a drive's specific one /// Catalogs store a GENERIC product_id ("BD-RE") vs a drive's specific one
/// ("BD-RE BU40N"), so pass 1 misses; pass 2 binds a UNIQUE four-field match — /// ("BD-RE BU40N"), so pass 1 misses; pass 2 binds a four-field match UNIQUE
/// a shared four-tuple binds only when the drive reported no product_id. /// across all sections — a shared tuple binds only with no product_id.
pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Option<ProfileMatch> { pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Option<ProfileMatch> {
let v = drive_id.vendor_id.trim(); let v = drive_id.vendor_id.trim();
let prod = drive_id.product_id.trim(); let prod = drive_id.product_id.trim();
@@ -321,19 +321,26 @@ pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Optio
let vs = drive_id.vendor_specific.trim(); let vs = drive_id.vendor_specific.trim();
let date = drive_id.firmware_date.trim(); let date = drive_id.firmware_date.trim();
for (platform, list) in [ let sections = [
(Platform::Mt1959A, &profiles.mt1959_a), (Platform::Mt1959A, &profiles.mt1959_a),
(Platform::Mt1959B, &profiles.mt1959_b), (Platform::Mt1959B, &profiles.mt1959_b),
(Platform::Renesas, &profiles.renesas), (Platform::Renesas, &profiles.renesas),
] { ];
if !prod.is_empty() let four_field = |p: &DriveProfile| {
&& let Some(p) = list.iter().find(|p| {
p.identity.vendor_id.trim() == v p.identity.vendor_id.trim() == v
&& p.identity.product_id.trim() == prod
&& p.identity.product_revision.trim() == r && p.identity.product_revision.trim() == r
&& p.identity.vendor_specific.trim() == vs && p.identity.vendor_specific.trim() == vs
&& p.identity.firmware_date.trim() == date && p.identity.firmware_date.trim() == date
}) };
let all = || {
sections
.iter()
.flat_map(|(platform, list)| list.iter().map(move |p| (*platform, p)))
};
if !prod.is_empty()
&& let Some((platform, p)) =
all().find(|(_, p)| four_field(p) && p.identity.product_id.trim() == prod)
{ {
return Some(ProfileMatch { return Some(ProfileMatch {
profile: p.clone(), profile: p.clone(),
@@ -341,42 +348,18 @@ pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Optio
}); });
} }
// Product-id-blind fallback: catalogs store a GENERIC product_id while // Unique across all sections: bind regardless of the reported product_id
// drives report a specific one, so the exact pass misses. Match the // (the common real-drive case a UHD LG BU40N relies on). A shared tuple
// four-field identity instead; guard mis-binding on shared tuples below. // binds only with NO product_id; otherwise it's an uncataloged variant.
let four_field: Vec<&DriveProfile> = list let mut matches = all().filter(|(_, p)| four_field(p));
.iter() let (platform, first) = matches.next()?;
.filter(|p| { if matches.next().is_some() && !prod.is_empty() {
p.identity.vendor_id.trim() == v return None;
&& p.identity.product_revision.trim() == r }
&& p.identity.vendor_specific.trim() == vs Some(ProfileMatch {
&& p.identity.firmware_date.trim() == date profile: first.clone(),
platform,
}) })
.collect();
match four_field.as_slice() {
// Exactly one four-field match — unambiguous; bind regardless of
// the reported product_id (the common real-drive case, and the
// pre-1.7.0 behaviour a UHD-capable LG BU40N relied on).
[only] => {
return Some(ProfileMatch {
profile: (*only).clone(),
platform,
});
}
// Shared four-tuple: bind only when the drive reported NO
// product_id. A non-empty one that missed the exact pass is an
// uncataloged variant and must not mis-bind to a sibling.
[first, ..] if prod.is_empty() => {
return Some(ProfileMatch {
profile: (*first).clone(),
platform,
});
}
_ => {}
}
}
None
} }
#[cfg(test)] #[cfg(test)]
@@ -473,6 +456,39 @@ mod tests {
assert_eq!(m.platform, Platform::Mt1959A); assert_eq!(m.platform, Platform::Mt1959A);
} }
/// An exact product_id match in a LATER section must beat a four-field
/// fallback in an earlier one; fallback uniqueness spans all sections.
#[test]
fn find_by_drive_id_exact_match_beats_earlier_section_fallback() {
use serde_json::json;
let entry = |prod: &str, sig: &str| {
json!({"identity": {"vendor_id":"HL-DT-ST","product_id":prod,
"product_revision":"1.03","vendor_specific":"NM00000",
"firmware_date":"211810241934"},
"signature":sig,"firmware":""})
};
let profiles: Profiles = serde_json::from_value(json!({
"mt1959_a": [entry("BD-RE", "aaaaaaaa")],
"mt1959_b": [entry("BD-RE BU40N", "bbbbbbbb")],
}))
.unwrap();
let mut id = make_drive_id("HL-DT-ST", "1.03", "NM00000", "211810241934");
id.product_id = "BD-RE BU40N".to_string();
let m = find_by_drive_id(&profiles, &id).expect("B's exact match");
assert_eq!(m.platform, Platform::Mt1959B);
assert_eq!(m.profile.signature, [0xbb, 0xbb, 0xbb, 0xbb]);
// Tuple shared across sections + uncataloged product_id: no match.
id.product_id = "BD-RE WH16NS".to_string();
assert!(find_by_drive_id(&profiles, &id).is_none());
// No product_id: shared tuple binds the first section's entry.
id.product_id.clear();
let m0 = find_by_drive_id(&profiles, &id).expect("empty product_id binds");
assert_eq!(m0.platform, Platform::Mt1959A);
}
#[test] #[test]
fn test_find_known_drive() { fn test_find_known_drive() {
let profiles = load_bundled().unwrap(); let profiles = load_bundled().unwrap();