On Fri, 2022-01-07 at 10:36 +0300, Dan Carpenter wrote: > Unfortunately this code has stumbled into some deep C standards > nonsense. These two structs have a 3 byte struct hole at the end. If > you partially initialize a struct then the C standard specifies that > all the struct holes are zeroed out. But when you initialize all the > members of the struct, as this code does, then struct holes may be left > with uninitialized stack data. This is from C11 section 6.7.9 and how > it is implemented in GCC. Wow, nice find ... > + memset(&data, 0, sizeof(data)); > + data.cmd = cpu_to_le32(MURU_SET_TXC_TX_STATS_EN); > + data.enable = enabled; > Maybe add a comment? This is not going to be obvious in the future. > return mt76_mcu_send_msg(&dev->mt76, MCU_EXT_CMD(MURU_CTRL), > &data, > sizeof(data), false); Or maybe instead just mark the thing __packed (and/or explicitly add the padding if needed), it seems weird that we'd send something to the *firmware* that has a struct layout subject to compiler/arch padding rules. johannes
