diff --git a/Cargo.lock b/Cargo.lock index abae724e5..981e33e75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3266,7 +3266,7 @@ dependencies = [ [[package]] name = "gateway-messages" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/management-gateway-service#745a508cb97b7ca9b4c10ec9592c980eb769b10d" +source = "git+https://github.com/oxidecomputer/management-gateway-service#9dd63c0d7533e333c2ebf522f1929443041fd726" dependencies = [ "bitflags 2.9.4", "hubpack", diff --git a/task/control-plane-agent/src/inventory.rs b/task/control-plane-agent/src/inventory.rs index 1ccc860bd..90085e2e8 100644 --- a/task/control-plane-agent/src/inventory.rs +++ b/task/control-plane-agent/src/inventory.rs @@ -129,8 +129,21 @@ impl Inventory { if !device.sensors.is_empty() { capabilities |= DeviceCapabilities::HAS_MEASUREMENT_CHANNELS; } + + // NOTE: the `from_bstr_unchecked` method expects that: + // + // 1. The given bytes contain utf-8 data + // 2. The given slice is <= SpComponent::MAX_ID_LENGTH + // + // Since we pass the bytes of a `str` (always good utf-8!), and our + // `str`s are built (and length-checked) at compile time, use of this + // method is justified. You don't see an unsafe block here, because + // SpComponent can be received over the wire, so even if we violated + // the rules above, there would be no potential soundness concerns. + let component = SpComponent::from_bstr_unchecked(device.id.as_bytes()); + DeviceDescription { - component: SpComponent { id: device.id }, + component, device: device.device, description: device.description, capabilities, @@ -178,8 +191,13 @@ impl TryFrom<&'_ SpComponent> for Index { type Error = SpError; fn try_from(component: &'_ SpComponent) -> Result { + // TODO(AJM): implement PartialEq/PartialOrd for `SpComponent` et. al, + // then make this nicer. We'll want this for some follow-up PMBus + // changes as well. if let Ok(entry_idx) = task_validate_api::DEVICE_INDICES_BY_SORTED_ID - .binary_search_by_key(&component.id, |&(id, _)| id) + .binary_search_by_key(&component.as_bstr(), |&(id, _)| { + id.as_bytes() + }) { let &(_, index) = task_validate_api::DEVICE_INDICES_BY_SORTED_ID .get(entry_idx) diff --git a/task/control-plane-agent/src/update/rot.rs b/task/control-plane-agent/src/update/rot.rs index 302bc395a..f5aa91b9f 100644 --- a/task/control-plane-agent/src/update/rot.rs +++ b/task/control-plane-agent/src/update/rot.rs @@ -78,7 +78,7 @@ impl ComponentUpdater for RotUpdate { .map_err(SpError::OtherComponentUpdateInProgress)?; // Which target are we updating? - ringbuf_entry!(Trace::Target(update.component.id[0], update.slot)); + ringbuf_entry!(Trace::Target(update.component.id()[0], update.slot)); let target = match (update.component, update.slot) { (SpComponent::ROT, 0) => UpdateTarget::ImageA, (SpComponent::ROT, 1) => UpdateTarget::ImageB, diff --git a/task/validate-api/build.rs b/task/validate-api/build.rs index 4f0d3c8b4..f57ed37e5 100644 --- a/task/validate-api/build.rs +++ b/task/validate-api/build.rs @@ -63,10 +63,10 @@ fn write_pub_device_descriptions() -> anyhow::Result<()> { writeln!(file, " DeviceDescription {{")?; writeln!(file, " device: {:?},", dev.device)?; writeln!(file, " description: {:?},", dev.description)?; - if let Some(id) = dev.device_id { - if let Ok(component) = SpComponent::try_from(id.as_ref()) { - writeln!(file, " id: {:?},", component.id)?; - if id2idx.insert(component.id, idx).is_some() { + if let Some(id) = dev.device_id.as_ref() { + if id.len() <= SpComponent::MAX_ID_LENGTH { + writeln!(file, " id: \"{id}\",")?; + if id2idx.insert(id.to_string(), idx).is_some() { println!("cargo::error=duplicate device id {id:?}",); duplicate_ids += 1; } @@ -106,11 +106,11 @@ fn write_pub_device_descriptions() -> anyhow::Result<()> { writeln!( file, - "pub static DEVICE_INDICES_BY_SORTED_ID: [([u8; MAX_ID_LENGTH], usize); {}] = [", + "pub static DEVICE_INDICES_BY_SORTED_ID: [(&str, usize); {}] = [", id2idx.len() )?; for (id, idx) in id2idx { - writeln!(file, " ({id:?}, {idx}),")?; + writeln!(file, " (\"{id}\", {idx}),")?; } writeln!(file, "];")?; diff --git a/task/validate-api/src/lib.rs b/task/validate-api/src/lib.rs index b55d93cd1..0725db1b1 100644 --- a/task/validate-api/src/lib.rs +++ b/task/validate-api/src/lib.rs @@ -74,7 +74,7 @@ pub struct DeviceDescription { pub device: &'static str, pub description: &'static str, pub sensors: &'static [SensorDescription], - pub id: [u8; MAX_ID_LENGTH], + pub id: &'static str, pub is_pmbus: bool, }