Skip to content

Commit 8712492

Browse files
authored
host-sp-messages: don't copy that floppy^H^H^H^H^H^HInventoryData (#2253)
The `InventoryData` enum in `host-sp-messages` is *real* big: at least 512B for the `DimmSpd` message, or perhaps more if one of the other variants whose fields I didn't want to add up is more than that. Currently, it `#[derive(Copy)]`s, which feels like a big stack overflow footgun: if an `InventoryData` value is accidentally moved by value, it gets copied on the stack. This kills the Hubris. Thus, this commit just removes the `#[derive(Copy)]`, which should take the bullets out of the footgun. Other `host-sp-messages` are left alone, and everything still builds fine. While the `SpToHost` and `HostToSp` messages also derive `Copy`, they aren't as big, and the `Copy` impl is necessary because they get stuck in ringbufs.
1 parent 674e868 commit 8712492

1 file changed

Lines changed: 6 additions & 1 deletion

File tree

  • lib/host-sp-messages/src

lib/host-sp-messages/src/lib.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,13 @@ impl From<drv_i2c_types::ResponseCode> for InventoryDataResult {
292292
/// These **cannot be reordered**; the host and SP must agree on them. New
293293
/// variants may be added to the end, and existing variants may be extended with
294294
/// new data (at the end), but no changes should be made to existing bytes.
295+
// Note: this type may ask you to let it derive `Copy`. DO NOT ALLOW THIS! An
296+
// `InventoryData` is *really big* --- over 512 bytes --- and deriving `Copy`
297+
// would make it easy to accidentally pass one by value on the stack, increasing
298+
// stack usage when it isn't necessary to do so. We would like to only allow
299+
// this type to be bytewise-copied explicitly by calling `.clone()`, instead.
295300
#[derive(
296-
Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, SerializedSize,
301+
Debug, Clone, PartialEq, Eq, Deserialize, Serialize, SerializedSize,
297302
)]
298303
#[allow(clippy::large_enum_variant)]
299304
pub enum InventoryData {

0 commit comments

Comments
 (0)